From: Ivan Maidanski <ivmai@mail.ru>
Date: Mon, 13 Nov 2017 04:58:36 +0000 (+0300)
Subject: Workaround TSan false positives in suspend/resume_thread
X-Git-Tag: v8.0.0~515
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=918500dcd5f629391e442f68620873909c733ea7;p=gc

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.
---

diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 4834a059..cfb4c1ae 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;