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

[Matt Morehouse via llvm-commits]

Looks like this patch is breaking the ppc64be-linux bot
<http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/4708>.
Check-tsan is timing out.  This may have introduced a deadlock.

On Sun, Nov 26, 2017 at 12:20 PM, Kamil Rytarowski via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171127/6f17cceb/attachment.html>