Review Request JDK-8164512: Replace ClassLoader use of finalizer with phantom reference to unload native library
david.holmes at oracle.com
Thu Oct 5 01:21:27 UTC 2017
On 5/10/2017 9:24 AM, mandy chung wrote:
> Updated webrev:
> JNI FindClass change has been separated from this patch . I made
> further clean up to the NativeLibrary implementation and replaced the
> use of Vector/Stack. I also added a native test to verify the native
> library can be unloaded and reloaded.
> Summary: The patch replaces the ClassLoader use of finalizer with
> phantom reference, specifically Cleaner, for unloading native
> libraries. It registers the class loader for cleanup only if it's not a
> builtin class loader which will never be unloaded.
This comment is not a sentence:
2442 /* If the library is being loaded (must be by the
2443 * because Runtime.load and Runtime.loadLibrary are
2444 * synchronous).
Overall the changes seem fine, but I do have a concern about the use of
ConcurrentHashMap. This would seem to be a significant change in the
initialization order - and I'm wondering how libjava itself actually
gets loaded ??
I assume the tests will be updated to use JNI_VERSION_10, assuming you
push the FindClass changes first?
Not sure I see the point in the FindClass("java/lang/ClassLoader") as if
it fails to find it then surely it already failed to find
"java/lang/Error" and you'll possibly crash trying to throw.
Not clear how/if this actually verifies any unloading actually takes
place? Also if the OnUnload throws an error and that happens in the
Cleaner thread, how would that exception result in the test reporting a
I think OnUnload has to update some state stored in the main test class
so that it can confirm the unloading occurred successfully, or not.
More information about the core-libs-dev