From: Ivan Maidanski <ivmai@mail.ru>
Date: Fri, 4 Oct 2019 17:42:54 +0000 (+0300)
Subject: Check thread is alive on each SuspendThread loop iteration (Win32)
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=872faf2abb6ca65a7f8c0c305ae5bb1b17ec1bd6;p=gc

Check thread is alive on each SuspendThread loop iteration (Win32)
(fix of commit 449eda034)

* win32_threads.c [!GC_NO_THREADS_DISCOVERY]
(GC_delete_gc_thread_no_free): Reset suspended if GC_win32_dll_threads.
* win32_threads.c [!MSWINCE] (GC_suspend): Move comment near exitCode
declaration to be near GetExitCodeThread() call; adjust comment.
* win32_threads.c [!MSWINCE && RETRY_GET_THREAD_CONTEXT] (GC_suspend):
Call GetExitCodeThread() on each iteration of the suspend loop; refine
ABORT message on ResumeThread failure.
---

diff --git a/win32_threads.c b/win32_threads.c
index 7dfeeb8f..88e54a5b 100644
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -694,6 +694,7 @@ STATIC void GC_delete_gc_thread_no_free(GC_vthread t)
       /* see GC_stop_world() for the information.                       */
       t -> stack_base = 0;
       t -> id = 0;
+      t -> suspended = FALSE;
 #     ifdef RETRY_GET_THREAD_CONTEXT
         t -> context_sp = NULL;
 #     endif
@@ -1266,11 +1267,6 @@ void GC_push_thread_structures(void)
 STATIC void GC_suspend(GC_thread t)
 {
 # ifndef MSWINCE
-    /* Apparently the Windows 95 GetOpenFileName call creates           */
-    /* a thread that does not properly get cleaned up, and              */
-    /* SuspendThread on its descriptor may provoke a crash.             */
-    /* This reduces the probability of that event, though it still      */
-    /* appears there's a race here.                                     */
     DWORD exitCode;
 # endif
 # ifdef RETRY_GET_THREAD_CONTEXT
@@ -1283,21 +1279,6 @@ STATIC void GC_suspend(GC_thread t)
 # endif
   UNPROTECT_THREAD(t);
   GC_acquire_dirty_lock();
-# ifndef MSWINCE
-    if (GetExitCodeThread(t -> handle, &exitCode) &&
-        exitCode != STILL_ACTIVE) {
-      GC_release_dirty_lock();
-#     ifdef GC_PTHREADS
-        t -> stack_base = 0; /* prevent stack from being pushed */
-#     else
-        /* this breaks pthread_join on Cygwin, which is guaranteed to  */
-        /* only see user pthreads                                      */
-        GC_ASSERT(GC_win32_dll_threads);
-        GC_delete_gc_thread_no_free(t);
-#     endif
-      return;
-    }
-# endif
 
 # ifdef MSWINCE
     /* SuspendThread() will fail if thread is running kernel code.      */
@@ -1308,6 +1289,25 @@ STATIC void GC_suspend(GC_thread t)
     }
 # elif defined(RETRY_GET_THREAD_CONTEXT)
     for (;;) {
+      /* Apparently the Windows 95 GetOpenFileName call creates         */
+      /* a thread that does not properly get cleaned up, and            */
+      /* SuspendThread on its descriptor may provoke a crash.           */
+      /* This reduces the probability of that event, though it still    */
+      /* appears there is a race here.                                  */
+      if (GetExitCodeThread(t -> handle, &exitCode)
+          && exitCode != STILL_ACTIVE) {
+        GC_release_dirty_lock();
+#       ifdef GC_PTHREADS
+          t -> stack_base = 0; /* prevent stack from being pushed */
+#       else
+          /* This breaks pthread_join on Cygwin, which is guaranteed to */
+          /* only see user threads.                                     */
+          GC_ASSERT(GC_win32_dll_threads);
+          GC_delete_gc_thread_no_free(t);
+#       endif
+        return;
+      }
+
       if (SuspendThread(t->handle) != (DWORD)-1) {
         CONTEXT context;
 
@@ -1321,7 +1321,7 @@ STATIC void GC_suspend(GC_thread t)
 
         /* Resume the thread, try to suspend it in a better location.   */
         if (ResumeThread(t->handle) == (DWORD)-1)
-          ABORT("ResumeThread failed");
+          ABORT("ResumeThread failed in suspend loop");
       }
       if (retry_cnt > 1) {
         GC_release_dirty_lock();
@@ -1332,6 +1332,17 @@ STATIC void GC_suspend(GC_thread t)
         ABORT("SuspendThread loop failed"); /* something must be wrong */
     }
 # else
+    if (GetExitCodeThread(t -> handle, &exitCode)
+        && exitCode != STILL_ACTIVE) {
+      GC_release_dirty_lock();
+#     ifdef GC_PTHREADS
+        t -> stack_base = 0; /* prevent stack from being pushed */
+#     else
+        GC_ASSERT(GC_win32_dll_threads);
+        GC_delete_gc_thread_no_free(t);
+#     endif
+      return;
+    }
     if (SuspendThread(t -> handle) == (DWORD)-1)
       ABORT("SuspendThread failed");
 # endif