Hi, Mandy
The changes look alright to me. One thing that I noticed:
ClassLoader.NativeLibrary.register() writes to 'loadedLibraryNames',
'nativeLibraries', and 'systemNativeLibraries' without first
synchronizing on them. There is not a bug here per se, as register() is
called from inside the needed synchronized blocks in loadLibrary0().
But perhaps it's worth a comment to call out that this is what is happening?
Thanks,
-Brent
On 9/22/17 3:18 PM, mandy chung wrote:
This patch proposes to replace the ClassLoader use of finalizer with
phantom reference, specifically Cleaner, for unloading native
libraries. It registers the class loader for cleanup only if it's not a
builtin class loader which will never be unloaded.
The spec for JNI_OnUnload [1] specifies that this function is called in
an unknown context whereas the hotspot implementation uses the class
loader being unloaded as the context, which I consider a bug. It
should not load a class defined to that class loader. The proposed spec
change for JNI_FindClass if called from JNI_OnUnload to use system class
loader (consistent with no caller context case).
Webrev:
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8164512/webrev.00/
CSR:
https://bugs.openjdk.java.net/browse/JDK-8187882
Please see the spec change an behavioral change in this CSR. A native
library may fail to be reloaded if it is loaded immediately or soon
after a class loader is GC'ed but the cleaner thread doesn't yet have
the chance to handle the unloading of the native library. Likewise,
there is no guarantee regarding the timing of finalization and a native
library may fail to reload. It's believed that the compatibility risk
should be low.
I'm looking into adding a native jtreg test that will be added in the
next revision.
Mandy
[1]
http://docs.oracle.com/javase/9/docs/specs/jni/invocation.html#jni_onunload
