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

[mandy chung]

On 9/27/17 5:49 AM, David Holmes wrote:
>
> I missed the fact that we already special case this for JNI_OnLoad and 
> JNI_OnUnload. 

Yes this is buried in JNI_FindClass method.
> I would have thought that in the OnLoad case we would find the 
> classloader of the class loading the native library without any need 
> to resort to the NativeLibrary support code in ClassLoader. I guess 
> that this:
>
>   // Find calling class
>   Klass* k = thread->security_get_caller_class(0);
>
> does not find the "caller" that I would have expected, but instead 
> finds java.lang.System because we're executing System.loadLibrary - 
> and hence finds the boot loader not the actual loader required.
>

In the current implementation (without this change), NativeLibrary.load 
and NativeLibrary.unload native methods are the caller calling 
JNI_OnLoad and JNI_OnUnload respectively.

> But the fact we jump through all these hoops is in itself questionable 
> because the specification for JNI_FindClass does not indicate this 
> will happen. It only accounts for two cases:
>
> 1. A JNI call from a declared native method - which uses the loader of 
> the class that defines the method
>
> 2. A JNI call "through the Invocation Interface" which I interpret as 
> being a JNI call from C code, from an attached thread, with no Java 
> frames on the stack. In which case the system loader is used.
>
> A call from JNI_OnLoad (or OnUnload) does not, to me, fit either of 
> these cases; nor does JNI_OnLoad say anything about the context in 
> which it executes. So it seems we have presumed that this case should 
> mean "use the loader of the class which loaded the native library". A 
> very reasonable approach, but not one defined by the specification as 
> far as I can see. 
That's the whole point of this discussion and the spec needs clarification.
> But given this, it is not unreasonable to also use the same 
> interpretation for JNI_OnUnload.
>
That might be how it ends up the current implementation.
> So there is a gap in the specification regarding the execution context 
> of the library lifecycle function hooks - other than onUnload being an 
> "unknown context".
This suggests that we should clarify in JNI_OnLoad spec to specify the 
context.

FYI.  I file a separate issue: 
https://bugs.openjdk.java.net/browse/JDK-8188052

Mandy