diff --git a/doc/api/n-api.md b/doc/api/n-api.md index db3fb92cd439f9..0e07ed8f0fbb4d 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -5096,11 +5096,18 @@ preventing data from being successfully added to the queue. If set to `napi_call_threadsafe_function()` never blocks if the thread-safe function was created with a maximum queue size of 0. -As a special case, when `napi_call_threadsafe_function()` is called from the -main thread, it will return `napi_would_deadlock` if the queue is full and it -was called with `napi_tsfn_blocking`. The reason for this is that the main -thread is responsible for reducing the number of items in the queue so, if it -waits for room to become available on the queue, then it will deadlock. +As a special case, when `napi_call_threadsafe_function()` is called from a +JavaScript thread, it will return `napi_would_deadlock` if the queue is full +and it was called with `napi_tsfn_blocking`. The reason for this is that the +JavaScript thread is responsible for removing items from the queue, thereby +reducing their number. Thus, if it waits for room to become available on the +queue, then it will deadlock. + +`napi_call_threadsafe_function()` will also return `napi_would_deadlock` if the +thread-safe function created on one JavaScript thread is called from another +JavaScript thread. The reason for this is to prevent a deadlock arising from the +possibility that the two JavaScript threads end up waiting on one another, +thereby both deadlocking. The actual call into JavaScript is controlled by the callback given via the `call_js_cb` parameter. `call_js_cb` is invoked on the main thread once for each @@ -5243,7 +5250,7 @@ changes: pr-url: https://github.com/nodejs/node/pull/32689 description: > Return `napi_would_deadlock` when called with `napi_tsfn_blocking` from - the main thread and the queue is full. + the main thread or a worker thread and the queue is full. --> ```C diff --git a/src/node_api.cc b/src/node_api.cc index 552538c6f05fcf..cb8bd4b482365e 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -129,7 +129,6 @@ class ThreadSafeFunction : public node::AsyncResource { is_closing(false), context(context_), max_queue_size(max_queue_size_), - main_thread(uv_thread_self()), env(env_), finalize_data(finalize_data_), finalize_cb(finalize_cb_), @@ -149,16 +148,36 @@ class ThreadSafeFunction : public node::AsyncResource { napi_status Push(void* data, napi_threadsafe_function_call_mode mode) { node::Mutex::ScopedLock lock(this->mutex); - uv_thread_t current_thread = uv_thread_self(); while (queue.size() >= max_queue_size && max_queue_size > 0 && !is_closing) { if (mode == napi_tsfn_nonblocking) { return napi_queue_full; - } else if (uv_thread_equal(¤t_thread, &main_thread)) { - return napi_would_deadlock; } + + // Here we check if there is a Node.js event loop running on this thread. + // If there is, and our queue is full, we return `napi_would_deadlock`. We + // do this for two reasons: + // + // 1. If this is the thread on which our own event loop runs then we + // cannot wait here because that will prevent our event loop from + // running and emptying the very queue on which we are waiting. + // + // 2. If this is not the thread on which our own event loop runs then we + // still cannot wait here because that allows the following sequence of + // events: + // + // 1. JSThread1 calls JSThread2 and blocks while its queue is full and + // because JSThread2's queue is also full. + // + // 2. JSThread2 calls JSThread1 before it's had a chance to remove an + // item from its own queue and blocks because JSThread1's queue is + // also full. + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + if (isolate != nullptr && node::GetCurrentEventLoop(isolate) != nullptr) + return napi_would_deadlock; + cond->Wait(lock); } @@ -438,7 +457,6 @@ class ThreadSafeFunction : public node::AsyncResource { // means we don't need the mutex to read them. void* context; size_t max_queue_size; - uv_thread_t main_thread; // These are variables accessed only from the loop thread. v8impl::Persistent ref;