[10] RFR(S): 8191360: Lookup of critical JNI method causes duplicate library loading with leaking handler

[Tobias Hartmann]

Hi Vladimir,

thanks for the review!

On 04.12.2017 19:35, Vladimir Ivanov wrote:
> PS: I'd prefer to see library-related code factored out and the library handle shared among 4 consecutive calls in
> NL::lookup_critical_entry(). But if you think it's too intrusive for the fix, please, file an RFE.

Yes, I've thought about this as well but decided not to do the refactoring since it's non-trivial and the fix might be
backported.

I'll file a separate RFE for that.

Best regards,
Tobias

> On 12/4/17 7:42 PM, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8191360
>> http://cr.openjdk.java.net/~thartmann/8191360/webrev.00/
>>
>> When creating a native wrapper, the compiler checks the shared library for a critical version of the native method
>> (-XX:+CriticalJNINatives, see comments in JDK-7013347 [1]) that can be called with reduced overhead. This is done in
>> NativeLookup::lookup_critical_style() with a call to dll_load() to get a handle to the (already loaded) library and a
>> subsequent call to dll_lookup() to get the entry address of the critical version.
>>
>> The problem is that without a call to dll_unload(), this handle to the library will stay live and internal reference
>> counting will prevent the library from ever being unloaded (even if the native method holder is unloaded). As a result,
>> static fields in the library code are not reset, causing the NativeLibraryTest to fail due to unexpected field values
>> once the library is loaded again.
>>
>> I've fixed this by closing the handle immediately after retrieving the critical entry address. This is fine because the
>> library is still kept alive by JNI (see JVM_LoadLibrary). As soon as the holder class and the library are unloaded (see
>> JVM_UnloadLibrary), the native wrapper that calls 'critical_entry' becomes unreachable and is unloaded as well.
>>
>> I've also thought about passing 'RTLD_NOLOAD' to dlopen [2] but although that avoids loading the library, it still
>> creates a handle (that needs to be closed) if the library is already loaded.
>>
>> Tested with failing test and Mach 5 hs-tier1, hs-tier2, hs-precheckin-comp (running).
>>
>> Thanks,
>> Tobias
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-7013347
>> [2] https://linux.die.net/man/3/dlopen
>>