n-api: detect deadlocks in thread-safe function

We introduce status `napi_would_deadlock` to be used as a return status
by `napi_call_threadsafe_function` if the call is made with
`napi_tsfn_blocking` on the main thread and the queue is full.

PR-URL: #32689
Fixes: #32615
Signed-off-by: Gabriel Schulhof <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

Author: Gabriel Schulhof

doc/api/n-api.md

@@ -457,6 +457,7 @@ typedef enum {
   napi_date_expected,
   napi_arraybuffer_expected,
   napi_detachable_arraybuffer_expected,
+  napi_would_deadlock,
 } napi_status;
 ```
 
@@ -5095,6 +5096,12 @@ 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.
+
 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
 value that was placed into the queue by a successful call to
@@ -5231,6 +5238,12 @@ This API may be called from any thread which makes use of `func`.
 <!-- YAML
 added: v10.6.0
 napiVersion: 4
+changes:
+  - version: REPLACEME
+    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.
 -->
 
 ```C
@@ -5248,9 +5261,13 @@ napi_call_threadsafe_function(napi_threadsafe_function func,
   `napi_tsfn_nonblocking` to indicate that the call should return immediately
   with a status of `napi_queue_full` whenever the queue is full.
 
+This API will return `napi_would_deadlock` if called with `napi_tsfn_blocking`
+from the main thread and the queue is full.
+
 This API will return `napi_closing` if `napi_release_threadsafe_function()` was
-called with `abort` set to `napi_tsfn_abort` from any thread. The value is only
-added to the queue if the API returns `napi_ok`.
+called with `abort` set to `napi_tsfn_abort` from any thread.
+
+The value is only added to the queue if the API returns `napi_ok`.
 
 This API may be called from any thread which makes use of `func`.
 

src/js_native_api_types.h

@@ -82,11 +82,15 @@ typedef enum {
   napi_date_expected,
   napi_arraybuffer_expected,
   napi_detachable_arraybuffer_expected,
+  napi_would_deadlock
 } napi_status;
 // Note: when adding a new enum value to `napi_status`, please also update
-// `const int last_status` in `napi_get_last_error_info()' definition,
-// in file js_native_api_v8.cc. Please also update the definition of
-// `napi_status` in doc/api/n-api.md to reflect the newly added value(s).
+//   * `const int last_status` in the definition of `napi_get_last_error_info()'
+//     in file js_native_api_v8.cc.
+//   * `const char* error_messages[]` in file js_native_api_v8.cc with a brief
+//     message explaining the error.
+//   * the definition of `napi_status` in doc/api/n-api.md to reflect the newly
+//     added value(s).
 
 typedef napi_value (*napi_callback)(napi_env env,
                                     napi_callback_info info);

src/js_native_api_v8.cc

@@ -740,6 +740,7 @@ const char* error_messages[] = {nullptr,
                                 "A date was expected",
                                 "An arraybuffer was expected",
                                 "A detachable arraybuffer was expected",
+                                "Main thread would deadlock",
 };
 
 napi_status napi_get_last_error_info(napi_env env,
@@ -751,7 +752,7 @@ napi_status napi_get_last_error_info(napi_env env,
   // message in the `napi_status` enum each time a new error message is added.
   // We don't have a napi_status_last as this would result in an ABI
   // change each time a message was added.
-  const int last_status = napi_detachable_arraybuffer_expected;
+  const int last_status = napi_would_deadlock;
 
   static_assert(
       NAPI_ARRAYSIZE(error_messages) == last_status + 1,

src/node_api.cc

@@ -129,6 +129,7 @@ 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_),
@@ -148,12 +149,15 @@ 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(&current_thread, &main_thread)) {
+        return napi_would_deadlock;
       }
       cond->Wait(lock);
     }
@@ -434,6 +438,7 @@ 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<v8::Function> ref;

test/node-api/test_threadsafe_function/binding.c

@@ -267,6 +267,60 @@ static napi_value StartThreadNoJsFunc(napi_env env, napi_callback_info info) {
     /** block_on_full */true, /** alt_ref_js_cb */true);
 }
 
+static void DeadlockTestDummyMarshaller(napi_env env,
+                                        napi_value empty0,
+                                        void* empty1,
+                                        void* empty2) {}
+
+static napi_value TestDeadlock(napi_env env, napi_callback_info info) {
+  napi_threadsafe_function tsfn;
+  napi_status status;
+  napi_value async_name;
+  napi_value return_value;
+
+  // Create an object to store the returned information.
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+
+  // Create a string to be used with the thread-safe function.
+  NAPI_CALL(env, napi_create_string_utf8(env,
+                                     "N-API Thread-safe Function Deadlock Test",
+                                     NAPI_AUTO_LENGTH,
+                                     &async_name));
+
+  // Create the thread-safe function with a single queue slot and a single thread.
+  NAPI_CALL(env, napi_create_threadsafe_function(env,
+                                                 NULL,
+                                                 NULL,
+                                                 async_name,
+                                                 1,
+                                                 1,
+                                                 NULL,
+                                                 NULL,
+                                                 NULL,
+                                                 DeadlockTestDummyMarshaller,
+                                                 &tsfn));
+
+  // Call the threadsafe function. This should succeed and fill the queue.
+  NAPI_CALL(env, napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_blocking));
+
+  // Call the threadsafe function. This should not block, but return
+  // `napi_would_deadlock`. We save the resulting status in an object to be
+  // returned.
+  status = napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_blocking);
+  add_returned_status(env,
+                      "deadlockTest",
+                      return_value,
+                      "Main thread would deadlock",
+                      napi_would_deadlock,
+                      status);
+
+  // Clean up the thread-safe function before returning.
+  NAPI_CALL(env, napi_release_threadsafe_function(tsfn, napi_tsfn_release));
+
+  // Return the result.
+  return return_value;
+}
+
 // Module init
 static napi_value Init(napi_env env, napi_value exports) {
   size_t index;
@@ -305,6 +359,7 @@ static napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NAPI_PROPERTY("StopThread", StopThread),
     DECLARE_NAPI_PROPERTY("Unref", Unref),
     DECLARE_NAPI_PROPERTY("Release", Release),
+    DECLARE_NAPI_PROPERTY("TestDeadlock", TestDeadlock),
   };
 
   NAPI_CALL(env, napi_define_properties(env, exports,

test/node-api/test_threadsafe_function/binding.gyp

@@ -2,7 +2,10 @@
   'targets': [
     {
       'target_name': 'binding',
-      'sources': ['binding.c']
+      'sources': [
+        'binding.c',
+        '../../js-native-api/common.c'
+      ]
     }
   ]
 }

test/node-api/test_threadsafe_function/test.js

@@ -210,8 +210,13 @@ new Promise(function testWithoutJSMarshaller(resolve) {
 }))
 .then((result) => assert.strictEqual(result.indexOf(0), -1))
 
-// Start a child process to test rapid teardown
+// Start a child process to test rapid teardown.
 .then(() => testUnref(binding.MAX_QUEUE_SIZE))
 
-// Start a child process with an infinite queue to test rapid teardown
-.then(() => testUnref(0));
+// Start a child process with an infinite queue to test rapid teardown.
+.then(() => testUnref(0))
+
+// Test deadlock prevention.
+.then(() => assert.deepStrictEqual(binding.TestDeadlock(), {
+  deadlockTest: 'Main thread would deadlock'
+}));