On Tue, 10 Sep 2024 19:23:16 GMT, Brent Christian <bchri...@openjdk.org> wrote:

>> From the bug description:
>> ForceGC would be improved by moving the Reference.reachabilityFence() calls 
>> for 'obj' and 'ref'.
>> 
>> Reference.reachabilityFence(obj) is currently placed after 'obj' has been 
>> set to null, so effectively does nothing. It should occur before obj = null;
>> 
>> For Reference.reachabilityFence(ref): 'ref' is a PhantomReference to 'obj', 
>> and is registered with 'queue'. ForceGC.waitFor() later remove()s the 
>> reference from the queue, as an indication that some GC and reference 
>> processing has taken place (hopefully causing the BooleanSupplier to return 
>> true).
>> 
>> The code expects the PhantomReference to be cleared and be put on the queue. 
>> But recall that a Reference refers to its queue, and not the other way 
>> around. If a Reference becomes unreachable and is garbage collected, it will 
>> never be enqueued.
>> 
>> I argue that the VM/GC could determine that 'ref' is not used by waitFor() 
>> and collect it before the call to queue.remove(). Moving 
>> Reference.reachabilityFence(ref) after the for() loop would prevent this 
>> scenario.
>> 
>> While this is only a very minor deficiency in ForceGC, I believe it would be 
>> good to ensure that the code behaves as expected.
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add try-finally block

Yeah, timeout stuff can be handled separately.

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

Marked as reviewed by smarks (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20898#pullrequestreview-2295462917

Reply via email to