[Support] Restrict ManagedStatic ThreadPoolExecutor to Windows

[MaskRay]

https://reviews.llvm.org/D70447 switched to ManagedStatic to work
around race conditions in MSVC runtimes and the MinGW runtime.
However, ManagedStatic is not suitable for other platforms.

However, this workaround is not suitable for other platforms (#66974).
lld::fatal calls exitLld(1), which calls llvm_shutdown to destroy
ManagedStatic objects. The worker threads will finish and invoke TLS
destructors (glibc __call_tls_dtors). This can lead to race conditions
if other threads attempt to access TLS objects that have already been
destroyed.

While lld's early exit mechanism needs more work, I believe Parallel.cpp
should avoid this pitfall as well.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Fangrui Song (MaskRay)

Changes

https://reviews.llvm.org/D70447 switched to ManagedStatic to work
around race conditions in MSVC runtimes and the MinGW runtime.
However, ManagedStatic is not suitable for other platforms.

However, this workaround is not suitable for other platforms (#66974).
lld::fatal calls exitLld(1), which calls llvm_shutdown to destroy
ManagedStatic objects. The worker threads will finish and invoke TLS
destructors (glibc __call_tls_dtors). This can lead to race conditions
if other threads attempt to access TLS objects that have already been
destroyed.

While lld's early exit mechanism needs more work, I believe Parallel.cpp
should avoid this pitfall as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/102989.diff

1 Files Affected:

• (modified) llvm/lib/Support/Parallel.cpp (+9)

diff --git a/llvm/lib/Support/Parallel.cpp b/llvm/lib/Support/Parallel.cpp
index af35947192c0db..849041f0e21505 100644
--- a/llvm/lib/Support/Parallel.cpp
+++ b/llvm/lib/Support/Parallel.cpp
@@ -156,6 +156,7 @@ class ThreadPoolExecutor : public Executor {
 };
 
 Executor *Executor::getDefaultExecutor() {
+#ifdef _WIN32
   // The ManagedStatic enables the ThreadPoolExecutor to be stopped via
   // llvm_shutdown() which allows a "clean" fast exit, e.g. via _exit(). This
   // stops the thread pool and waits for any worker thread creation to complete
@@ -179,6 +180,14 @@ Executor *Executor::getDefaultExecutor() {
       ManagedExec;
   static std::unique_ptr<ThreadPoolExecutor> Exec(&(*ManagedExec));
   return Exec.get();
+#else
+  // ManagedStatic is not desired on other platforms. When `Exec` is terminated
+  // by llvm_shutdown(), worker threads will clean up and invoke TLS
+  // destructors. This can lead to race conditions if other threads attempt to
+  // access TLS objects that have already been destroyed.
+  static ThreadPoolExecutor Exec(strategy);
+  return &Exec;
+#endif
 }
 } // namespace
 } // namespace detail

[mstorsjo]

LGTM

[nga888]

I've been on PTO, but this LGTM. Only comment is that the mention of other threads accessing TLS objects reads a bit odd. IIUC, isn't the issue related to the fact that other threads access objects allocated by a different thread local allocator?

[MaskRay]

I've been on PTO, but this LGTM. Only comment is that the mention of other threads accessing TLS objects reads a bit odd. IIUC, isn't the issue related to the fact that other threads access objects allocated by a different thread local allocator?

Yes, the issue was due to threads access objects allocated by a different thread local allocator. The objects might be destroyed TLS destructors.