Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default search paths for Unix systems #264

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fzakaria
Copy link

The JDK already includes the default search paths for Unix/Linux through it's system property java.library.path

You can see the default path set by the JDK for Unix/Linux here

In an effort to enforce reproducibility for the NixOS project, we've recently patched the JDK8 to clear out the default java.library.path - NixOS/nixpkgs#123708

Unfortunately, this does not bubble up to thejnr-ffi gem since they additionally hardcode the values.

This contribution removes that hardcoded list. In the normal case, the list remains the same since it will get the default list through java.library.path however now it can be cleared out appropriately.

The JDK already includes the default search paths for Unix/Linux through
it's system property `java.library.path`

You can see the default path set by the JDK for Unix/Linux here: https://github.com/AdoptOpenJDK/openjdk-jdk8u/blob/master/hotspot/src/os/linux/vm/os_linux.cpp#L371

In an effort to enforce reproducibility for the NixOS project, we've
recently patched the JDK8 to clear out the default java.library.path
NixOS/nixpkgs#123708

Unfortunately, this does not bubble up to the Jnr-FFI since they
additionally hardcode the values.

This commit removes that hardcoded list.
@basshelal
Copy link
Contributor

I added this because macOS didn't include these paths by default or even any reasonable Unix lib paths at all.
I mentioned this here #229 (comment)

I retried this today with different JDKs (adoptopenjdk 8,11 and 16) to similar results. I believe those paths that are returned from java.library.path are for the jvm dylibs and nothing more. GNU/Linux uses /etc/ld.so.conf and /etc/ld.so.conf.d/ which will include the default Unix lib paths and we already search that but macOS has no similar configuration files that I'm aware of (could be wrong, not a macOS expert).

I agree with you that hard-coding them probably wasn't the right solution, at least for GNU/Linux. I think the better solution might be to change:

if (Platform.getNativePlatform().isUnix())

to:

if (Platform.getNativePlatform().getOS() == Platform.OS.DARWIN)

Removing the hard-coded values from macOS means consumers have to manually add them which might not be a good idea? I don't know, it's annoying that macOS doesn't have an elegant solution to custom lib paths like GNU/Linux does.

This is honestly more of a macOS problem than a Unix one because even the system dylibs are in obscure places that are not easy to deterministically get. I can always check if libasound.so is installed on a GNU/Linux machine with ease because /etc/ld.so.conf and /etc/ld.so.conf.d/ will point to the default locations where it will be found but on macOS the system dylibs are in odd locations and have been changing (old stackoverflow answers are now incorrect).

What's your opinion on this? Pinging @headius as well for his opionion

@fzakaria
Copy link
Author

@basshelal I'm not on Darwin so I can't test but the source code looks like it has similar default paths.
https://github.com/AdoptOpenJDK/openjdk-jdk8u/blob/master/hotspot/src/os/bsd/vm/os_bsd.cpp#L350

#ifndef DEFAULT_LIBPATH
#define DEFAULT_LIBPATH "/lib:/usr/lib"
#endif

Hardcoding paths cannot be the answer here.
The whole point of leveraging system properties is so that it's configurable.

I am in full support of this change and disagree that it should be kept even for Darwin.

@basshelal
Copy link
Contributor

I never got these no matter what I tried, only paths relating to the jvm dylib locations.

I must be doing something wrong.

Even then though, DYLD_LIBRARY_PATH and LD_LIBRARY_PATH are empty by default on macOS unless you export them yourself in your terminal, but even then, the JVM can't even read them because not all env variables are passed to a subprocess. See this: https://stackoverflow.com/questions/60126159/how-to-set-ld-library-path-dyld-library-path-on-macos

If I'm not doing anything wrong then there is no elegant solution. Hardcoding is wrong but macOS is just stupid if this is the case, consumers have to be aware that macOS does not provide you with system-wide default locations and you have to add them yourself, even for a simple search.

So if you have libfoo.dylib installed in /lib/ or other Unix lib locations Platform.libraryLocations("foo", null) will be empty.

We need to make sure consumers aware of this in case they waste time like I did wondering why JNR-FFI can't find their newly installed libjack.dylib because macOS doesn't want us to find it unless we ask explicitly with the expected standard Unix locations.

@headius
Copy link
Member

headius commented Aug 31, 2021

I would like to spin some JNR releases today in advance of the JRuby 9.3 release, but I think this PR still has some open questions at least on Darwin. @basshelal @fzakaria did you have any other conversations about this?

@headius headius added this to the 2.2.6 milestone Aug 31, 2021
@fzakaria
Copy link
Author

I still don't believe the approach taken earlier was correct -- the search paths added aren't even idiomatic in MacOS as places to search.

I will fork it for our own needs to maintain hermeticity for the work we are doing but wanted to contribute the work upstream.

Thank you for your time reviewing it. I don't think there's a rush to get it approved (although I would be in favor of it).

@headius
Copy link
Member

headius commented Aug 31, 2021

I confirmed locally what @basshelal pointed out: Darwin OpenJDK/Hotspot does not include any of the standard lib paths in java.library.path by default.

$ java -XshowSettings:properties
...
    java.library.path = /Users/headius/Library/Java/Extensions
        /Library/Java/Extensions
        /Network/Library/Java/Extensions
        /System/Library/Java/Extensions
        /usr/lib/java
        .

This is confirmed on 11 and 17. Interestingly, OpenJ9 does include one standard path:

    java.library.path = /Library/Java/JavaVirtualMachines/adoptopenjdk-11-openj9.jdk/Contents/Home/lib/compressedrefs
        /Library/Java/JavaVirtualMachines/adoptopenjdk-11-openj9.jdk/Contents/Home/lib
        /usr/lib

So we at least have one proven case where the default (i.e. unmodified) java.library.path is not a suitable fallback when searching for system libraries. Given that I think we need a bit more discussion about how to handle (and whether to handle) this peculiar case on Darwin.

@headius
Copy link
Member

headius commented Sep 1, 2021

At least one other JDK hacker, @shipilev, believes this may be a bug, so I have filed an issue with OpenJDK: https://bugs.openjdk.java.net/browse/JDK-8273222

@headius headius modified the milestones: 2.2.6, 2.2.7 Sep 7, 2021
@headius headius modified the milestones: 2.2.7, 2.2.8 Sep 16, 2021
@headius headius modified the milestones: 2.2.8, 2.2.9 Oct 26, 2021
@headius headius modified the milestones: 2.2.9, 2.2.10 Nov 22, 2021
@headius headius modified the milestones: 2.2.10, 2.2.11 Dec 1, 2021
@headius headius modified the milestones: 2.2.11, 2.2.12 Jan 6, 2022
@headius headius modified the milestones: 2.2.12, 2.2.13 Mar 10, 2022
@headius headius modified the milestones: 2.2.13, 2.2.14 Nov 10, 2022
@briceburg
Copy link

Can the default lib paths be configurable via a system property or environment variable before falling back to the hard-coded values?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants