Review Request JDK-8164512: Replace ClassLoader use of finalizer with phantom reference to unload native library

[Brent Christian]

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