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

Tobias Hartmann tobias.hartmann at oracle.com
Tue Dec 5 06:50:52 UTC 2017


Thanks David.

Best regards,
Tobias

On 05.12.2017 07:30, David Holmes wrote:
> Looks good!
> 
> Thanks,
> David
> 
> On 5/12/2017 4:27 PM, Tobias Hartmann wrote:
>> Hi David,
>>
>> thanks for the review!
>>
>> On 04.12.2017 22:46, David Holmes wrote:
>>> I'm unclear why you added the critical entry to test/jdk/java/lang/ClassLoader/nativeLibrary/libnativeLibraryTest.c ??
>>> The impact of that on the test is not clear to me. It also obscures whether the test is fixed just by the actual fix.
>>
>> Right, I should have mentioned that in the RFR. The reason is that I first had a fix that only unloaded the shared
>> library if no critical method was found, i.e., if critical_entry == NULL. This fixes the crash in the original test
>> (because there is no critical version of the method). For additional coverage, I've added a critical entry to the test
>> library.
>>
>> Thinking about it again, the test changes are not necessary. Here's a webrev without:
>> http://cr.openjdk.java.net/~thartmann/8191360/webrev.01/
>>
>>> I agree with Vladimir that there is scope for being more efficient with the library handling in this code, but that's
>>> future work.
>>
>> Right, I'll file a separate RFE and link it to the bug.
>>
>> Best regards,
>> Tobias
>>>> 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
>>>>


More information about the hotspot-dev mailing list