On Wed, 25 Oct 2023 21:08:01 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
> The jtreg starts the main thread in a separate ThreadGroup and checks > unhandled exceptions for this group. However, it doesn't catch all unhandled > exceptions. There is a jtreg issue for this > https://bugs.openjdk.org/browse/CODETOOLS-7903526. > Catching such issues for virtual threads is important because they are not > included in any groups. So this fix implements the handler for the test > thread factory. > > A few tests start failing. > > The test > serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorVMEventsTest.java > has testcases for platform and virtual threads. So, there is there's no need > to run it with the thread factory. > > The test > java/lang/Thread/virtual/ThreadAPI.java > tests UncaughtExceptionHandler and virtual threads. No need to run it with a > thread factory. > > Test > test/jdk/java/util/concurrent/tck/ThreadTest.java is updated to not check the > default empty handler. > > Probably, we need some common approach about dealing with the > UncaughtExceptionHandler in jtreg. Hello Leonid, looking at the changes in this PR, what's being proposed is that when jtreg lauches tests through a virtual thread, then this wrapping code will set a JVM level `UncaughtExceptionHandler` by calling `Thread.setDefaultUncaughtExceptionHandler(...)`. The implementation of this `UncaughtExceptionHandler` calls `System.exit(1)`. Wouldn't that kill the test VM? I think that would then impact everything else including jtreg report generation and such for the test, isn't it? I had a look at https://bugs.openjdk.org/browse/JDK-8318839 but it doesn't have enough details to help understand what currently happens when a test launched through a virtual thread from jtreg throws an uncaught exception. How/what gets reported for that test execution? test/jdk/java/util/concurrent/tck/ThreadTest.java line 79: > 77: */ > 78: public void testGetAndSetDefaultUncaughtExceptionHandler() { > 79: assertNull(Thread.getDefaultUncaughtExceptionHandler()); I think this is a very broad change in this test case and either shouldn't be done or should be done conditionally (I can't think of the right condition now because I haven't fully grasped the context of this PR). test/jtreg_test_thread_factory/src/share/classes/Virtual.java line 37: > 35: // The virtual threads don't belong to any group and need > global handler. > 36: Thread.setDefaultUncaughtExceptionHandler((t, e) -> { > 37: if (e instanceof ThreadDeath) { `ThreadDeath` has been deprecated for removal since Java 20, so this should no longer be needed. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16369#issuecomment-1780470987 PR Review Comment: https://git.openjdk.org/jdk/pull/16369#discussion_r1372631138 PR Review Comment: https://git.openjdk.org/jdk/pull/16369#discussion_r1372630017
