<div dir="ltr">Looks like this patch is breaking the <a href="http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/4708">ppc64be-linux bot</a>. Check-tsan is timing out. This may have introduced a deadlock.</div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Nov 26, 2017 at 12:20 PM, Kamil Rytarowski via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kamil<br>
Date: Sun Nov 26 12:20:42 2017<br>
New Revision: 319004<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=319004&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=319004&view=rev</a><br>
Log:<br>
Prevent Thread Exited/Joined events race<br>
<br>
Summary:<br>
Add atomic verification to ensure that Thread is Joined after marking it<br>
Finished.<br>
<br>
It is required for NetBSD in order to prevent Thread Exited/Joined race,<br>
that may occur when native system libpthread(3) cannot be reliably traced<br>
in a way to guarantee that the mentioned events happen one after another.<br>
<br>
This change fixes at least TSan and LSan on NetBSD.<br>
<br>
Sponsored by <The NetBSD Foundation><br>
<br>
Reviewers: joerg, dvyukov, vitalybuka<br>
<br>
Reviewed By: dvyukov<br>
<br>
Subscribers: llvm-commits, kubamracek, #sanitizers<br>
<br>
Tags: #sanitizers<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D40294" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D40294</a><br>
<br>
Modified:<br>
compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.cc<br>
compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.h<br>
<br>
Modified: compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc?rev=319004&r1=319003&r2=319004&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.cc?rev=319004&<wbr>r1=319003&r2=319004&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.cc (original)<br>
+++ compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.cc Sun Nov 26 12:20:42 2017<br>
@@ -21,6 +21,7 @@ ThreadContextBase::<wbr>ThreadContextBase(u32<br>
status(ThreadStatusInvalid),<br>
detached(false), workerthread(false), parent_tid(0), next(0) {<br>
name[0] = '\0';<br>
+ atomic_store(&thread_<wbr>destroyed, 0, memory_order_release);<br>
}<br>
<br>
ThreadContextBase::~<wbr>ThreadContextBase() {<br>
@@ -44,6 +45,14 @@ void ThreadContextBase::SetDead() {<br>
OnDead();<br>
}<br>
<br>
+void ThreadContextBase::<wbr>SetDestroyed() {<br>
+ atomic_store(&thread_<wbr>destroyed, 1, memory_order_release);<br>
+}<br>
+<br>
+bool ThreadContextBase::<wbr>GetDestroyed() {<br>
+ return !!atomic_load(&thread_<wbr>destroyed, memory_order_acquire);<br>
+}<br>
+<br>
void ThreadContextBase::SetJoined(<wbr>void *arg) {<br>
// FIXME(dvyukov): print message and continue (it's user error).<br>
CHECK_EQ(false, detached);<br>
@@ -85,6 +94,7 @@ void ThreadContextBase::SetCreated(<wbr>uptr<br>
void ThreadContextBase::Reset() {<br>
status = ThreadStatusInvalid;<br>
SetName(0);<br>
+ atomic_store(&thread_<wbr>destroyed, 0, memory_order_release);<br>
OnReset();<br>
}<br>
<br>
@@ -243,16 +253,25 @@ void ThreadRegistry::DetachThread(<wbr>u32 ti<br>
}<br>
<br>
void ThreadRegistry::JoinThread(u32 tid, void *arg) {<br>
- BlockingMutexLock l(&mtx_);<br>
- CHECK_LT(tid, n_contexts_);<br>
- ThreadContextBase *tctx = threads_[tid];<br>
- CHECK_NE(tctx, 0);<br>
- if (tctx->status == ThreadStatusInvalid) {<br>
- Report("%s: Join of non-existent thread\n", SanitizerToolName);<br>
- return;<br>
- }<br>
- tctx->SetJoined(arg);<br>
- QuarantinePush(tctx);<br>
+ bool destroyed = false;<br>
+ do {<br>
+ {<br>
+ BlockingMutexLock l(&mtx_);<br>
+ CHECK_LT(tid, n_contexts_);<br>
+ ThreadContextBase *tctx = threads_[tid];<br>
+ CHECK_NE(tctx, 0);<br>
+ if (tctx->status == ThreadStatusInvalid) {<br>
+ Report("%s: Join of non-existent thread\n", SanitizerToolName);<br>
+ return;<br>
+ }<br>
+ if ((destroyed = tctx->GetDestroyed())) {<br>
+ tctx->SetJoined(arg);<br>
+ QuarantinePush(tctx);<br>
+ }<br>
+ }<br>
+ if (!destroyed)<br>
+ internal_sched_yield();<br>
+ } while (!destroyed);<br>
}<br>
<br>
// Normally this is called when the thread is about to exit. If<br>
@@ -281,6 +300,7 @@ void ThreadRegistry::FinishThread(<wbr>u32 ti<br>
tctx->SetDead();<br>
QuarantinePush(tctx);<br>
}<br>
+ tctx->SetDestroyed();<br>
}<br>
<br>
void ThreadRegistry::StartThread(<wbr>u32 tid, tid_t os_id, bool workerthread,<br>
<br>
Modified: compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h?rev=319004&r1=319003&r2=319004&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.h?rev=319004&<wbr>r1=319003&r2=319004&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.h (original)<br>
+++ compiler-rt/trunk/lib/<wbr>sanitizer_common/sanitizer_<wbr>thread_registry.h Sun Nov 26 12:20:42 2017<br>
@@ -50,6 +50,8 @@ class ThreadContextBase {<br>
u32 parent_tid;<br>
ThreadContextBase *next; // For storing thread contexts in a list.<br>
<br>
+ atomic_uint32_t thread_destroyed; // To address race of Joined vs Finished<br>
+<br>
void SetName(const char *new_name);<br>
<br>
void SetDead();<br>
@@ -60,6 +62,9 @@ class ThreadContextBase {<br>
u32 _parent_tid, void *arg);<br>
void Reset();<br>
<br>
+ void SetDestroyed();<br>
+ bool GetDestroyed();<br>
+<br>
// The following methods may be overriden by subclasses.<br>
// Some of them take opaque arg that may be optionally be used<br>
// by subclasses.<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>