]> granicus.if.org Git - gc/commitdiff
Fix data race in last_stop_count access (suspend_handler_inner)
authorIvan Maidanski <ivmai@mail.ru>
Thu, 23 Nov 2017 17:15:54 +0000 (20:15 +0300)
committerIvan Maidanski <ivmai@mail.ru>
Thu, 23 Nov 2017 17:15:54 +0000 (20:15 +0300)
* include/private/pthread_stop_world.h [!GC_OPENBSD_UTHREADS]
(thread_stop_info.last_stop_count): Do not define if NACL; change the
type from word to AO_t; add volatile qualifier; fix a typo in comment
("GC_stop_count").
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(update_last_stop_count): Remove.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Replace update_last_stop_count() call with
AO_store_release(&me->stop_info.last_stop_count,my_stop_count).
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_all): Replace p->stop_info.last_stop_count with
AO_load(&p->stop_info.last_stop_count).

include/private/pthread_stop_world.h
pthread_stop_world.c

index 49b87bc607e15a523bb93ce9757386c4790a6df9..ec5dfea622e9a600a3065d70b121b16cb7e09999 100644 (file)
 #define GC_PTHREAD_STOP_WORLD_H
 
 struct thread_stop_info {
-#   ifndef GC_OPENBSD_UTHREADS
-      word last_stop_count;     /* GC_last_stop_count value when thread */
-                                /* last successfully handled a suspend  */
-                                /* signal.                              */
+#   if !defined(GC_OPENBSD_UTHREADS) && !defined(NACL)
+      volatile AO_t last_stop_count;
+                        /* The value of GC_stop_count when the thread   */
+                        /* last successfully handled a suspend signal.  */
 #   endif
 
     ptr_t stack_ptr;            /* Valid only when stopped.             */
index 0370cf314b8641d7da81406e13357254b4ed2148..dde9de24c8ac2b855b2967a1df1afb6d0a0398e7 100644 (file)
@@ -261,12 +261,6 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy, void *context);
 # define GC_lookup_thread_async GC_lookup_thread
 #endif
 
-GC_ATTR_NO_SANITIZE_THREAD
-static void update_last_stop_count(GC_thread me, AO_t my_stop_count)
-{
-  me -> stop_info.last_stop_count = my_stop_count;
-}
-
 STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
                                      void * context GC_ATTR_UNUSED)
 {
@@ -341,7 +335,7 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
   /* thread has been stopped.  Note that sem_post() is          */
   /* the only async-signal-safe primitive in LinuxThreads.      */
   sem_post(&GC_suspend_ack_sem);
-  update_last_stop_count(me, my_stop_count);
+  AO_store_release(&me->stop_info.last_stop_count, my_stop_count);
 
   /* Wait until that thread tells us to restart by sending      */
   /* this thread a GC_sig_thr_restart signal (should be masked  */
@@ -671,7 +665,8 @@ STATIC int GC_suspend_all(void)
 #             ifdef GC_ENABLE_SUSPEND_THREAD
                 if (p -> suspended_ext) continue;
 #             endif
-              if (p -> stop_info.last_stop_count == GC_stop_count) continue;
+              if (AO_load(&p->stop_info.last_stop_count) == GC_stop_count)
+                continue;
               n_live_threads++;
 #           endif
 #           ifdef DEBUG_THREADS