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 If you hide whitespace this change becomes trivial to review :) LGTM. ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20898#pullrequestreview-2294914217
