Fix data race in GC_suspend/resume_thread

* include/private/pthread_support.h [GC_ENABLE_SUSPEND_THREAD
&& !GC_DARWIN_THREADS && !GC_OPENBSD_UTHREADS && !NACL]
(GC_Thread_Rep.suspended_ext): New field.
* include/private/pthread_support.h (SUSPENDED_EXT): Remove macro.
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (GC_suspend_handler_inner): Replace
me->flags & SUSPENDED_EXT with AO_load(&me->suspended_ext).
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (suspend_self_inner): Replace
me->flags & SUSPENDED_EXT with AO_load_acquire(&me->suspended_ext).
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (GC_suspend_thread): Replace
t->flags & SUSPENDED_EXT with t->suspended_ext; replace
t->flags|=SUSPENDED_EXT with AO_store_release(&t->suspended_ext, TRUE);
add comment.
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (GC_resume_thread): Replace
t->flags&=~SUSPENDED_EXT with AO_store(&t->suspended_ext, FALSE).
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (GC_is_thread_suspended,
GC_suspend_all, GC_start_world): Replace t->flags & SUSPENDED_EXT with
t->suspended_ext.
* pthread_support.c (GC_register_my_thread): Remove assertion that uses
SUSPENDED_EXT.

diff --git a/include/private/pthread_support.h b/include/private/pthread_support.h
index 4ab9c210c828b150faf3e22878b308fc13563594..9699995bded5e5a4bfd1f4535b855d7e9730c4de 100644 (file)
--- a/include/private/pthread_support.h
+++ b/include/private/pthread_support.h
@@ -64,6 +64,11 @@ typedef struct GC_Thread_Rep {
     /* and GC_suspend_handler_inner (which sets store_stop.stack_ptr).  */
     struct thread_stop_info stop_info;
 
+#   if defined(GC_ENABLE_SUSPEND_THREAD) && !defined(GC_DARWIN_THREADS) \
+        && !defined(GC_OPENBSD_UTHREADS) && !defined(NACL)
+      volatile AO_t suspended_ext;  /* Thread was suspended externally. */
+#   endif
+
     unsigned char flags;
 #       define FINISHED 1       /* Thread has exited.                   */
 #       define DETACHED 2       /* Thread is treated as detached.       */
@@ -74,7 +79,6 @@ typedef struct GC_Thread_Rep {
                                 /* it unregisters itself, since it      */
                                 /* may not return a GC pointer.         */
 #       define MAIN_THREAD 4    /* True for the original thread only.   */
-#       define SUSPENDED_EXT 8  /* Thread was suspended externally.     */
 #       define DISABLED_GC 0x10 /* Collections are disabled while the   */
                                 /* thread is exiting.                   */
 

diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 7d53234df1ab4c9ea8fe99f15e6cd1169644965a..0370cf314b8641d7da81406e13357254b4ed2148 100644 (file)
--- a/pthread_stop_world.c
+++ b/pthread_stop_world.c
@@ -291,7 +291,7 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
   me = GC_lookup_thread_async(self);
 
 # ifdef GC_ENABLE_SUSPEND_THREAD
-    if ((me -> flags & SUSPENDED_EXT) != 0) {
+    if (AO_load(&me->suspended_ext)) {
 #     ifdef SPARC
         me -> stop_info.stack_ptr = GC_save_regs_in_stack();
 #     else
@@ -440,7 +440,7 @@ STATIC void GC_restart_handler(int sig)
     static void *GC_CALLBACK suspend_self_inner(void *client_data) {
       GC_thread me = (GC_thread)client_data;
 
-      while ((me -> flags & SUSPENDED_EXT) != 0) {
+      while (AO_load_acquire(&me->suspended_ext)) {
         /* TODO: Use sigsuspend() instead. */
         GC_brief_async_signal_safe_sleep();
       }
@@ -454,12 +454,14 @@ STATIC void GC_restart_handler(int sig)
 
       LOCK();
       t = GC_lookup_thread((pthread_t)thread);
-      if (t == NULL || (t -> flags & SUSPENDED_EXT) != 0) {
+      if (t == NULL || t -> suspended_ext) {
         UNLOCK();
         return;
       }
 
-      t -> flags |= SUSPENDED_EXT;
+      /* Set the flag making the change visible to the signal handler.  */
+      AO_store_release(&t->suspended_ext, (AO_t)TRUE);
+
       if ((pthread_t)thread == pthread_self()) {
         UNLOCK();
         /* It is safe as "t" cannot become invalid here (no race with   */
@@ -514,21 +516,21 @@ STATIC void GC_restart_handler(int sig)
       LOCK();
       t = GC_lookup_thread((pthread_t)thread);
       if (t != NULL)
-        t -> flags &= ~SUSPENDED_EXT;
+        AO_store(&t->suspended_ext, (AO_t)FALSE);
       UNLOCK();
     }
 
     GC_API int GC_CALL GC_is_thread_suspended(GC_SUSPEND_THREAD_ID thread) {
       GC_thread t;
-      int flags = 0;
+      int is_suspended = 0;
       DCL_LOCK_STATE;
 
       LOCK();
       t = GC_lookup_thread((pthread_t)thread);
-      if (t != NULL)
-        flags = t -> flags;
+      if (t != NULL && t -> suspended_ext)
+        is_suspended = (int)TRUE;
       UNLOCK();
-      return (flags & SUSPENDED_EXT) != 0;
+      return is_suspended;
     }
 # endif /* GC_ENABLE_SUSPEND_THREAD */
 
@@ -663,9 +665,12 @@ STATIC int GC_suspend_all(void)
     for (i = 0; i < THREAD_TABLE_SZ; i++) {
       for (p = GC_threads[i]; p != 0; p = p -> next) {
         if (!THREAD_EQUAL(p -> id, self)) {
-            if ((p -> flags & (FINISHED | SUSPENDED_EXT)) != 0) continue;
+            if ((p -> flags & FINISHED) != 0) continue;
             if (p -> thread_blocked) /* Will wait */ continue;
 #           ifndef GC_OPENBSD_UTHREADS
+#             ifdef GC_ENABLE_SUSPEND_THREAD
+                if (p -> suspended_ext) continue;
+#             endif
               if (p -> stop_info.last_stop_count == GC_stop_count) continue;
               n_live_threads++;
 #           endif
@@ -1044,9 +1049,12 @@ GC_INNER void GC_start_world(void)
     for (i = 0; i < THREAD_TABLE_SZ; i++) {
       for (p = GC_threads[i]; p != 0; p = p -> next) {
         if (!THREAD_EQUAL(p -> id, self)) {
-            if ((p -> flags & (FINISHED | SUSPENDED_EXT)) != 0) continue;
+            if ((p -> flags & FINISHED) != 0) continue;
             if (p -> thread_blocked) continue;
 #           ifndef GC_OPENBSD_UTHREADS
+#             ifdef GC_ENABLE_SUSPEND_THREAD
+                if (p -> suspended_ext) continue;
+#             endif
               n_live_threads++;
 #           endif
 #           ifdef DEBUG_THREADS

diff --git a/pthread_support.c b/pthread_support.c
index c70ab7da7d8bf891794a974a3759ac24218cc6cb..2d68293e99a3cba9ba252c391c651345447eff10 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -1688,7 +1688,6 @@ GC_API int GC_CALL GC_register_my_thread(const struct GC_stack_base *sb)
     } else if ((me -> flags & FINISHED) != 0) {
         /* This code is executed when a thread is registered from the   */
         /* client thread key destructor.                                */
-        GC_ASSERT((me -> flags & SUSPENDED_EXT) == 0);
         GC_record_stack_base(me, sb);
         me -> flags &= ~FINISHED; /* but not DETACHED */
 #       ifdef GC_EXPLICIT_SIGNALS_UNBLOCK