On Wed, 20 May 2026 14:14:44 GMT, Jorn Vernee <[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). Looks reasonable, thanks for following up. I don't quite understand the failure mode the test was supposed to catch, but here are some generic questions/comments: test/jdk/java/foreign/TestUpcallStress.java line 70: > 68: @BeforeClass > 69: public void setup() { > 70: executor = Executors.newFixedThreadPool(THREAD_COUNT); Do you really want 100 threads on all systems? Or do you want `NCPU` threads, but at least 100 tasks? 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"); } } ------------- PR Review: https://git.openjdk.org/jdk/pull/31216#pullrequestreview-4329718287 PR Review Comment: https://git.openjdk.org/jdk/pull/31216#discussion_r3275018644 PR Review Comment: https://git.openjdk.org/jdk/pull/31216#discussion_r3275002585
