On Wed, 20 May 2026 15:04:33 GMT, Aleksey Shipilev <[email protected]> wrote:
>> We were already seeing some issues on mac when the test was added, and it >> was therefor disabled on those platforms. Now we've had reports that this >> test is causing OOMs on other systems as well. >> >> Rather than outright removing the test, I've detuned it to create less >> thread/memory churn, more in line with other tests. >> >> Since The test has been detuned, it no longer needs to be in it's own test >> group, and can safely run as part of the regular `jdk_foreign` tests. I've >> changed the jtreg test groups to reflect this. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > test/jdk/java/foreign/TestUpcallStress.java line 77: > >> 75: executor.shutdown(); >> 76: // Let it run for a while, and then just terminate >> 77: executor.awaitTermination(Utils.adjustTimeout(30), >> TimeUnit.SECONDS); > > I don't think this guarantees the threads would not linger? We are currently > protected by `othervm` and `@AfterClass` for it, so it might not be a > practical concern. But maybe you want: > > > executor.shutdown(); > if (!executor.awaitTermination(30 seconds)) { > executor.shutdownNow(); > if (!executor.awaitTermination(30 seconds)) { > throw new IllegalStateException("Threads did not terminate"); > } > } I wanted to avoid putting in a time out that makes the test fail. The idea is that we run this test for a while, and then just shut down the process if it doesn't finish in that time window. This relies on `othervm` as you've noticed. `shutdownNow` will also not actually terminate running threads, it will just discard any waiting tasks, so we still don't get the guarantee that threads won't linger in that case. (we have the process shutdown for that). The `shutdown` + `awaitTermination` is intended to let the test exit early if we run out of tasks before the deadline (which I found to be the case on my machine). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31216#discussion_r3276226208
