> 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/ ==============================================================================