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

Need to try all paths listed by 'java.library.path' before throwing UnsatisfiedLinkError #1136

Merged
merged 1 commit into from Apr 24, 2019

Conversation

Sanne
Copy link
Contributor

@Sanne Sanne commented Apr 5, 2019

I believe this could be the fix for #438

The code in :

public void loadLibrary(String name, boolean isAbsolute) {
if (paths == null) {
String[] tokens = SubstrateUtil.split(System.getProperty("java.library.path", ""), File.pathSeparator);
paths = Arrays.stream(tokens).map(t -> t.isEmpty() ? "." : t).toArray(String[]::new);
}

is able to deal with the requirement to list multiple paths for the java.library.path system property, but then the code attempting to load the libraries is expecting to have "false" returned on failure from one path, so to try the next path.

However the failed attempt to load the library from the first path immediately causes an UnsatisfiedLinkError to be thrown, so the additional paths are never tested.

see also:

I'd be happy to add an integration test, but I'm not familiar with how you might prefer something like this to be structured. If you could point me to something similar I'd be glad to learn and try putting one together.

Thanks

@AzeemJiva
Copy link

Did you run manual tests? Various permutations (single path exception thrown or not, multiple paths, etc)? Looks reasonable but question what happens if the last path in the list doesn't satisfy the linker?

@Sanne
Copy link
Contributor Author

Sanne commented Apr 9, 2019

hi @AzeemJiva , yes I tested this with local builds and it fixes all our issues with loading libraries.

If no single path satisfies the loading, an UnsatisfiedLinkError is thrown: same semantics as previously.

@Sanne
Copy link
Contributor Author

Sanne commented Apr 23, 2019

@AzeemJiva @cstancu could you please have a look?

This is a blocker to use SSL / https on Linux, we'd really hope to have it in a release soon.

@AzeemJiva
Copy link

LGTM, but I don't have merge permissions.

@graalvmbot graalvmbot merged commit cbadcf6 into oracle:master Apr 24, 2019
@peter-hofer
Copy link
Member

Thank you for your submission.

@Sanne
Copy link
Contributor Author

Sanne commented Apr 24, 2019

thanks all!

@Sanne Sanne deleted the SystemLibraryLoad branch April 24, 2019 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants