From 918500dcd5f629391e442f68620873909c733ea7 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Mon, 13 Nov 2017 07:58:36 +0300 Subject: [PATCH] Workaround TSan false positives in suspend/resume_thread * pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL && GC_ENABLE_SUSPEND_THREAD] (set_suspended_ext_flag): New static function (with GC_ATTR_NO_SANITIZE_THREAD attribute); add comment. * pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL && GC_ENABLE_SUSPEND_THREAD] (GC_suspend_thread): Call set_suspended_ext_flag() instead of t->flags|=SUSPENDED_EXT. * pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL && GC_ENABLE_SUSPEND_THREAD] (GC_resume_thread): Add GC_ATTR_NO_SANITIZE_THREAD attribute; add comment. --- pthread_stop_world.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 4834a05..cfb4c1a 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -420,6 +420,16 @@ STATIC void GC_restart_handler(int sig) return NULL; } + /* TSan reports data races between the suspension flag setter */ + /* (called from GC_suspend_thread) and GC_suspend_handler_inner, */ + /* suspend_self_inner. The first one seems to be a false positive */ + /* as the handler is invoked after RAISE_SIGNAL, and the 2nd one is */ + /* safe to be ignored as the flag is checked in a loop. */ + static void set_suspended_ext_flag(GC_thread t) GC_ATTR_NO_SANITIZE_THREAD + { + t -> flags |= SUSPENDED_EXT; + } + GC_API void GC_CALL GC_suspend_thread(GC_SUSPEND_THREAD_ID thread) { GC_thread t; IF_CANCEL(int cancel_state;) @@ -432,7 +442,7 @@ STATIC void GC_restart_handler(int sig) return; } - t -> flags |= SUSPENDED_EXT; + set_suspended_ext_flag(t); if ((pthread_t)thread == pthread_self()) { UNLOCK(); /* It is safe as "t" cannot become invalid here (no race with */ @@ -480,7 +490,12 @@ STATIC void GC_restart_handler(int sig) UNLOCK(); } - GC_API void GC_CALL GC_resume_thread(GC_SUSPEND_THREAD_ID thread) { + /* Same as for GC_suspend_thread(), TSan reports data races between */ + /* this function and GC_suspend_handler_inner, suspend_self_inner; */ + /* it is safe to ignore them both. */ + GC_API void GC_CALL GC_resume_thread(GC_SUSPEND_THREAD_ID thread) + GC_ATTR_NO_SANITIZE_THREAD + { GC_thread t; DCL_LOCK_STATE; -- 2.7.4