On Fri, 21 Mar 2025 18:50:40 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: > > 'return' not needed in lambda The `FinalizerHistogramTest` tests the `java.lang.ref.FinalizerHistogram` class. To do that it sets up a situation where it generates several `MyObject` instances and allows (hopes) that they are GCed. In the `finalize()` method of the `MyObject` class a forever blocking operation is intentionally done, so that the `java.lang.ref.Finalizer` class which is responsible for calling the `finalize()` method on this class becomes blocked when it calls `MyObject.finalize()` on one such `MyObject` instance. That way, the internal `queue` that the `Finalizer` class maintains accumulates additional objects whose `finalize()` needs to be called. The test then prints out the contents of this internal queue and expects that there be at least one more `MyObject` instance in this queue awaiting their `finalize()` to be invoked. Most of the times this all works fine and the test sees the `MyObject` instance(s) in that queue and passes. In the cases where this intermittently fails, what seems to be happening is that, out of the numerous `MyObject` instances that the test hopes will be GCed, just one of them gets GCed and the `Finalizer` calls that instance's `finalize()` method. The test notices that the `finalize()` has been invoked and immediately goes on to print out the contents of the internal `queue` of the `Finalizer` expecting to see more `MyObject` instances in it. It doesn't find any and ends up failing. As noted, this is a test bug because in its current form, there's no guarantee that the internal `queue` would have (at that moment) any more instances queued up when the first GCed `MyObject.finalize()` method invoked. The proposed change here updates the test in such a way that we wait for at least 2 instances of `MyObject` to be picked up for GC (and thus end up in that internal `queue`) and the `MyObject.finalize()` of the first instance blocks forever, thus providing a guarantee that when the test access that queue, it will find at least the second instance of `MyObject` in it. I think the overall idea/change to this test looks good to me. I do have a comment about the implementation detail of this change which I've added inline. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24143#issuecomment-2754648351
