RFR: 8306467: Fix nsk/jdb/kill/kill001 to work with new JVMTI StopThread support for virtual threads. [v2]

[Alex Menkov]

On Mon, 15 May 2023 20:22:53 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>>> looks like we just need to move synchronized (or some other code to force vthread to be mounted) to inside the try and remove Thread.sleep, no other statements are needed.
>> 
>> That should work. At first I was unsure because the async exception is not actually thrown until executing the next statement after the synchronized. But there should be a branch at the end of the try block, and the exception should be thrown before it is executed. Hopefully the exception table covers that bytecode as being in the try block.
>
> This became a lot trickier to understand than I expected. The JVM does not have to throw the async exception at the next executed bytecode. The interpreter seems to do it at an invoke or after a goto (so the target of the goto is the instruction that will appear to have thrown the async exception). I think JIT'd code is similar, although maybe throws on a backwards branch and not after a forward branch. For this reason we need some sort of statement in place after the synchronized block to trigger the throwing of the exception before we leave the try block.
> 
> Another thing that messes up the test is that within the synchronized block, you can't have anything that will trigger the throw of the async exception. For example:
> 
>         try {
>             synchronized (kill001a.lock) {
>                 kill001a.log.display("entered synchronized");
>             }
>             kill001a.log.display("exited synchronized");
>         } catch (Throwable t) {
> 
> This messes up the test because now the exception will be thrown from within the synchronized block. This triggers a catch of the exception by an implicit catch clauses whose only purpose is to do the monitorexit and then rethrow the exception (which the explicit catch clause will then catch). The problem is that the test only expects one throw per thread, and now it gets 2. Although fixable on the debugger side, it is probably best to avoid.
> 
> I've pushed an update. Let me know if you think it is ok.

Looks good to me.
Please update "@summary" statement about "notKilled" (now "killed") field

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13967#discussion_r1194369025