On Tue, 21 Apr 2026 04:47:06 GMT, Chris Plummer <[email protected]> wrote:

>> SendaoYan has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   1. Merge two goldAlot.clear/System.gc; 2. Use while(!finalizaRun) instead 
>> of while(true)
>
> test/jdk/com/sun/jdi/FinalizerTest.java line 88:
> 
>> 86:             throw new RuntimeException(e);
>> 87:         }
>> 88:         // Now, we have to make sure the finalizer
> 
> I think the comments need to make it clear that if System.gc() plus 
> System.runFinalization() did not trigger the finalizer, then 
> FINALIZER_DONE.await() will time out and the code below will be needed as a 
> second attempt to trigger finalization. However, it's not clear to me if you 
> ever actually enter this code below. It seems it was rare in the past and 
> only happened due to a race with reading finalizerRun, which is now fixed. 
> You can also argue that the code above to trigger finalization is not needed. 
> We can just skip right to the loop. You really need FINALIZER_DONE either. 
> Making finalizerRun volatile should fix the race issue.

Hi @plummercj I did some test earlier shows that add volatile modifier to 
`finalizerRun` do not fix the race issue, maybe because finalize() executed by 
another low proirity Finalizer thread? So I add CountDownLatch finalizerDone 
variable try to wait finalize() triggered and finish.

I have change the comments.

The the finalizerRun has been removed and replaced as `finalizerDone.getCount()`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3115460101

Reply via email to