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

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

Reply via email to