From ba0616b09bd3354ea9f27ec1ffc1480adbab78b8 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Sat, 2 Apr 2022 14:36:29 +0300 Subject: [PATCH] Avoid hang in usleep during signals resend in child process if TSan Issue #236 (bdwgc). If a multi-threaded process has been forked, then TSan (as of now) cannot reasonably function in the child, e.g. usleep() may hang because some internal lock is not released at fork. The current solution is just to disable signals resend in the child process if there are threads not survived during fork. * include/private/pthread_stop_world.h [CAN_HANDLE_FORK && THREAD_SANITIZER && SIGNAL_BASED_STOP_WORLD] (GC_retry_signals): Declare GC_EXTERN variable. * pthread_stop_world.c [CAN_HANDLE_FORK && THREAD_SANITIZER] (GC_retry_signals): Define as GC_INNER (instead of STATIC). * pthread_stop_world.c (GC_retry_signals): Always initialize to FALSE; move comment to GC_stop_init. * pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL && !NO_RETRY_SIGNALS] (GC_stop_init): Set GC_retry_signals to TRUE. * pthread_support.c [CAN_HANDLE_FORK] (GC_remove_all_threads_but_me): Update comment; change return type to GC_bool; define, set and return removed local variable. * pthread_support.c [CAN_HANDLE_FORK] (fork_child_proc): Define threads_removed local variable and set it to the result of GC_remove_all_threads_but_me(). * pthread_support.c [CAN_HANDLE_FORK && THREAD_SANITIZER && SIGNAL_BASED_STOP_WORLD] (fork_child_proc): If threads_removed or GC_parallel then GC_retry_signals set to FALSE; add comment. --- include/private/pthread_stop_world.h | 5 +++++ pthread_stop_world.c | 15 ++++++++++----- pthread_support.c | 26 +++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/include/private/pthread_stop_world.h b/include/private/pthread_stop_world.h index a1b52da72..81581dd49 100644 --- a/include/private/pthread_stop_world.h +++ b/include/private/pthread_stop_world.h @@ -54,6 +54,11 @@ GC_INNER void GC_stop_init(void); GC_INNER void *GC_CALLBACK suspend_self_inner(void *client_data); #endif +#if defined(CAN_HANDLE_FORK) && defined(THREAD_SANITIZER) \ + && defined(SIGNAL_BASED_STOP_WORLD) + GC_EXTERN GC_bool GC_retry_signals; +#endif + EXTERN_C_END #endif diff --git a/pthread_stop_world.c b/pthread_stop_world.c index ba0771c95..cbb3b2b89 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -142,13 +142,12 @@ STATIC volatile AO_t GC_stop_count; /* before they are expected to stop (unless */ /* they have stopped voluntarily). */ -#ifndef NO_RETRY_SIGNALS - /* Any platform could lose signals, so let's be conservative and */ - /* always enable signals retry logic. */ - STATIC GC_bool GC_retry_signals = TRUE; +#if defined(CAN_HANDLE_FORK) && defined(THREAD_SANITIZER) + GC_INNER #else - STATIC GC_bool GC_retry_signals = FALSE; + STATIC #endif + GC_bool GC_retry_signals = FALSE; /* * We use signals to stop threads during GC. @@ -1404,6 +1403,11 @@ GC_INNER void GC_stop_init(void) if (sigdelset(&suspend_handler_mask, GC_sig_thr_restart) != 0) ABORT("sigdelset failed"); +# ifndef NO_RETRY_SIGNALS + /* Any platform could lose signals, so let's be conservative and */ + /* always enable signals retry logic. */ + GC_retry_signals = TRUE; +# endif /* Override the default value of GC_retry_signals. */ str = GETENV("GC_RETRY_SIGNALS"); if (str != NULL) { @@ -1418,6 +1422,7 @@ GC_INNER void GC_stop_init(void) GC_COND_LOG_PRINTF( "Will retry suspend and restart signals if necessary\n"); } + # ifndef NO_SIGNALS_UNBLOCK_IN_MAIN /* Explicitly unblock the signals once before new threads creation. */ GC_unblock_gc_signals(); diff --git a/pthread_support.c b/pthread_support.c index 6742b1d8b..1cc76b57c 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -820,10 +820,12 @@ GC_API void GC_CALL GC_register_altstack(void *stack, GC_word stack_size, /* Remove all entries from the GC_threads table, except the */ /* one for the current thread. We need to do this in the child */ /* process after a fork(), since only the current thread */ -/* survives in the child. */ -STATIC void GC_remove_all_threads_but_me(void) +/* survives in the child. Returns true if at least one thread */ +/* has been removed. */ +STATIC GC_bool GC_remove_all_threads_but_me(void) { pthread_t self = pthread_self(); + GC_bool removed = FALSE; int hv; for (hv = 0; hv < THREAD_TABLE_SZ; ++hv) { @@ -875,10 +877,12 @@ STATIC void GC_remove_all_threads_but_me(void) # if !defined(THREAD_SANITIZER) || !defined(CAN_CALL_ATFORK) if (p != &first_thread) GC_INTERNAL_FREE(p); # endif + removed = TRUE; } } store_to_threads_table(hv, me); } + return removed; } #endif /* CAN_HANDLE_FORK */ @@ -1226,6 +1230,8 @@ static void fork_parent_proc(void) #endif static void fork_child_proc(void) { + GC_bool threads_removed; + GC_release_dirty_lock(); # if defined(PARALLEL_MARK) if (GC_parallel) { @@ -1238,7 +1244,21 @@ static void fork_child_proc(void) } # endif /* Clean up the thread table, so that just our thread is left. */ - GC_remove_all_threads_but_me(); + threads_removed = GC_remove_all_threads_but_me(); +# if defined(THREAD_SANITIZER) && defined(SIGNAL_BASED_STOP_WORLD) + /* If a multi-threaded process has been forked, then TSan (as of */ + /* now) cannot reasonably function in the child, e.g. usleep() */ + /* may hang because some internal lock is not released at fork. */ + if (threads_removed +# ifdef PARALLEL_MARK + || GC_parallel +# endif + ) { + GC_retry_signals = FALSE; + } +# else + (void)threads_removed; +# endif # ifdef PARALLEL_MARK /* Turn off parallel marking in the child, since we are probably */ /* just going to exec, and we would have to restart mark threads. */