On Mon, 30 Oct 2023 18:29:30 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> 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 r eport. 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 thre... > > @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? @lmesnik Using `System.exit` is just wrong - we don't use that in JTREG tests at all. I would suggest just ProblemListing problematic tests that won't work as expected with virtual threads, until jtreg addresses this. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16369#issuecomment-1786171041
