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

[Kim Barrett]

> On Sep 22, 2017, at 6:18 PM, mandy chung <mandy.chung at oracle.com> 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

Thanks for dealing with this.

============================================================================== 
src/java.base/share/native/libjava/ClassLoader.c
 415 Java_java_lang_ClassLoader_00024NativeLibrary_00024Unloader_unload
 416 (JNIEnv *env, jobject this, jstring name, jboolean isBuiltin, jlong address)

With this change, the "this" argument is no longer used.

Because of this, the native function could be a static member function
of the new Unloader class, or could (I think) still be a (now static)
member function of NativeLibrary.  The latter would not require a name
change, only a signature change.  But I don't really care which class
has the method.

============================================================================== 
src/java.base/share/classes/java/lang/ClassLoader.java
2394         public void register() {
                 [...]
2406                 // register the class loader for cleanup when unloaded
2407                 if (loader != getBuiltinPlatformClassLoader() &&
2408                     loader != getBuiltinAppClassLoader()) {
2409                     CleanerFactory.cleaner()
2410                         .register(loader, new Unloader(name, handle, isBuiltin));
2411                 }

Can anything before the cleanup registration throw?  If so, do we leak
because we haven't registered the cleanup yet?  And what if the
registration itself fails to complete?

============================================================================== 
src/hotspot/share/prims/jni.cpp
 405     // Special handling to make sure JNI_OnLoad are executed in the correct class context.

s/are executed/is executed/

==============================================================================