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

[David Holmes]

On 27/09/2017 12:11 PM, mandy chung wrote:
> On 9/26/17 7:06 PM, David Holmes wrote:
>>
>>> It is not tied with the Cleaner change. Instead, the FindClass bug 
>>> blocks the finalizer to Cleaner change.
>>>
>>> FindClass bug is uncovered when I implemented the change from 
>>> finalizer to Cleaner (or phantom reference).   There is a test 
>>> calling FindClass to look for a class defined by the class loader 
>>> being unloaded, say L. L is not Gc'ed and so FindClass successfully 
>>> finds the class (which resurrect the class loader which was marked 
>>> finalizable).
>>>
>>> Is that clearer?
>>
>> So the issue is only that this test breaks?? 
> 
> No.  The test reveals a bug in JNI_FindClass that uses a class loader 
> being finalized as the context when NativeLibrary is the caller.
>> And you want to change the FindClass spec to make it clear the test is 
>> what needs to be changed?
> No.   It is a bug in the hotspot implementation.   The JNI spec says 
> that the context of JNI_OnUnload being called is unknown.  The hotspot 
> implementation of FindClass uses the class loader associated with that 
> native library as the context when invoked from JNI_OnUnload which is 
> wrong.

I'm not sure I agree it is wrong. As I've said elsewhere there's a good 
chance that if you are trying to load classes via FindClass as part of a 
unload hook (which implies you are using custom classloaders), then it 
may be only the current loader or a parent (still custom) can load that 
class. But we're on the fringe of realistic expectations here as the 
context is specified as being "unknown".

That said given the spec says "unknown" the behaviour of the VM could 
change and still be in spec.

I presume that when using a cleaner the current classloader that would 
be used by FindClass is the system loader? Hence the observed behaviour 
of FindClass "changes" if you switch to the cleaner from the finalizer - 
and can't be reverted to the old behaviour by using a command-line flag. 
Hence if we want to be able to revert we have to do that in a 
FindClass-only change first. Then drop-in the cleaner update and remove 
the flag.

> I will file a separate JBS issue to separate this JNI bug.

Okay. I see this as a RFE not a bug per-se: change from "unknown 
context" to a specific well known context.

Thanks,
David

> Mandy