[jdk19] RFR: 8287847: Fatal Error when suspending virtual thread after it has terminated

[Alan Bateman]

On Tue, 28 Jun 2022 22:33:07 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> This fixes a bug in the debug agent when there is a request to suspend a virtual thread that has already terminated. The issue was that unless the debug agent was currently under a "suspend all", it would not properly put the virtual thread on the `otherThreads` list, and instead added it to `runningVThreads`. This meant at the end of `insertThread()` the following code tried to do a JVMTI `SetThreadLocalStorage`, which can't be done on a terminated thread.
> 
> 
>         if (list != &otherThreads) {
>             setThreadLocalStorage(node->thread, (void*)node);
>         }

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 418:

> 416:               // Thread not alive so put on otherThreads list instead of runningVThreads.
> 417:               // It might not have started yet or might have terminated. Either way,
> 418:               // otherThreads is the place for it.

If a terminated thread is added to otherThreads then will be it eventually removed?

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 419:

> 417:               // It might not have started yet or might have terminated. Either way,
> 418:               // otherThreads is the place for it.
> 419:               list = &otherThreads;

Minor nit but the native code here uses 4-space indentation.

test/jdk/com/sun/jdi/SuspendAfterDeath.java line 62:

> 60: public class SuspendAfterDeath extends TestScaffold {
> 61:     private volatile ThreadReference thread;
> 62:     private volatile boolean breakpointReached = false;

The volatile-write to initialise this to false is not needed here.

test/jdk/com/sun/jdi/SuspendAfterDeath.java line 99:

> 97:             List argList = new ArrayList(Arrays.asList(args));
> 98:             argList.add("Virtual");
> 99:             args = (String[]) argList.toArray(args);

The raw type and casting caught my addition here. Here's something more succulent if you'd like:

args = Stream.concat(Stream.of(args), Stream.of("Virtual"))
                .toArray(String[]::new);

test/jdk/com/sun/jdi/SuspendAfterDeath.java line 135:

> 133:         }
> 134:     }
> 135: }

I wonder if we need to add `@run main/othervm -Dmain.wrapper=Virtual SuspendAfterDeath` to the test description so that running the jdk_jdi test group will exercise the issue. As it stands, I think it would require a test run with -Dmain.wrapper=Virtual to exercise this code.

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

PR: https://git.openjdk.org/jdk19/pull/88