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

[Vladimir Ivanov]

Looks good.

Best regards,
Vladimir Ivanov

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.

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
>