From: Ivan Maidanski <ivmai@mail.ru>
Date: Thu, 23 Nov 2017 23:21:46 +0000 (+0300)
Subject: Fix lack of barriers to synchronize memory for suspend_handler
X-Git-Tag: v8.0.0~491
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=28a64c87b66d8c08da0b1c62ded59e59ccf3f5f5;p=gc

Fix lack of barriers to synchronize memory for suspend_handler

pthread_kill is not on the list of functions which synchronize memory.

* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_world_is_stopped): Reformat comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Use AO_load_acquire instead of AO_load
to fetch the value of GC_stop_count; add comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_start_world): Use AO_store_release instead of AO_store to reset
GC_world_is_stopped; add comment.
---

diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 51faa1c7..81e50b33 100644
--- a/pthread_stop_world.c
+++ b/pthread_stop_world.c
@@ -113,10 +113,10 @@ STATIC volatile AO_t GC_stop_count = 0;
                         /* Incremented at the beginning of GC_stop_world. */
 
 STATIC volatile AO_t GC_world_is_stopped = FALSE;
-                        /* FALSE ==> it is safe for threads to restart, i.e. */
-                        /* they will see another suspend signal before they  */
-                        /* are expected to stop (unless they have voluntarily */
-                        /* stopped).                                         */
+                        /* FALSE ==> it is safe for threads to restart, */
+                        /* i.e. they will see another suspend signal    */
+                        /* before they are expected to stop (unless     */
+                        /* they have stopped voluntarily).              */
 
 #ifdef GC_OSF1_THREADS
   STATIC GC_bool GC_retry_signals = TRUE;
@@ -287,7 +287,9 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
   pthread_t self = pthread_self();
   GC_thread me;
   IF_CANCEL(int cancel_state;)
-  AO_t my_stop_count = AO_load(&GC_stop_count);
+  AO_t my_stop_count = AO_load_acquire(&GC_stop_count);
+                        /* After the barrier, this thread should see    */
+                        /* the actual content of GC_threads.            */
 
   DISABLE_CANCEL(cancel_state);
       /* pthread_setcancelstate is not defined to be async-signal-safe. */
@@ -1045,7 +1047,10 @@ GC_INNER void GC_start_world(void)
 #   endif
 
 #   ifndef GC_OPENBSD_UTHREADS
-      AO_store(&GC_world_is_stopped, FALSE);
+      AO_store_release(&GC_world_is_stopped, FALSE);
+                    /* The updated value should now be visible to the   */
+                    /* signal handler (note that pthread_kill is not on */
+                    /* the list of functions which synchronize memory). */
 #   endif
     for (i = 0; i < THREAD_TABLE_SZ; i++) {
       for (p = GC_threads[i]; p != 0; p = p -> next) {