[PATCH] D95184: [ASan] Stop blocking child thread progress from parent thread in `pthread_create` interceptor.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.


================
Comment at: compiler-rt/lib/asan/asan_allocator.cpp:1197
+      reinterpret_cast<__asan::AsanThreadContext *>(tctx);
+  __asan::AsanThread *asanThread = atctx->thread;
+
----------------
historically sanitizers use snake case for local variable, from Google coding style


================
Comment at: compiler-rt/lib/asan/asan_allocator.cpp:1202-1204
+  if (!(atctx->status == ThreadStatusCreated ||
+        atctx->status == ThreadStatusRunning))
+    return;
----------------
  if (atctx->status != ThreadStatusCreated && atctx->status != ThreadStatusRunning)
    return;



================
Comment at: compiler-rt/lib/asan/asan_interceptors.cpp:228
+    // Note AsanThreadContexts never get destroyed so that still leaks.
+    t->Destroy();
   }
----------------
delcypher wrote:
> @vitalybuka **Is `AsanThread::Destroy()` safe to call from this thread and in this context where the corresponding thread was never started.?** I wasn't really sure. 
> 
> I see that calling this will call `DTLS_Destroy()` and I'm not sure if that needs to be called from a different thread (i.e. called from the thread before its destroyed). It looks like this function is a no-op on macOS which is where I'm testing.
> 
> 
I think DTLS_Destroy should be fine here. I assume this thread will never showup in suspended_threads so lsan will not try to scan it.

As I understand Ctx is not leaked but pushed into quarantine by ThreadRegistry::FinishThread


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95184/new/

https://reviews.llvm.org/D95184