GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — n-api: detect deadlocks in thread-safe function · nodejs/node@3907de7

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • doc/api
    • n-api.md
  • src
    • js_native_api_types.h
    • js_native_api_v8.cc
    • node_api.cc
  • test/node-api/test_threadsafe_function
    • binding.c
    • binding.gyp
    • test.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/api/n-api.md‎

Copy file name to clipboardExpand all lines: doc/api/n-api.md
+19-2Lines changed: 19 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,7 @@ typedef enum {
457457
napi_date_expected,
458458
napi_arraybuffer_expected,
459459
napi_detachable_arraybuffer_expected,
460+
napi_would_deadlock,
460461
} napi_status;
461462
```
462463

@@ -5095,6 +5096,12 @@ preventing data from being successfully added to the queue. If set to
50955096
`napi_call_threadsafe_function()` never blocks if the thread-safe function was
50965097
created with a maximum queue size of 0.
50975098

5099+
As a special case, when `napi_call_threadsafe_function()` is called from the
5100+
main thread, it will return `napi_would_deadlock` if the queue is full and it
5101+
was called with `napi_tsfn_blocking`. The reason for this is that the main
5102+
thread is responsible for reducing the number of items in the queue so, if it
5103+
waits for room to become available on the queue, then it will deadlock.
5104+
50985105
The actual call into JavaScript is controlled by the callback given via the
50995106
`call_js_cb` parameter. `call_js_cb` is invoked on the main thread once for each
51005107
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`.
52315238
<!-- YAML
52325239
added: v10.6.0
52335240
napiVersion: 4
5241+
changes:
5242+
- version: REPLACEME
5243+
pr-url: https://github.com/nodejs/node/pull/32689
5244+
description: >
5245+
Return `napi_would_deadlock` when called with `napi_tsfn_blocking` from
5246+
the main thread and the queue is full.
52345247
-->
52355248

52365249
```C
@@ -5248,9 +5261,13 @@ napi_call_threadsafe_function(napi_threadsafe_function func,
52485261
`napi_tsfn_nonblocking` to indicate that the call should return immediately
52495262
with a status of `napi_queue_full` whenever the queue is full.
52505263

5264+
This API will return `napi_would_deadlock` if called with `napi_tsfn_blocking`
5265+
from the main thread and the queue is full.
5266+
52515267
This API will return `napi_closing` if `napi_release_threadsafe_function()` was
5252-
called with `abort` set to `napi_tsfn_abort` from any thread. The value is only
5253-
added to the queue if the API returns `napi_ok`.
5268+
called with `abort` set to `napi_tsfn_abort` from any thread.
5269+
5270+
The value is only added to the queue if the API returns `napi_ok`.
52545271

52555272
This API may be called from any thread which makes use of `func`.
52565273

Collapse file

‎src/js_native_api_types.h‎

Copy file name to clipboardExpand all lines: src/js_native_api_types.h
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,15 @@ typedef enum {
8282
napi_date_expected,
8383
napi_arraybuffer_expected,
8484
napi_detachable_arraybuffer_expected,
85+
napi_would_deadlock
8586
} napi_status;
8687
// Note: when adding a new enum value to `napi_status`, please also update
87-
// `const int last_status` in `napi_get_last_error_info()' definition,
88-
// in file js_native_api_v8.cc. Please also update the definition of
89-
// `napi_status` in doc/api/n-api.md to reflect the newly added value(s).
88+
// * `const int last_status` in the definition of `napi_get_last_error_info()'
89+
// in file js_native_api_v8.cc.
90+
// * `const char* error_messages[]` in file js_native_api_v8.cc with a brief
91+
// message explaining the error.
92+
// * the definition of `napi_status` in doc/api/n-api.md to reflect the newly
93+
// added value(s).
9094

9195
typedef napi_value (*napi_callback)(napi_env env,
9296
napi_callback_info info);
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,7 @@ const char* error_messages[] = {nullptr,
740740
"A date was expected",
741741
"An arraybuffer was expected",
742742
"A detachable arraybuffer was expected",
743+
"Main thread would deadlock",
743744
};
744745

745746
napi_status napi_get_last_error_info(napi_env env,
@@ -751,7 +752,7 @@ napi_status napi_get_last_error_info(napi_env env,
751752
// message in the `napi_status` enum each time a new error message is added.
752753
// We don't have a napi_status_last as this would result in an ABI
753754
// change each time a message was added.
754-
const int last_status = napi_detachable_arraybuffer_expected;
755+
const int last_status = napi_would_deadlock;
755756

756757
static_assert(
757758
NAPI_ARRAYSIZE(error_messages) == last_status + 1,
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ class ThreadSafeFunction : public node::AsyncResource {
129129
is_closing(false),
130130
context(context_),
131131
max_queue_size(max_queue_size_),
132+
main_thread(uv_thread_self()),
132133
env(env_),
133134
finalize_data(finalize_data_),
134135
finalize_cb(finalize_cb_),
@@ -148,12 +149,15 @@ class ThreadSafeFunction : public node::AsyncResource {
148149

149150
napi_status Push(void* data, napi_threadsafe_function_call_mode mode) {
150151
node::Mutex::ScopedLock lock(this->mutex);
152+
uv_thread_t current_thread = uv_thread_self();
151153

152154
while (queue.size() >= max_queue_size &&
153155
max_queue_size > 0 &&
154156
!is_closing) {
155157
if (mode == napi_tsfn_nonblocking) {
156158
return napi_queue_full;
159+
} else if (uv_thread_equal(&current_thread, &main_thread)) {
160+
return napi_would_deadlock;
157161
}
158162
cond->Wait(lock);
159163
}
@@ -434,6 +438,7 @@ class ThreadSafeFunction : public node::AsyncResource {
434438
// means we don't need the mutex to read them.
435439
void* context;
436440
size_t max_queue_size;
441+
uv_thread_t main_thread;
437442

438443
// These are variables accessed only from the loop thread.
439444
v8impl::Persistent<v8::Function> ref;
Collapse file

‎test/node-api/test_threadsafe_function/binding.c‎

Copy file name to clipboardExpand all lines: test/node-api/test_threadsafe_function/binding.c
+55Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,60 @@ static napi_value StartThreadNoJsFunc(napi_env env, napi_callback_info info) {
267267
/** block_on_full */true, /** alt_ref_js_cb */true);
268268
}
269269

270+
static void DeadlockTestDummyMarshaller(napi_env env,
271+
napi_value empty0,
272+
void* empty1,
273+
void* empty2) {}
274+
275+
static napi_value TestDeadlock(napi_env env, napi_callback_info info) {
276+
napi_threadsafe_function tsfn;
277+
napi_status status;
278+
napi_value async_name;
279+
napi_value return_value;
280+
281+
// Create an object to store the returned information.
282+
NAPI_CALL(env, napi_create_object(env, &return_value));
283+
284+
// Create a string to be used with the thread-safe function.
285+
NAPI_CALL(env, napi_create_string_utf8(env,
286+
"N-API Thread-safe Function Deadlock Test",
287+
NAPI_AUTO_LENGTH,
288+
&async_name));
289+
290+
// Create the thread-safe function with a single queue slot and a single thread.
291+
NAPI_CALL(env, napi_create_threadsafe_function(env,
292+
NULL,
293+
NULL,
294+
async_name,
295+
1,
296+
1,
297+
NULL,
298+
NULL,
299+
NULL,
300+
DeadlockTestDummyMarshaller,
301+
&tsfn));
302+
303+
// Call the threadsafe function. This should succeed and fill the queue.
304+
NAPI_CALL(env, napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_blocking));
305+
306+
// Call the threadsafe function. This should not block, but return
307+
// `napi_would_deadlock`. We save the resulting status in an object to be
308+
// returned.
309+
status = napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_blocking);
310+
add_returned_status(env,
311+
"deadlockTest",
312+
return_value,
313+
"Main thread would deadlock",
314+
napi_would_deadlock,
315+
status);
316+
317+
// Clean up the thread-safe function before returning.
318+
NAPI_CALL(env, napi_release_threadsafe_function(tsfn, napi_tsfn_release));
319+
320+
// Return the result.
321+
return return_value;
322+
}
323+
270324
// Module init
271325
static napi_value Init(napi_env env, napi_value exports) {
272326
size_t index;
@@ -305,6 +359,7 @@ static napi_value Init(napi_env env, napi_value exports) {
305359
DECLARE_NAPI_PROPERTY("StopThread", StopThread),
306360
DECLARE_NAPI_PROPERTY("Unref", Unref),
307361
DECLARE_NAPI_PROPERTY("Release", Release),
362+
DECLARE_NAPI_PROPERTY("TestDeadlock", TestDeadlock),
308363
};
309364

310365
NAPI_CALL(env, napi_define_properties(env, exports,
Collapse file

‎test/node-api/test_threadsafe_function/binding.gyp‎

Copy file name to clipboardExpand all lines: test/node-api/test_threadsafe_function/binding.gyp
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
'targets': [
33
{
44
'target_name': 'binding',
5-
'sources': ['binding.c']
5+
'sources': [
6+
'binding.c',
7+
'../../js-native-api/common.c'
8+
]
69
}
710
]
811
}
Collapse file

‎test/node-api/test_threadsafe_function/test.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_threadsafe_function/test.js
+8-3Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,13 @@ new Promise(function testWithoutJSMarshaller(resolve) {
210210
}))
211211
.then((result) => assert.strictEqual(result.indexOf(0), -1))
212212

213-
// Start a child process to test rapid teardown
213+
// Start a child process to test rapid teardown.
214214
.then(() => testUnref(binding.MAX_QUEUE_SIZE))
215215

216-
// Start a child process with an infinite queue to test rapid teardown
217-
.then(() => testUnref(0));
216+
// Start a child process with an infinite queue to test rapid teardown.
217+
.then(() => testUnref(0))
218+
219+
// Test deadlock prevention.
220+
.then(() => assert.deepStrictEqual(binding.TestDeadlock(), {
221+
deadlockTest: 'Main thread would deadlock'
222+
}));

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.