On Thu, 16 Oct 2025 22:21:19 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Albert Mingkun Yang has updated the pull request with a new target base due
>> to a merge or a rebase. The incremental webrev excludes the unrelated
>> changes brought in by the merge/rebase. The pull request contains four
>> additional commits since the last revision:
>>
>> - Merge branch 'master' into test-unload
>> - copyright
>> - review
>> - test-unload
>
> test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 251:
>
>> 249:
>> 250: // force GC to unload marked class loader and its classes
>> 251: if (isClassLoaderReclaimed()) {
>
> Q: There was a wait loop before to wait for `ClassLoader` to be reclaimed.
> How does this work now with the `isClassLoaderReclaimed()`?
The previous approach relied on an async notification by the Cleaner thread
that the class had been gc'd, so some waiting was needed (but probably not 15
seconds). There were also bugs in this support. unloadClass() is first called
by the test to make sure that the class **does not** unload due to an
intentional reference. This first call sets customClassLoader to null. That
means when later unloadClass() is used again to this time make sure the class
unloads, there will be no waiting, because for some reason unloadClass() chose
to set waitingTime to 0 when customClassLoader is null. This meant that often
unloadClass() would exit before the unloading completed. This is probably why
the caller used to try up to 5 times. That left a race condition with the
setting of is_reclaimed. It could be set sometime after unloadClass() finished
and before it was entered again, but upon entry is_reclaimed is always set to
false, and it will never be set true again after that. Thus the test thinks
the class never unloaded even though it did.
There are other ways to fix this issue (mainly always wait for 15 seconds and
make sure unloadClass() always works on the first try). However, Albert's
PhantomReference approach is a cleaner implementation and no longer relies
waiting for some async process to complete the unloading of the class.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27840#discussion_r2437639816
