On Mon, 24 Feb 2025 05:50:40 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this test-only change which proposes to address > an intermittent failure in the > `test/jdk/java/util/zip/ZipFile/TestCleaner.java` test? > > This test does operations on Inflater/Deflater/ZipFile and closes those > instances and then waits for GC to kick in. After wait for a second, it then > checks that the underlying resources held by these instances have been > cleaned (by the Cleaner). > > Once in a while, we have noticed that this test fails because the resources > haven't been cleaned. I suspect this is because within the (fixed) 1 second > wait time, the Cleaner hasn't yet invoked the cleaning action for these > instances. > > The commit in this PR updates the test to run it in `othervm` so that the > Cleaner actions aren't delayed by any other test or code that might have > previously run on the `agentvm`. Furthermore, the test is also updated to the > use the `ForceGC` test util which takes into account the jtreg test timeout > factor for deciding how long to wait for the Cleaner to initiate the cleaning > action. Our CI is configured with a timeout factor of 4, so with this change, > instead of a fixed maximum 1 second wait time, the test will now wait a > maximum of 4 seconds for the cleaner action to be invoked. > > The test continues to pass with this change, even with a repeat execution of > 50 runs. Hi Jai, I think the changes look ok overall. Thank you for tackling this one. ------------- Marked as reviewed by lancea (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23742#pullrequestreview-2644615337