Skip to content

Commit 50ab90a

Browse files
authored
make get_threadpool thread safe (#14358)
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. Differential Revision: D82560656
1 parent dc87d22 commit 50ab90a

File tree

1 file changed

+14
-12
lines changed

1 file changed

+14
-12
lines changed

extension/threadpool/threadpool.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include <executorch/extension/threadpool/threadpool.h>
1010

1111
#include <algorithm>
12-
#include <atomic>
1312
#include <memory>
1413

1514
#include <executorch/extension/threadpool/threadpool_guard.h>
@@ -101,17 +100,20 @@ ThreadPool* get_threadpool() {
101100
return nullptr; // NOLINT(facebook-hte-NullableReturn)
102101
}
103102

104-
int num_threads = cpuinfo_get_processors_count();
105-
/*
106-
* For llvm-tsan, holding limit for the number of locks for a single thread
107-
* is 63 (because of comparison < 64 instead of <=). pthreadpool's worst
108-
* case is the number of threads in a pool. So we want to limit the threadpool
109-
* size to 64 when running with tsan. However, sometimes it is tricky to
110-
* detect if we are running under tsan, for now capping the default
111-
* threadcount to the tsan limit unconditionally.
112-
*/
113-
constexpr int tsan_thread_limit = 63;
114-
num_threads = std::min(num_threads, tsan_thread_limit);
103+
static const int num_threads = ([]() {
104+
int result = cpuinfo_get_processors_count();
105+
106+
/*
107+
* For llvm-tsan, holding limit for the number of locks for a single thread
108+
* is 63 (because of comparison < 64 instead of <=). pthreadpool's worst
109+
* case is the number of threads in a pool. So we want to limit the
110+
* threadpool size to 64 when running with tsan. However, sometimes it is
111+
* tricky to detect if we are running under tsan, for now capping the
112+
* default threadcount to the tsan limit unconditionally.
113+
*/
114+
constexpr int tsan_thread_limit = 63;
115+
return std::min(result, tsan_thread_limit);
116+
})();
115117
static auto threadpool = std::make_unique<ThreadPool>(num_threads);
116118

117119
// Inheriting from old threadpool to get around segfault issue

0 commit comments

Comments
 (0)