On Sat, 7 Sep 2024 00:40:24 GMT, Stuart Marks <sma...@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.
>
> test/lib/jdk/test/lib/util/ForceGC.java line 102:
> 
>> 100:             }
>> 101:         }
>> 102:         Reference.reachabilityFence(ref);
> 
> I think everything from the creation of ref to the line above needs to 
> enclosed in a try-statement, with the finally-clause including RF(ref).

Arguably the same might also apply to the other call to reachability fence: 
that is - we might need two try-finally to keep things by-the-book?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20898#discussion_r1750398105

Reply via email to