[compiler-rt] r319004 - Prevent Thread Exited/Joined events race
Kamil Rytarowski via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 27 08:26:04 PST 2017
Hmm, is there a shell account to check it on ppc64be-linux?
Are there problems with atomic functions?
I don't see anything wrong with the code off-hand.
On 27.11.2017 17:12, Matt Morehouse wrote:
> 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 <mailto: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
> <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
> <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
> <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
> <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 <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 850 bytes
Desc: OpenPGP digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171127/6383974f/attachment.sig>
More information about the llvm-commits
mailing list