[compiler-rt] r319004 - Prevent Thread Exited/Joined events race

[Kamil Rytarowski via llvm-commits]

Author: kamil
Date: Sun Nov 26 12:20:42 2017
New Revision: 319004

URL: http://llvm.org/viewvc/llvm-project?rev=319004&view=rev
Log:
Prevent Thread Exited/Joined events race

Summary:
Add atomic verification to ensure that Thread is Joined after marking it
Finished.

It is required for NetBSD in order to prevent Thread Exited/Joined race,
that may occur when native system libpthread(3) cannot be reliably traced
in a way to guarantee that the mentioned events happen one after another.

This change fixes at least TSan and LSan on NetBSD.

Sponsored by <The NetBSD Foundation>

Reviewers: joerg, dvyukov, vitalybuka

Reviewed By: dvyukov

Subscribers: llvm-commits, kubamracek, #sanitizers

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D40294

Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc?rev=319004&r1=319003&r2=319004&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc Sun Nov 26 12:20:42 2017
@@ -21,6 +21,7 @@ ThreadContextBase::ThreadContextBase(u32
       status(ThreadStatusInvalid),
       detached(false), workerthread(false), parent_tid(0), next(0) {
   name[0] = '\0';
+  atomic_store(&thread_destroyed, 0, memory_order_release);
 }
 
 ThreadContextBase::~ThreadContextBase() {
@@ -44,6 +45,14 @@ void ThreadContextBase::SetDead() {
   OnDead();
 }
 
+void ThreadContextBase::SetDestroyed() {
+  atomic_store(&thread_destroyed, 1, memory_order_release);
+}
+
+bool ThreadContextBase::GetDestroyed() {
+  return !!atomic_load(&thread_destroyed, memory_order_acquire);
+}
+
 void ThreadContextBase::SetJoined(void *arg) {
   // FIXME(dvyukov): print message and continue (it's user error).
   CHECK_EQ(false, detached);
@@ -85,6 +94,7 @@ void ThreadContextBase::SetCreated(uptr
 void ThreadContextBase::Reset() {
   status = ThreadStatusInvalid;
   SetName(0);
+  atomic_store(&thread_destroyed, 0, memory_order_release);
   OnReset();
 }
 
@@ -243,16 +253,25 @@ void ThreadRegistry::DetachThread(u32 ti
 }
 
 void ThreadRegistry::JoinThread(u32 tid, void *arg) {
-  BlockingMutexLock l(&mtx_);
-  CHECK_LT(tid, n_contexts_);
-  ThreadContextBase *tctx = threads_[tid];
-  CHECK_NE(tctx, 0);
-  if (tctx->status == ThreadStatusInvalid) {
-    Report("%s: Join of non-existent thread\n", SanitizerToolName);
-    return;
-  }
-  tctx->SetJoined(arg);
-  QuarantinePush(tctx);
+  bool destroyed = false;
+  do {
+    {
+      BlockingMutexLock l(&mtx_);
+      CHECK_LT(tid, n_contexts_);
+      ThreadContextBase *tctx = threads_[tid];
+      CHECK_NE(tctx, 0);
+      if (tctx->status == ThreadStatusInvalid) {
+        Report("%s: Join of non-existent thread\n", SanitizerToolName);
+        return;
+      }
+      if ((destroyed = tctx->GetDestroyed())) {
+        tctx->SetJoined(arg);
+        QuarantinePush(tctx);
+      }
+    }
+    if (!destroyed)
+      internal_sched_yield();
+  } while (!destroyed);
 }
 
 // Normally this is called when the thread is about to exit.  If
@@ -281,6 +300,7 @@ void ThreadRegistry::FinishThread(u32 ti
     tctx->SetDead();
     QuarantinePush(tctx);
   }
+  tctx->SetDestroyed();
 }
 
 void ThreadRegistry::StartThread(u32 tid, tid_t os_id, bool workerthread,

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h?rev=319004&r1=319003&r2=319004&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h Sun Nov 26 12:20:42 2017
@@ -50,6 +50,8 @@ class ThreadContextBase {
   u32 parent_tid;
   ThreadContextBase *next;  // For storing thread contexts in a list.
 
+  atomic_uint32_t thread_destroyed; // To address race of Joined vs Finished
+
   void SetName(const char *new_name);
 
   void SetDead();
@@ -60,6 +62,9 @@ class ThreadContextBase {
                   u32 _parent_tid, void *arg);
   void Reset();
 
+  void SetDestroyed();
+  bool GetDestroyed();
+
   // The following methods may be overriden by subclasses.
   // Some of them take opaque arg that may be optionally be used
   // by subclasses.