node-api: fix crash on re-entering TSFN finalization during env shutdown

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 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();

test/node-api/test_threadsafe_function_teardown/binding.c

@@ -0,0 +1,57 @@
+#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) {}
+
+// 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;
+  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;
+  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,
+                                                NULL,
+                                                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));
+
+  return exports;
+}

test/node-api/test_threadsafe_function_teardown/binding.gyp

@@ -0,0 +1,8 @@
+{
+  "targets": [
+    {
+      "target_name": "binding",
+      "sources": ["binding.c"]
+    }
+  ]
+}

test/node-api/test_threadsafe_function_teardown/test.js

@@ -0,0 +1,20 @@
+'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
+
+if (isMainThread) {
+  const worker = new Worker(__filename);
+  worker.on('error', common.mustNotCall());
+  worker.on('exit', common.mustCall((code) => {
+    assert.strictEqual(code, 0);
+  }));
+} else {
+  require(`./build/${common.buildType}/binding`);
+}