Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
On Fri, 11 Feb 2022 10:11:50 GMT, Maurizio Cimadamore wrote: > there is still residual common logic in how clients are expected to load > libraries (e.g. either using a file name/absolute path, or using a library > name, without file separator chars). These assumption seem very heavily > influenced by how System::load/loadLibrary do things, I believe - and I > wonder if they can, in the future, create other obstacles for a raw kind of > library loading (which expects to be mostly a wrapper around > dlopen/LoadLibrary). But that's a question/problem for another day. Good point. With the removal of the restriction, the raw library loading implementation can further be cleaned up (not be tied with the jni library loading logic). I'll look into what it takes and whether I should do it in this PR. - PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
On Fri, 11 Feb 2022 03:49:45 GMT, Mandy Chung wrote: >> This patch removes the restriction in the raw library loading mechanism that >> does not allow mix-n-match of loading a library as a JNI library and as a >> raw library. >> >> The raw library loading mechanism is designed for panama to load native >> library essentially equivalent to dlopen/dlclose calls independent of JNI >> library loading. If a native library is loaded as a JNI library and a raw >> library, it will get different NativeLibrary instances. When a class loader >> is being unloaded, JNI_Unload will be invoked but the native library may not >> be unloaded until NativeLibrary::unload is explicitly called for the raw >> library. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > review comment Looks good. I like the fact that the factory for raw library now takes a MH.lookup instead of a class. And I also like that the paths for loading a library in raw mode vs. JNI mode are more clearly separated, I think this should help Panama, thanks! One fly-by comment is that there is still residual common logic in how clients are expected to load libraries (e.g. either using a file name/absolute path, or using a library name, without file separator chars). These assumption seem very heavily influenced by how System::load/loadLibrary do things, I believe - and I wonder if they can, in the future, create other obstacles for a raw kind of library loading (which expects to be mostly a wrapper around dlopen/LoadLibrary). But that's a question/problem for another day. - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
On Fri, 11 Feb 2022 03:49:45 GMT, Mandy Chung wrote: >> This patch removes the restriction in the raw library loading mechanism that >> does not allow mix-n-match of loading a library as a JNI library and as a >> raw library. >> >> The raw library loading mechanism is designed for panama to load native >> library essentially equivalent to dlopen/dlclose calls independent of JNI >> library loading. If a native library is loaded as a JNI library and a raw >> library, it will get different NativeLibrary instances. When a class loader >> is being unloaded, JNI_Unload will be invoked but the native library may not >> be unloaded until NativeLibrary::unload is explicitly called for the raw >> library. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > review comment Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
> This patch removes the restriction in the raw library loading mechanism that > does not allow mix-n-match of loading a library as a JNI library and as a raw > library. > > The raw library loading mechanism is designed for panama to load native > library essentially equivalent to dlopen/dlclose calls independent of JNI > library loading. If a native library is loaded as a JNI library and a raw > library, it will get different NativeLibrary instances. When a class loader > is being unloaded, JNI_Unload will be invoked but the native library may not > be unloaded until NativeLibrary::unload is explicitly called for the raw > library. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: review comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/7435/files - new: https://git.openjdk.java.net/jdk/pull/7435/files/bbb44b6b..6e492d2b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7435=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7435=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7435.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7435/head:pull/7435 PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
On Fri, 11 Feb 2022 02:38:08 GMT, Athijegannathan Sundararajan
wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review comment
>
> src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 58:
>
>> 56: * will fail.
>> 57: */
>> 58: public abstract class NativeLibraries {
>
> could this be sealed with only two specific subtypes defined here?
It could. It's just an internal class and not so much of a concern of
unexpected subclasses. I'll leave it as is.
> src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 251:
>
>> 249: // do not search java.library.path
>> 250: super(loader, loader != null ? null : NativeLibraries.class,
>> 251: loader != null ? true : false);
>
> The last argument expression of the super class could be just loader != null
Updated. Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/7435
