From 21cf901ff695cdcf9ddffe453adf151c90d2f5d5 Mon Sep 17 00:00:00 2001 From: Patryk Ozga Date: Wed, 24 Sep 2025 16:21:01 -0700 Subject: [PATCH] make get_threadpool thread safe (#14358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: ### Key Changes Made making num_threads const ensures there is no data race. ### Benefits of This Fix * **Eliminates the Data Race**: The tsan error should no longer occur because the threadpool initialization is now atomic * **Thread Safety**: Multiple threads can safely call `get_threadpool()` concurrently * **Maintains Compatibility**: All existing functionality is preserved ### Verification * ✅ Code compiles without errors * ✅ Buck build succeeds * ✅ No diagnostic issues * ✅ Maintains existing functionality This fix should resolve the tsan failures encountered when running assistant integration tests under ThreadSanitizer. The data race that was occurring between threads T391 and T393 on the `num_threads` variable at address `0x000016aa6cf0` should now be eliminated. Reviewed By: swolchok Differential Revision: D82560656 Pulled By: Polyomino --- extension/threadpool/threadpool.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/extension/threadpool/threadpool.cpp b/extension/threadpool/threadpool.cpp index 5fee732b053..e9f3b0f5f4a 100644 --- a/extension/threadpool/threadpool.cpp +++ b/extension/threadpool/threadpool.cpp @@ -9,7 +9,6 @@ #include #include -#include #include #include @@ -101,17 +100,20 @@ ThreadPool* get_threadpool() { return nullptr; // NOLINT(facebook-hte-NullableReturn) } - int num_threads = cpuinfo_get_processors_count(); - /* - * For llvm-tsan, holding limit for the number of locks for a single thread - * is 63 (because of comparison < 64 instead of <=). pthreadpool's worst - * case is the number of threads in a pool. So we want to limit the threadpool - * size to 64 when running with tsan. However, sometimes it is tricky to - * detect if we are running under tsan, for now capping the default - * threadcount to the tsan limit unconditionally. - */ - constexpr int tsan_thread_limit = 63; - num_threads = std::min(num_threads, tsan_thread_limit); + static const int num_threads = ([]() { + int result = cpuinfo_get_processors_count(); + + /* + * For llvm-tsan, holding limit for the number of locks for a single thread + * is 63 (because of comparison < 64 instead of <=). pthreadpool's worst + * case is the number of threads in a pool. So we want to limit the + * threadpool size to 64 when running with tsan. However, sometimes it is + * tricky to detect if we are running under tsan, for now capping the + * default threadcount to the tsan limit unconditionally. + */ + constexpr int tsan_thread_limit = 63; + return std::min(result, tsan_thread_limit); + })(); static auto threadpool = std::make_unique(num_threads); // Inheriting from old threadpool to get around segfault issue