On Fri, 6 Sep 2024 19:57:41 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.
test/lib/jdk/test/lib/util/ForceGC.java line 82:
> 80: PhantomReference<Object> ref = new PhantomReference<>(obj, queue);
> 81: Reference.reachabilityFence(obj);
> 82: obj = null;
You're right to question the utility of calling reachabilityFence(obj) after
obj has been nulled out. But I'm still questioning the utility of calling
RF(obj) at all. We don't care when obj is determined to be unreachable; what we
care about is that the GC has done some reference processing. Seems to me we
can simplify the above lines to
PhantomReference<Object> ref = new PhantomReference<>(new Object(), queue);
and get rid of the local variable obj entirely.
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).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20898#discussion_r1747839937
PR Review Comment: https://git.openjdk.org/jdk/pull/20898#discussion_r1747840429
