Hi Mandy
On 5/10/2017 9:24 AM, mandy chung wrote:
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8164512/webrev.01/
JNI FindClass change has been separated from this patch [1]. 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.
src/java.base/share/classes/java/lang/ClassLoader.java
This comment is not a sentence:
2442 /* If the library is being loaded (must be by the
same thread,
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?
libnativeLibraryTest.c:
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.
NativeLibraryTest.java:
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
failure ??
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.
Thanks,
David
thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049389.html
