RFR: 8298783: java/lang/ref/FinalizerHistogramTest.java failed with "RuntimeException: MyObject is not found in test output" [v3]

[Jaikiran Pai]

On Fri, 21 Mar 2025 18:50:40 GMT, Brent Christian <bchristi at 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:
> 
>   'return' not needed in lambda

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

> 73:                                ref2.refersTo(null) &&
> 74:                                trappedCount.intValue() > 0
> 75:             );

Hello Brent, I intially thought this still had the chance of intermittently timing out waiting for this condition to be satisfied, because I think there's no guarantee that the `MyObject` instances referred to by `ref1` and `ref2` would be the "prioritized" ones (compared to the others that we create in a loop) that would be eligible for GC. But then I noticed that we don't make use of the return value from the `ForceGC.wait()` call and I think that's a good thing.

Having said that, I wonder if this change would now lead to more frequent intermittent failures because unlike previously where we used to virtually wait "forever" in the `while(wasTrapped < 1);`, with this change we now wait for a maximum of "1 second * TIMEOUT_FACTOR". I suspect we need to add some additional logic here which force waits for at least one `MyObject.finalize()` to be invoked (i.e. `trappedCount` is at least 1) before proceeding to do anything else.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24143#discussion_r2014355448