Repository navigation
Fix issues with apps not finding libraries under ORIGIN/subdir structure - #238
Conversation
spindle_mkdir was failing to do the right thing if it was asked to make a directory with '../' path components that took it above the cachepath directory root. In the process of fixing that, I've also updated spindle_mkdir to handle symlinks properly in the cache. We now have a spindle_mkdir variant that takes the cachepath root and treats that as a root directory (so cachepath/../../ loops realpath's back to just cachepath/). This was mostly written by GPT-6/Sol with modifications/fixes by me. Co-authored-by: Codex <codex@openai.com>
… be missed. The issue was that we were dynmacially creating directories in the cache when we identified a file that needed to be cached in that directory. But ld.so was also maintaining a list of exists/not-exists directory caches, which could conflict with spindle's dynamic cache creation. To reproduce we needed a library located via $ORIGIN/something, which meant the dynamic linker was searching the spindle cache for libraries. We first needed to load another library (say libm.so in some testcases) that didn't exist in that $ORIGIN/something location. ld.so would iterate through its search list, notice that $ORIGIN/something didn't exist yet and mark it not-exists. Then we needed to try to load the original library that was in $ORIGIN/something, but ld.so would no longer search that path and the library wouldn't be found. Also this didn't happen on systems where we successfully install ld.so trampoline hooks, as the interceptions of the stat() calls would fix the result of the 'does this directory exist' test in ld.so. To fix this, we now create a directory in the cache when it is first searched, even if we do not yet have files to put in it.
Also fix issue with other tests when running fork variants of tests. Some libraries in the child process were not logged as loaded after a fork, which wasn't a problem because they were also not be logged as 'should be loaded' because those messages were being incorrect attributed to the parent process. Everything was loading okay, just the logging was working correct. Fixed both.
|
I've confirmed that this fixes the problem with Java and with the stc testcase in E4S. I notice that it's creating directories in the cache even where the original directory didn't exist, not just in cases where it does. For example, after running the reproducer with spindleuser@spindlenode:/tmp/commpath/cachepath/spindle.744c4f35ed7495ee$ tree
.
|-- home
| `-- spindleuser
| |-- 0-spindlens-dso-test-libfoo-before-libm
| |-- Spindle-inst
| | `-- lib
| | `-- spindle
| | `-- 1-spindlens-dso-libspindle_audit_pipe.so
| `-- lib
| |-- 2-spindlens-dso-libfoo.so
| `-- glibc-hwcaps
| |-- x86-64-v2
| `-- x86-64-v3
|-- lib
| `-- x86_64-linux-gnu
`-- usr
`-- lib
`-- x86_64-linux-gnu
`-- 3-spindlens-dso-libm.so.6
15 directories, 4 fileswhere there are now directories for |
|
Oops. Good catch. It was not supposed to do that. |
|
I've confirmed that this fixes the problem where it was creating directories even where one didn't exist, and the E4S |
|
The Here is a minimal reproducer for the issue: I have this layout, with two scripts in two different directories: test/
|-- bin
| `-- run.sh
`-- libexec
`-- helper.shbin/run.sh: #!/bin/bash
set -x
"$(dirname "$0")/../libexec/helper.sh"libexec/helper.sh: #!/bin/bash
echo PASSEDOn the spindleuser@spindlenode:~/test$ bash ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
PASSED
spindleuser@spindlenode:~/test$ spindle bash ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
PASSEDHowever, with this PR, it fails under Spindle: spindleuser@spindlenode:~/test$ bash ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
PASSED
spindleuser@spindlenode:~/test$ spindle bash ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
./bin/run.sh: line 3: ./bin/../libexec/helper.sh: cannot execute: required file not found
spindleuser@spindlenode:~/test$Oddly, I have to invoke the script as an argument to bash to reproduce the problem. If I run it as spindleuser@spindlenode:~/test$ ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
PASSED
spindleuser@spindlenode:~/test$ spindle ./bin/run.sh
++ dirname ./bin/run.sh
+ ./bin/../libexec/helper.sh
PASSEDHere is a log of the failure: from which the failure appears to be: |
|
Thanks for finding this. |
…s through non-existant directories.
|
The E4S testsuite now fully passes, but the Slurm Plugin failure looks legitimate and not some random fluke. I can reproduce it in the testing container locally. |
nchaimov
left a comment
There was a problem hiding this comment.
The Slurm Plugin testsuite is passing in the CI environment now, and I've run it locally as well to confirm that the problem is no longer occurring. All E4S tests pass.
The issue was that we were dynamically creating directories in the cache when we identified a file that needed to be cached in that directory. But ld.so was also maintaining a list of exists/not-exists directory caches, which could conflict with spindle's dynamic cache creation. To reproduce we needed a library located via $ORIGIN/something, which meant the dynamic linker was searching the spindle cache for libraries. We first needed to load another library (say libm.so in some testcases) that didn't exist in that $ORIGIN/something location. ld.so would iterate through its search list, notice that $ORIGIN/something didn't exist yet and mark it not-exists. Then we needed to try to load the original library that was in $ORIGIN/something, but ld.so would no longer search that path and the library wouldn't be found. Also this didn't happen on systems where we successfully install ld.so trampoline hooks, as the interceptions of the stat() calls would fix the result of the 'does this directory exist' test in ld.so. To fix this, we now create a directory in the cache when it is first searched, even if we do not yet have files to put in it.
Also add tests, and fix issues with fork tests not working properly.
Also write spindle_mkdir to support with feature.