Commit e97c75e1548 for nodejs
commit e97c75e15482eba7a4208e68a763fc0a0fcc9ee9
Author: Chengzhong Wu <legendecas@gmail.com>
Date: Sat Oct 31 15:25:47 2026 -0400
node-api: fix crash on TSFN finalization reentrance at shutdown
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
PR-URL: https://github.com/nodejs/node/pull/65967
Fixes: https://github.com/nodejs/node/issues/65100
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
diff --git a/src/node_api.cc b/src/node_api.cc
index 40bdf143a98..b7854c2fe42 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -319,15 +319,22 @@ class ThreadSafeFunction {
}
}
+ // Runs on JS thread.
void MaybeDelete() {
+ CHECK_EQ(state, kClosing);
+ // Release the resources like napi_env reference and maybe call into
+ // user code. `state` must be `kClosing` when calling into user code here
+ // to avoid the `delete` below racing with re-entrant finalization.
+ ReleaseResources();
+
{
node::Mutex::ScopedLock lock(this->mutex);
+ // Mark the TSFN as ready to be deleted.
+ state = kClosed;
if (thread_count > 0) {
// At this point this TSFN is effectively done, but we need to keep
// it alive for other threads that still have pointers to it until
// they release them.
- // But we already release all the resources that we can at this point
- ReleaseResources();
return;
}
}
@@ -383,9 +390,10 @@ class ThreadSafeFunction {
inline void* Context() { return context; }
protected:
+ // This calls into user code via `env->Unref()`, which may trigger finalizers,
+ // and calls back into `napi_release_threadsafe_function`.
void ReleaseResources() {
if (state != kClosed) {
- state = kClosed;
ref.Reset();
node::RemoveEnvironmentCleanupHook(env->isolate, Cleanup, this);
env->Unref();
diff --git a/test/node-api/test_threadsafe_function_shutdown/binding.gyp b/test/node-api/test_threadsafe_function_shutdown/binding.gyp
index eb08b447a94..0cc8c234132 100644
--- a/test/node-api/test_threadsafe_function_shutdown/binding.gyp
+++ b/test/node-api/test_threadsafe_function_shutdown/binding.gyp
@@ -1,11 +1,19 @@
{
"targets": [
{
- "target_name": "binding",
- "sources": ["binding.cc"],
+ "target_name": "concurrent_calls",
+ "sources": ["concurrent_calls.cc"],
"cflags_cc": ["--std=c++20"],
'cflags!': [ '-fno-exceptions', '-fno-rtti' ],
'cflags_cc!': [ '-fno-exceptions', '-fno-rtti' ],
+ },
+ {
+ "target_name": "reentrant_release",
+ "sources": ["reentrant_release.c"]
+ },
+ {
+ "target_name": "multi_thread_count_release",
+ "sources": ["multi_thread_count_release.c"]
}
]
}
diff --git a/test/node-api/test_threadsafe_function_shutdown/binding.cc b/test/node-api/test_threadsafe_function_shutdown/concurrent_calls.cc
similarity index 100%
rename from test/node-api/test_threadsafe_function_shutdown/binding.cc
rename to test/node-api/test_threadsafe_function_shutdown/concurrent_calls.cc
diff --git a/test/node-api/test_threadsafe_function_shutdown/multi_thread_count_release.c b/test/node-api/test_threadsafe_function_shutdown/multi_thread_count_release.c
new file mode 100644
index 00000000000..4f20a7b0b2d
--- /dev/null
+++ b/test/node-api/test_threadsafe_function_shutdown/multi_thread_count_release.c
@@ -0,0 +1,69 @@
+#include <node_api.h>
+#include "../../js-native-api/common.h"
+
+#define THREAD_COUNT 3
+
+static void CallJs(napi_env env, napi_value cb, void* context, void* data) {
+ NODE_API_BASIC_ASSERT_RETURN_VOID(false, "The queue stays empty");
+}
+
+static void Finalize(napi_env env, void* data, void* hint) {
+ napi_ref callback_ref = data;
+ napi_value callback;
+ napi_value undefined;
+ NODE_API_CALL_RETURN_VOID(
+ env, napi_get_reference_value(env, callback_ref, &callback));
+ NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, callback_ref));
+ NODE_API_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined));
+ NODE_API_CALL_RETURN_VOID(
+ env, napi_call_function(env, undefined, callback, 0, NULL, NULL));
+}
+
+static napi_value Run(napi_env env, napi_callback_info info) {
+ size_t argc = 2;
+ napi_value args[2];
+ NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+ bool use_abort;
+ napi_ref callback_ref;
+ napi_threadsafe_function tsfn;
+ NODE_API_CALL(env, napi_get_value_bool(env, args[0], &use_abort));
+ NODE_API_CALL(env, napi_create_reference(env, args[1], 1, &callback_ref));
+ napi_value name;
+ NODE_API_CALL(env,
+ napi_create_string_utf8(
+ env, "tsfn_thread_count", NAPI_AUTO_LENGTH, &name));
+ NODE_API_CALL(env,
+ napi_create_threadsafe_function(env,
+ NULL,
+ NULL,
+ name,
+ 0,
+ THREAD_COUNT,
+ callback_ref,
+ Finalize,
+ NULL,
+ CallJs,
+ &tsfn));
+ if (use_abort) {
+ NODE_API_CALL(env, napi_release_threadsafe_function(tsfn, napi_tsfn_abort));
+ for (int i = 1; i < THREAD_COUNT; i++) {
+ napi_status status =
+ napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_nonblocking);
+ NODE_API_ASSERT(
+ env, status == napi_closing, "Call after abort returns napi_closing");
+ }
+ } else {
+ for (int i = 0; i < THREAD_COUNT; i++) {
+ NODE_API_CALL(env,
+ napi_release_threadsafe_function(tsfn, napi_tsfn_release));
+ }
+ }
+ return NULL;
+}
+
+NAPI_MODULE_INIT() {
+ napi_value run;
+ NODE_API_CALL(
+ env, napi_create_function(env, "run", NAPI_AUTO_LENGTH, Run, NULL, &run));
+ return run;
+}
diff --git a/test/node-api/test_threadsafe_function_shutdown/reentrant_release.c b/test/node-api/test_threadsafe_function_shutdown/reentrant_release.c
new file mode 100644
index 00000000000..2fd7fa1c1bb
--- /dev/null
+++ b/test/node-api/test_threadsafe_function_shutdown/reentrant_release.c
@@ -0,0 +1,82 @@
+#include <node_api.h>
+#include <stdlib.h>
+#include "../../js-native-api/common.h"
+
+typedef struct {
+ napi_threadsafe_function tsfn;
+} Holder;
+
+static void CallJs(napi_env env, napi_value js_cb, void* context, void* data) {
+ NODE_API_BASIC_ASSERT_RETURN_VOID(false, "The queue stays empty");
+}
+
+// Written on the worker thread while its napi_env tears down. Read on the main
+// thread after the worker thread is joined.
+static bool tsfn_finalized = false;
+static bool reentered = false;
+
+static void FinalizeTsfn(napi_env env, void* data, void* hint) {
+ tsfn_finalized = true;
+}
+
+static napi_value GetReentered(napi_env env, napi_callback_info info) {
+ napi_value result;
+ NODE_API_CALL(env, napi_get_boolean(env, reentered, &result));
+ return result;
+}
+
+// Runs while the napi_env tears down, reached from the thread-safe function
+// dropping its own napi_env reference. Releasing the thread-safe function
+// from here reenters TSFN finalization.
+static void FinalizeHolder(napi_env env, void* data, void* hint) {
+ Holder* holder = data;
+ reentered = tsfn_finalized;
+ napi_status status =
+ napi_release_threadsafe_function(holder->tsfn, napi_tsfn_abort);
+ if (status != napi_ok) {
+ abort();
+ }
+ free(holder);
+}
+
+NAPI_MODULE_INIT() {
+ napi_value name;
+ napi_value external;
+ napi_value getter;
+ Holder* holder = malloc(sizeof(*holder));
+
+ NODE_API_CALL(
+ env,
+ napi_create_string_utf8(env, "tsfn_teardown", NAPI_AUTO_LENGTH, &name));
+
+ // The initial thread count is never released, so the thread-safe function is
+ // still ref-ed by another thread when the environment tears down.
+ NODE_API_CALL(env,
+ napi_create_threadsafe_function(env,
+ NULL,
+ NULL,
+ name,
+ 0,
+ 1,
+ NULL,
+ FinalizeTsfn,
+ NULL,
+ CallJs,
+ &holder->tsfn));
+ // Allow the worker uv_loop to exit.
+ NODE_API_CALL(env, napi_unref_threadsafe_function(env, holder->tsfn));
+
+ // Held by the module exports, which the module cache keeps alive, so the
+ // napi_external finalizer runs during napi_env teardown.
+ NODE_API_CALL(
+ env, napi_create_external(env, holder, FinalizeHolder, NULL, &external));
+ NODE_API_CALL(env, napi_set_named_property(env, exports, "holder", external));
+ NODE_API_CALL(
+ env,
+ napi_create_function(
+ env, "reentered", NAPI_AUTO_LENGTH, GetReentered, NULL, &getter));
+ NODE_API_CALL(env,
+ napi_set_named_property(env, exports, "reentered", getter));
+
+ return exports;
+}
diff --git a/test/node-api/test_threadsafe_function_shutdown/test.js b/test/node-api/test_threadsafe_function_shutdown/test-concurrent-calls.js
similarity index 84%
rename from test/node-api/test_threadsafe_function_shutdown/test.js
rename to test/node-api/test_threadsafe_function_shutdown/test-concurrent-calls.js
index 9b2587b5bbd..154aceea991 100644
--- a/test/node-api/test_threadsafe_function_shutdown/test.js
+++ b/test/node-api/test_threadsafe_function_shutdown/test-concurrent-calls.js
@@ -4,7 +4,7 @@ const common = require('../../common');
const process = require('process');
const assert = require('assert');
const { fork } = require('child_process');
-const binding = require(`./build/${common.buildType}/binding`);
+const binding = require(`./build/${common.buildType}/concurrent_calls`);
if (process.argv[2] === 'child') {
binding();
diff --git a/test/node-api/test_threadsafe_function_shutdown/test-multi-thread-count-release.js b/test/node-api/test_threadsafe_function_shutdown/test-multi-thread-count-release.js
new file mode 100644
index 00000000000..3c309884adb
--- /dev/null
+++ b/test/node-api/test_threadsafe_function_shutdown/test-multi-thread-count-release.js
@@ -0,0 +1,11 @@
+'use strict';
+
+const common = require('../../common');
+const run = require(`./build/${common.buildType}/multi_thread_count_release`);
+
+// Ensures that the finalizer is called exactly once regardless of initial thread count
+// of a TSFN.
+// Also this verifies that multiple thread count does not cause re-entrant finalization.
+
+run(false, common.mustCall()); // Three releases.
+run(true, common.mustCall()); // One abort, then two calls returning napi_closing.
diff --git a/test/node-api/test_threadsafe_function_shutdown/test-reentrant-release.js b/test/node-api/test_threadsafe_function_shutdown/test-reentrant-release.js
new file mode 100644
index 00000000000..a8b6b8c092e
--- /dev/null
+++ b/test/node-api/test_threadsafe_function_shutdown/test-reentrant-release.js
@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const { Worker, isMainThread } = require('worker_threads');
+
+// A worker that exits while a native addon still owns a thread-safe function.
+// The TSFN finalizer could trigger napi_env finalization and the addon
+// may re-enter the TSFN finalization.
+// Refs: https://github.com/nodejs/node/issues/65100
+
+// Loading the addon on the main thread too keeps it, and its static state,
+// loaded after the worker unloads it.
+const binding = require(`./build/${common.buildType}/reentrant_release`);
+
+if (isMainThread) {
+ const worker = new Worker(__filename);
+ worker.on('error', common.mustNotCall());
+ worker.on('exit', common.mustCall((code) => {
+ assert.strictEqual(code, 0);
+ assert.strictEqual(binding.reentered(), true);
+ }));
+}