Skip to content

Commit c79bcd8

Browse files
committed
fix: solved deadlock problem in finalization
1 parent 87a78f2 commit c79bcd8

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

packages/host/cpp/ThreadsafeFunction.cpp

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,22 @@ napi_status ThreadSafeFunction::release(
190190

191191
const auto remaining = threadCount_.fetch_sub(1, std::memory_order_acq_rel);
192192

193-
// When the last thread is gone (or we're closing), notify and finalize.
194-
if (remaining <= 1 || closing_.load(std::memory_order_acquire)) {
195-
std::lock_guard lock{queueMutex_};
196-
const bool emptyQueue = queue_.empty();
197-
if (maxQueueSize_) {
198-
queueCv_.notify_all();
193+
// When exactly the last thread is gone (or we're aborting), notify and
194+
// finalize. Use == 1 instead of <= 1 to avoid race where multiple threads
195+
// trigger finalization.
196+
const bool shouldTriggerFinalize =
197+
(remaining == 1) || closing_.load(std::memory_order_acquire);
198+
199+
if (shouldTriggerFinalize) {
200+
bool emptyQueue = false;
201+
{
202+
std::lock_guard lock{queueMutex_};
203+
emptyQueue = queue_.empty();
204+
if (maxQueueSize_) {
205+
queueCv_.notify_all();
206+
}
199207
}
208+
// Call finalize() outside the lock to avoid deadlock if JS thread re-enters
200209
if (aborted_.load(std::memory_order_acquire) || emptyQueue) {
201210
finalize();
202211
}
@@ -263,6 +272,8 @@ void ThreadSafeFunction::processQueue() {
263272
if (wasAtMaxCapacity && maxQueueSize_) {
264273
queueCv_.notify_one();
265274
}
275+
} else {
276+
empty = true;
266277
}
267278
}
268279

@@ -300,6 +311,6 @@ bool ThreadSafeFunction::isClosingOrAborted() const noexcept {
300311

301312
bool ThreadSafeFunction::shouldFinalize() const noexcept {
302313
return threadCount_.load(std::memory_order_acquire) == 0 &&
303-
!closing_.load(std::memory_order_acquire);
314+
closing_.load(std::memory_order_acquire);
304315
}
305316
} // namespace callstack::nodeapihost

0 commit comments

Comments
 (0)