8269268: JDWP: Properly fix thread lookup assert in findThread()

Reviewed-by: kevinw, amenkov, sspitsyn

Author: plummercj

src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c

@@ -1230,6 +1230,10 @@ cbVMDeath(jvmtiEnv *jvmti_env, JNIEnv *env)
     EventInfo info;
     LOG_CB(("cbVMDeath"));
 
+    /* Setting this flag is needed by findThread(). It's ok to set it before
+       the callbacks are cleared.*/
+    gdata->jvmtiCallBacksCleared = JNI_TRUE;
+
     /* Clear out ALL callbacks at this time, we don't want any more. */
     /*    This should prevent any new BEGIN_CALLBACK() calls. */
     (void)memset(&(gdata->callbacks),0,sizeof(gdata->callbacks));

src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c

@@ -254,12 +254,36 @@ findThread(ThreadList *list, jthread thread)
             node = nonTlsSearch(getEnv(), &otherThreads, thread);
         }
         /*
-         * Search runningThreads list. The TLS lookup may have failed because the
-         * thread has terminated, but the ThreadNode may still be present.
+         * Normally we can assume that a thread with no TLS will never be in the runningThreads
+         * list. This is because we always set the TLS when adding to runningThreads.
+         * However, when a thread exits, its TLS is automatically cleared. Normally this
+         * is not a problem because the debug agent will first get a THREAD_END event,
+         * and that will cause the thread to be removed from runningThreads, thus we
+         * avoid this situation of having a thread in runningThreads, but with no TLS.
+         *
+         * However... there is one exception to this. While handling VM_DEATH, the first thing
+         * the debug agent does is clear all the callbacks. This means we will no longer
+         * get THREAD_END events as threads exit. This means we might find threads on
+         * runningThreads with no TLS during VM_DEATH. Essentially the THREAD_END that
+         * would normally have resulted in removing the thread from runningThreads is
+         * missed, so the thread remains on runningThreads.
+         *
+         * The end result of all this is that if the TLS lookup failed, we still need to check
+         * if the thread is on runningThreads, but only if JVMTI callbacks have been cleared.
+         * Otherwise the thread should not be on the runningThreads.
          */
-        if ( node == NULL ) {
-            if ( list == NULL || list == &runningThreads ) {
-                node = nonTlsSearch(getEnv(), &runningThreads, thread);
+        if ( !gdata->jvmtiCallBacksCleared ) {
+            /* The thread better not be on runningThreads if the TLS lookup failed. */
+            JDI_ASSERT(!nonTlsSearch(getEnv(), &runningThreads, thread));
+        } else {
+            /*
+             * Search the runningThreads list. The TLS lookup may have failed because the
+             * thread has terminated, but we never got the THREAD_END event.
+             */
+            if ( node == NULL ) {
+                if ( list == NULL || list == &runningThreads ) {
+                    node = nonTlsSearch(getEnv(), &runningThreads, thread);
+                }
             }
         }
     }

src/jdk.jdwp.agent/share/native/libjdwp/util.h

@@ -133,8 +133,11 @@ typedef struct {
     int           objectsByIDsize;
     int           objectsByIDcount;
 
-     /* Indication that the agent has been loaded */
-     jboolean isLoaded;
+    /* Indication that the agent has been loaded */
+    jboolean isLoaded;
+
+    /* Indication that VM_DEATH has been recieved and the JVMTI callbacks have been cleared. */
+    volatile jboolean jvmtiCallBacksCleared;
 
 } BackendGlobalData;