Skip to content

Commit 2d43467

Browse files
felixweilbachfacebook-github-bot
authored andcommitted
Mitigate deadlock on DLL unload (#16416)
Summary: ThreadPool gets stored in a static variable here extension/threadpool/threadpool.cpp:146 This means the destructor of ThreadPool will be run when the process exits or a DLL containing this code unloads. While working with ExecuTorch I experienced a deadlock during unloading our DLL (which contained ExecuTorch) at runtime. This was caused by the pthreadpool_destroy function pthreadpool/src/windows.c:366 waiting forever on the worker threads. Why this is happening exactly is unclear to me. It is likely a race condition inside Windows Parallel Loader (https://blogs.blackberry.com/en/2017/10/windows-10-parallel-loading-breakdown) as I could see its functions in the stack trace of the stuck worker threads after they returned from their main function. The issue was mitigated on my side by calling `executorch::extension::threadpool::get_threadpool()->_unsafe_reset_threadpool(0);` before unloading the DLL. This is just a workaround. I think a proper fix would be to rework the ThreadPool singleton and allow for explicit termination of it. Differential Revision: D89889628
1 parent daf93a1 commit 2d43467

File tree

2 files changed

+15
-0
lines changed

2 files changed

+15
-0
lines changed

extension/threadpool/threadpool.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ bool ThreadPool::_unsafe_reset_threadpool(uint32_t new_thread_count) {
7575
return true;
7676
}
7777

78+
void ThreadPool::_unsafe_destroy_threadpool() {
79+
std::lock_guard<std::mutex> lock{mutex_};
80+
ET_LOG(Info, "Destroying threadpool.");
81+
threadpool_.reset();
82+
}
83+
7884
void ThreadPool::run(
7985
runtime::FunctionRef<void(size_t)> fn,
8086
const size_t range) {

extension/threadpool/threadpool.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@ class ThreadPool final {
6969
"This API is experimental and may change without notice. Consider using UseNThreadsThreadPoolGuard")]]
7070
bool _unsafe_reset_threadpool(uint32_t num_threads);
7171

72+
/**
73+
* INTERNAL: Destroys the threadpool. This is not a thread safe call. When calling this method,
74+
* threads of the threadpool might be doing some work. Some other code may
75+
* also be holding on to the threadpool pointer, that is no longer valid.
76+
*/
77+
[[deprecated(
78+
"This API is experimental and may change without notice.")]]
79+
void _unsafe_destroy_threadpool();
80+
7281
/**
7382
* Run, in parallel, function fn(task_id) over task_id in range [0, range).
7483
* This function is blocking. All input is processed by the time it returns.

0 commit comments

Comments
 (0)