From 62ad477c29f603e5e2eb46e01512d7e0a0d5ae9a Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 24 Nov 2017 02:21:46 +0300 Subject: [PATCH] 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. --- pthread_stop_world.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index e06215ec..e8467344 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -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) { -- 2.50.1