Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a0ccf94

Browse filesBrowse files
KevinEadyaduh95
KevinEady
authored and
aduh95
committed
node-api: execute tsfn finalizer after queue drains when aborted
A threadsafe function may utilize its context in its `call_js` callback. The context should be valid during draining of the queue when the threadsafe function is aborted. Fixes: #60026 PR-URL: #61956 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent fd3bf38 commit a0ccf94
Copy full SHA for a0ccf94

4 files changed

+112-2Lines changed: 112 additions & 2 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ class ThreadSafeFunction {
308308
return napi_ok;
309309
}
310310

311-
void EmptyQueueAndMaybeDelete() {
311+
void EmptyQueue() {
312312
std::queue<void*> drain_queue;
313313
{
314314
node::Mutex::ScopedLock lock(this->mutex);
@@ -317,6 +317,9 @@ class ThreadSafeFunction {
317317
for (; !drain_queue.empty(); drain_queue.pop()) {
318318
call_js_cb(nullptr, nullptr, context, drain_queue.front());
319319
}
320+
}
321+
322+
void MaybeDelete() {
320323
{
321324
node::Mutex::ScopedLock lock(this->mutex);
322325
if (thread_count > 0) {
@@ -464,11 +467,12 @@ class ThreadSafeFunction {
464467

465468
void Finalize() {
466469
v8::HandleScope scope(env->isolate);
470+
EmptyQueue();
467471
if (finalize_cb) {
468472
AsyncResource::CallbackScope cb_scope(&*async_resource);
469473
env->CallFinalizer<false>(finalize_cb, finalize_data, context);
470474
}
471-
EmptyQueueAndMaybeDelete();
475+
MaybeDelete();
472476
}
473477

474478
void CloseHandlesAndMaybeDelete(bool set_closing = false) {
Collapse file

‎test/node-api/test_threadsafe_function_abort/binding.cc‎

Copy file name to clipboard
+89Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
#include <js_native_api.h>
2+
#include <node_api.h>
3+
#include <node_api_types.h>
4+
5+
#include <assert.h>
6+
#include <cstdio>
7+
#include <cstdlib>
8+
#include <type_traits>
9+
#include <utility>
10+
11+
template <typename R, auto func, typename... Args>
12+
inline auto call(const char* name, Args&&... args) -> R {
13+
napi_status status;
14+
if constexpr (std::is_same_v<R, void>) {
15+
status = func(std::forward<Args>(args)...);
16+
if (status == napi_ok) {
17+
return;
18+
}
19+
} else {
20+
R ret;
21+
status = func(std::forward<Args>(args)..., &ret);
22+
if (status == napi_ok) {
23+
return ret;
24+
}
25+
}
26+
std::fprintf(stderr, "%s: %d\n", name, status);
27+
std::abort();
28+
}
29+
30+
#define NAPI_CALL(ret_type, func, ...) \
31+
call<ret_type, func>(#func, ##__VA_ARGS__)
32+
33+
class Context {
34+
public:
35+
enum class State { kCreated, kCalled } state = State::kCreated;
36+
};
37+
38+
void tsfn_callback(napi_env env,
39+
napi_value js_cb,
40+
void* ctx_p,
41+
void* /* data */) {
42+
auto ctx = static_cast<Context*>(ctx_p);
43+
assert(ctx->state == Context::State::kCreated);
44+
ctx->state = Context::State::kCalled;
45+
}
46+
47+
void tsfn_finalize(napi_env env,
48+
void* /* finalize_data */,
49+
void* finalize_hint) {
50+
auto ctx = static_cast<Context*>(finalize_hint);
51+
assert(ctx->state == Context::State::kCalled);
52+
delete ctx;
53+
}
54+
55+
auto run(napi_env env, napi_callback_info info) -> napi_value {
56+
auto global = NAPI_CALL(napi_value, napi_get_global, env);
57+
auto undefined = NAPI_CALL(napi_value, napi_get_undefined, env);
58+
auto ctx = new Context();
59+
auto tsfn = NAPI_CALL(napi_threadsafe_function,
60+
napi_create_threadsafe_function,
61+
env,
62+
nullptr,
63+
global,
64+
undefined,
65+
0,
66+
1 /* initial_thread_count */,
67+
nullptr,
68+
tsfn_finalize,
69+
ctx,
70+
tsfn_callback);
71+
72+
NAPI_CALL(
73+
void, napi_call_threadsafe_function, tsfn, nullptr, napi_tsfn_blocking);
74+
75+
NAPI_CALL(void, napi_unref_threadsafe_function, env, tsfn);
76+
77+
NAPI_CALL(void,
78+
napi_release_threadsafe_function,
79+
tsfn,
80+
napi_threadsafe_function_release_mode::napi_tsfn_abort);
81+
return NAPI_CALL(napi_value, napi_get_undefined, env);
82+
}
83+
84+
napi_value init(napi_env env, napi_value exports) {
85+
return NAPI_CALL(
86+
napi_value, napi_create_function, env, nullptr, 0, run, nullptr);
87+
}
88+
89+
NAPI_MODULE(NODE_GYP_MODULE_NAME, init)
Collapse file

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

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"targets": [
3+
{
4+
"target_name": "binding",
5+
"sources": ["binding.cc"],
6+
"cflags_cc": ["--std=c++20"],
7+
'cflags!': [ '-fno-exceptions', '-fno-rtti' ],
8+
'cflags_cc!': [ '-fno-exceptions', '-fno-rtti' ],
9+
}
10+
]
11+
}
Collapse file

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

Copy file name to clipboard
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
'use strict';
2+
3+
const common = require('../../common');
4+
const binding = require(`./build/${common.buildType}/binding`);
5+
6+
binding();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.