[PATCH] D40294: Prevent Thread Exited/Joined events race

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:87
 void ThreadContextBase::Reset() {
   status = ThreadStatusInvalid;
   SetName(0);
----------------
dvyukov wrote:
> This also needs to reset thread_destroyed because contexts are reused.
I see, this might be the reason why I observed the races!


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:97
+bool ThreadContextBase::GetDestroyed() {
+  return !!atomic_load(&thread_destroyed, memory_order_acquire);
+}
----------------
dvyukov wrote:
> Now looking at the code more closely, I think we can contain this all inside of ThreadRegistry without changing public API and exposing more guts.
> 
> We could set thread_destroyed in ThreadRegistry::FinishThread (right after tctx->SetFinished()), and wait for the flag in ThreadRegistry::JoinThread (need to release the mutex around yield).
> This looks like a cleaner solution and will fix all sanitizers at once.
> Will this work for netbsd?
> 
> Sorry for not noticing it earlier.
I will give it a try!


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:190
   }
-  return kUnknownTid;
+  return 0;
 }
----------------
dvyukov wrote:
> we now generally use nullptr for pointers
OK!


Repository:
  rL LLVM

https://reviews.llvm.org/D40294