On Fri, 28 Mar 2025 01:08:42 GMT, Brent Christian <bchri...@openjdk.org> wrote:

>> I propose some cleanups to `FinalizerHistogramTest.java` to hopefully clear 
>> up the intermittent failures:
>> 
>> * run with `othervm`: this test blocks the (global) finalizer thread, and 
>> also requires the (global) finalizer thread to enter the test's `finalize()` 
>> method
>> * The test uses `volatile` ints, but sets them based on their current value, 
>> which is not reliable; convert to `AtomicInteger`
>> * use `PhantomReference`s to ensure that at least two `MyObject`s have 
>> become unreachable. If one is stuck in `finalize()`, at least one is still 
>> waiting to be finalized and should show up in the histogram.
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   convert to WeakRefs, use a RefQ, print ForceGC results

Thank you Brent for the updates. Good catch on the `PhantomReference` usage in 
the test. The latest changes look good to me.

test/jdk/java/lang/ref/FinalizerHistogramTest.java line 131:

> 129:         volatile boolean ref1Cleared = false;
> 130:         volatile boolean ref2Cleared = false;
> 131:         Thread thread;

Nit: This need not be a field in the class and instead can be local to the 
constructor where the Thread is started. But it's OK to leave it like this if 
you prefer to.

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

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24143#pullrequestreview-2724847209
PR Review Comment: https://git.openjdk.org/jdk/pull/24143#discussion_r2018243524

Reply via email to