RFR: 8318839: Update test thread factory to catch all exceptions

[Leonid Mesnik]

On Sun, 29 Oct 2023 14:10:32 GMT, Jaikiran Pai <jpai at 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, in order to understand what exactly we are trying to solve here, I ran a few tests to see how things work without the changes being proposed in this PR. Here's my findings.
> 
> A bit of background first. When jtreg runs, either in agent mode or othervm mode, it creates a specific thread within which the actual test code runs. In either of these modes, it uses a custom jtreg specific `ThreadGroup` instance for this thread which is running the test code. This instance of `ThreadGroup` has specific overridden implementation of the `public void uncaughtException(Thread t, Throwable e)` API which keeps track of uncaught exception that might have been thrown by any threads that could have been spawned by the test. After `start()`ing the thread which runs the test code, the jtreg framework then waits for that thread to complete and once completed (either exceptionally or normally), jtreg framework then queries a state on the custom `ThreadGroup` instance to see if any uncaught exception(s) were received during the lifetime of this thread which ran that test. If it finds any, then it marks the test as failed and reports such a failure appropriately in the test re
 port. As noted, this applies for both the agent mode and other vm mode. Some important aspects of this implementation is that:
> 
> - The custom `ThreadGroup` which has the overridden implementation of the `uncaughtException(Thread t, Throwable e)` method doesn't interfere with the JVM level default exception handler.
> 
> - After the thread which ran the test code completes, the decision on whether to fail or pass a test is taken by checking the custom `ThreadGroup`'s state. Once this decision is done, the decision is immediately reported in relevant ways and the test status is marked (i.e. finalized) at this point.
> 
> - If this was running in agent vm mode, the agent vm mode continues to operate and isn't terminated and thus is available for subsequent tests. This point I think is important to remember for reasons that will be noted later in this comment.
> 
> Now coming to the part where in https://bugs.openjdk.org/browse/JDK-8303703 we introduced a way where jtreg instead of creating a platform thread (backed by its custom implementation of the `ThreadGroup`) to run the test code, now checks the presence of a `ThreadFactory` and if present, lets it create a `Thread` which runs this test code. In the JDK repo, we have an implementation of a `ThreadFactory` which creates a virtual thread instead of platform...

@jaikiran, your analysis is correct. 
@jaikiran , @dholmes-ora  The jtreg is going to be fixed to handle all uncaught exceptions. See PR: https://github.com/openjdk/jtreg/pull/172
The problem might happens not only with test thread factory, but for any virtual threads and might be for system threads.  It is more generic solution and might take a lot of time to be correctly implemented. So this pr is just temporary fix until jtreg is updated.  I could withdraw this PR, but not sure what are the risks/issues if I integrate it. We are going just to have a ugly error reporting for the uncaught threads when test thread factory is used  or missed something?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16369#issuecomment-1785813862