On Tue, 21 Apr 2026 06:34:27 GMT, SendaoYan <[email protected]> wrote:
>> Hi all, >> >> Test com/sun/jdi/FinalizerTest.java intermittent fails. This is because >> arraylist holdAlot do not clear or set as null before recliam heap memory by >> invoke System.gc. This PR add arraylist.clear() before call System.gc() to >> avoid OOME in the senond System.runFinalization(). >> >> The first System.runFinalization() has a synchronize bug. >> System.runFinalization() run in another low priority Finalizer thread. In >> the main thread maybe read variable finalizerRun before Finalizer thread >> change it. So waitForAFinalizer will run into whille(true) allocation loop >> sometimes. This PR add a countdownlatch synchronize to make sure main thread >> read the finalizerRun value after Finalizer thread write it. >> >> This PR also remove the unused variable and optimize the imports, and add >> -Xmx256M to debuggee jvm process, this will make debuggee throw OOME steady >> in large physical memory machine. >> >> Before this PR, test failure probability about 1/30. After this PR test run >> 3k times and all passed. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > 1. Fix the comments about the senond attemt trigger finalize(); 2. Replaced > finalizerRun as FINALIZER_DONE.getCount() test/jdk/com/sun/jdi/FinalizerTest.java line 83: > 81: try { > 82: // finalize() run in another lower priority Finalizer thread, > 83: // wait for it to finish Suggestion: // finalize() is run in another lower priority Finalizer thread. // Wait for it to finish. test/jdk/com/sun/jdi/FinalizerTest.java line 84: > 82: // finalize() run in another lower priority Finalizer thread, > 83: // wait for it to finish > 84: finalizerDone.await(5, TimeUnit.SECONDS); Have you tested to make sure there are cases where this does time out and we enter the code below? it might be worth doing some testing with the gc() and runFinalization() code above disabled to make sure the code below is working properly. test/jdk/com/sun/jdi/FinalizerTest.java line 91: > 89: // If System.gc() and System.runFinalization() did not trigger the > 90: // finalizer, then finalizerDone.await() will timed out and the > code > 91: // below will be needed as second attempt to trigger finalization. Suggestion: // finalizer, then finalizerDone.await() will time out and the code // below will be needed as a second attempt to trigger finalization. test/jdk/com/sun/jdi/FinalizerTest.java line 98: > 96: } > 97: try { > 98: while(finalizerDone.getCount() > 0) { Suggestion: while (finalizerDone.getCount() > 0) { test/jdk/com/sun/jdi/FinalizerTest.java line 103: > 101: } > 102: } > 103: catch ( Throwable thrown ) { // OutOfMemoryError Suggestion: } catch ( Throwable thrown ) { // OutOfMemoryError test/jdk/com/sun/jdi/FinalizerTest.java line 110: > 108: System.runFinalization(); > 109: } > 110: return; If I'm understanding the logic here properly, if for some reason waitForAFinalizer() never triiggers finalization, it will eventually exit here, and the only real sign that something went wrong is the debugger side of the test timing out because it does not get a breakpoint event in the finalizer. Maybe we should at least have a println here to indicate that the finalizer was never triggered. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139511019 PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139635058 PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139515567 PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139529607 PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139530969 PR Review Comment: https://git.openjdk.org/jdk/pull/30829#discussion_r3139625084
