On Wed, 10 May 2023 23:22:03 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> In TestScaffold.java we check the debuggee process exitValue, and allow a 0 > or a 1. Otherwise the test fails. You get exitValue 1 when the debuggee exits > with an exception. Allowing this was necessary because some tests purposely > make the debuggee exit with an exception. However, this runs the risk of not > detecting a debuggee exception when none was expected. > > [JDK-8306758](https://bugs.openjdk.org/browse/JDK-8306758) added support for > allowing the test to determine which exitValues are acceptable. This means we > can now change TestScaffold to by default only allow exitValue 0. Tests that > expect an exception can override `TestScaffold.allowedExitValue(int)` to > allow (or only expect) an exitValue of 1. There are 2 test that will need > updating once TestScaffold by default no longer allows exitValue 1: > > - ResumeOneThread.java has a latent virtual thread bug that this PR is > exposing. In order to fix > [JDK-8283796](https://bugs.openjdk.org/browse/JDK-8283796), the debuggee is > doing a Thread.setDaemon(false) on the threads it creates. This results in an > exception when used on a virtual thread, causing the debuggee to exit with > exitValue 1, which previously went undetected. The changes for the CR no > longer allow exitValue 1, exposing this bug. The proper fix for > [JDK-8283796](https://bugs.openjdk.org/browse/JDK-8283796) should have been > to instead Thread.join() on the created threads. > > - ExceptionEvents.java has some test modes that expect the debuggee to exit > with an exception, so `allowedExitValue()` is overridden to expect exitValue > 1 instead of 0 for these test modes. > > There is also one minor fix needed in TestScaffold when using virtual > threads. The virtual thread factory related code includes a catch clause to > catch all exceptions. It does this in the code that invokes the virtual > thread main method. If any exception is caught, it is saved away. I think the > intent was to rethrow it in the main thread just like it would have been > thrown if not using the virtual thread factory, allowing the debuggee to exit > with the exception (and exitValue 1). However, the code wasn't rethrowing it, > so if an exception was thrown by the debuggee virtual thread, the test still > exited with exitValue 0. This causes unexpected debuggee exceptions to go > unnoticed, and also causes tests that expect a debuggee exception to fail. I > tried to fix this by rethrowing the exception, but this causes some tests > that track ExceptionEvents to complain about an unexpected exception. So I... Looks good. Trivia: tg.uncaughtThrowable.printStackTrace(System.out); It may not matter and may be intentionally on System.out here, but an uncaught exception in main thread would normally go on System.err. ------------- Marked as reviewed by kevinw (Committer). PR Review: https://git.openjdk.org/jdk/pull/13919#pullrequestreview-1422343550
