]> granicus.if.org Git - gc/commitdiff
Fix lack of barriers to synchronize memory for suspend_handler
authorIvan Maidanski <ivmai@mail.ru>
Thu, 23 Nov 2017 23:21:46 +0000 (02:21 +0300)
committerIvan Maidanski <ivmai@mail.ru>
Mon, 18 Dec 2017 07:14:55 +0000 (10:14 +0300)
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.

pthread_stop_world.c

index e06215ecf969490ca07fbf208537207f9f6c85da..e8467344421e86000631aac921e88c3233587d12 100644 (file)
@@ -107,10 +107,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;
@@ -230,7 +230,9 @@ STATIC void GC_suspend_handler_inner(ptr_t sig_arg,
   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.            */
 
   if ((signed_word)sig_arg != GC_sig_suspend) {
 #   if defined(GC_FREEBSD_THREADS)
@@ -807,7 +809,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) {