Don't link plugins with libhts.dylib on macOS (correction to PR 1072)#1184
Merged
daviesrob merged 1 commit intosamtools:developfrom Dec 15, 2020
Merged
Don't link plugins with libhts.dylib on macOS (correction to PR 1072)#1184daviesrob merged 1 commit intosamtools:developfrom
daviesrob merged 1 commit intosamtools:developfrom
Conversation
PR samtools#1072 changed plugin linking so that plugins are linked back to the dynamic libhts.so/.dylib, to facilitate use when libhts is itself dynamically dlopen()ed with RTLD_LOCAL, e.g., by the Python runtime which uses default dlopen() flags which on Linux means RTLD_LOCAL. This broke plugin loading on macOS when opening plugins in an executable in which libhts.a has been statically linked, as there were then two copies of the library globals (notably hfile.c::schemes), one from the executable's libhts.a and one from the plugin's libhts.NN.dylib. (The Linux loading model does not suffer from this issue.) The default dlopen() flag on macOS is RTLD_GLOBAL, so this can be fixed by reverting the change (on macOS only) and depending on the symbols supplied by a static libhts.a, a dynamically linked libhts.NN.dylib, or a RTLD_GLOBALly dlopen()ed libhts.NN.dylib. This rebreaks the case of dlopen()ing libhts on macOS while explicitly specifying RTLD_LOCAL, but this is not a common case. Fixes samtools#1176. Disable the `plugins-dlhts -l` test case on macOS. Add a test of accessing plugins from an executable with a statically linked libhts.a (namely, htsfile) to test/test.pl.
Member
|
Thanks. At some point we should look into a better method for plug-ins to get the symbols needed from the HTSlib that loaded them. Meanwhile this looks like a good improvement to the current system. |
jmarshall
added a commit
to jmarshall/pysam
that referenced
this pull request
Dec 15, 2020
Apply PR samtools/htslib#1184. On macOS, plugins pulling in libhts.dylib conflicts with a statically linked libhts.a from the main executable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[A followup to PR #1072, needed to fix #1176: the previous PR causes breakage limited to macOS when the main executable contains a statically linked libhts.a.]
PR #1072 changed plugin linking so that plugins are linked back to the dynamic libhts.so/.dylib, to facilitate use when libhts is itself dynamically dlopen()ed with RTLD_LOCAL, e.g., by the Python runtime which uses default dlopen() flags which on Linux means RTLD_LOCAL.
This broke plugin loading on macOS when opening plugins in an executable in which libhts.a has been statically linked, as there were then two copies of the library globals (notably hfile.c's
schemes), one from the executable's libhts.a and one from the plugin's libhts.NN.dylib. (The Linux loading model does not suffer from this issue.)The default dlopen() flag on macOS is RTLD_GLOBAL, so this can be fixed by reverting the change (on macOS only) and depending on the symbols supplied by a static libhts.a, a dynamically linked libhts.NN.dylib, or a RTLD_GLOBALly dlopen()ed libhts.NN.dylib. This rebreaks the case of dlopen()ing libhts on macOS while explicitly specifying RTLD_LOCAL, but this is not a common case.
Correspondingly adjust tests:
plugins-dlhts -ltest case on macOS.