On Sat, 7 Sep 2024 00:39:16 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 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. The reason for the explicit reference and RF, as I recall, is to guard against the allocation of the new object being elided entirely, with the `PhantomReference` constructor being passed null (or itself being elided) and no reference processing ever actually happening. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20898#discussion_r1749562854
