From 0b4f3a5b8c441f5380925e4c61606c17868050fb Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Thu, 29 Aug 2019 19:34:49 +0800
Subject: [PATCH] tsfn: fix crash on releasing tsfn

Refs: https://github.com/nodejs/node-addon-api/issues/531

PR-URL: https://github.com/nodejs/node-addon-api/pull/532
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 napi-inl.h                                    |  4 +--
 napi.h                                        |  7 ++++-
 test/binding.cc                               |  2 ++
 test/binding.gyp                              |  1 +
 test/index.js                                 |  2 ++
 .../threadsafe_function_ptr.cc                | 26 +++++++++++++++++++
 .../threadsafe_function_ptr.js                | 10 +++++++
 7 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 test/threadsafe_function/threadsafe_function_ptr.cc
 create mode 100644 test/threadsafe_function/threadsafe_function_ptr.js

diff --git a/napi-inl.h b/napi-inl.h
index 2f743a2d9..b7a29a092 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -3926,12 +3926,12 @@ inline ThreadSafeFunction ThreadSafeFunction::New(napi_env env,
 }
 
 inline ThreadSafeFunction::ThreadSafeFunction()
-  : _tsfn(new napi_threadsafe_function(nullptr)) {
+  : _tsfn(new napi_threadsafe_function(nullptr), _d) {
 }
 
 inline ThreadSafeFunction::ThreadSafeFunction(
     napi_threadsafe_function tsfn)
-  : _tsfn(new napi_threadsafe_function(tsfn)) {
+  : _tsfn(new napi_threadsafe_function(tsfn), _d) {
 }
 
 inline ThreadSafeFunction::ThreadSafeFunction(ThreadSafeFunction&& other)
diff --git a/napi.h b/napi.h
index b5d346b89..aa650de41 100644
--- a/napi.h
+++ b/napi.h
@@ -2029,8 +2029,13 @@ namespace Napi {
                        napi_value jsCallback,
                        void* context,
                        void* data);
+    struct Deleter {
+      // napi_threadsafe_function is managed by Node.js, leave it alone.
+      void operator()(napi_threadsafe_function*) const {};
+    };
 
-    std::unique_ptr<napi_threadsafe_function> _tsfn;
+    std::unique_ptr<napi_threadsafe_function, Deleter> _tsfn;
+    Deleter _d;
   };
   #endif
 
diff --git a/test/binding.cc b/test/binding.cc
index 141493e94..bf0f7c0f5 100644
--- a/test/binding.cc
+++ b/test/binding.cc
@@ -34,6 +34,7 @@ Object InitObjectDeprecated(Env env);
 #endif // !NODE_ADDON_API_DISABLE_DEPRECATED
 Object InitPromise(Env env);
 #if (NAPI_VERSION > 3)
+Object InitThreadSafeFunctionPtr(Env env);
 Object InitThreadSafeFunction(Env env);
 #endif
 Object InitTypedArray(Env env);
@@ -75,6 +76,7 @@ Object Init(Env env, Object exports) {
 #endif // !NODE_ADDON_API_DISABLE_DEPRECATED
   exports.Set("promise", InitPromise(env));
 #if (NAPI_VERSION > 3)
+  exports.Set("threadsafe_function_ptr", InitThreadSafeFunctionPtr(env));
   exports.Set("threadsafe_function", InitThreadSafeFunction(env));
 #endif
   exports.Set("typedarray", InitTypedArray(env));
diff --git a/test/binding.gyp b/test/binding.gyp
index 56d636adc..c8acb1ba3 100644
--- a/test/binding.gyp
+++ b/test/binding.gyp
@@ -32,6 +32,7 @@
         'object/object.cc',
         'object/set_property.cc',
         'promise.cc',
+        'threadsafe_function/threadsafe_function_ptr.cc',
         'threadsafe_function/threadsafe_function.cc',
         'typedarray.cc',
         'objectwrap.cc',
diff --git a/test/index.js b/test/index.js
index e8b26da80..7f97168c5 100644
--- a/test/index.js
+++ b/test/index.js
@@ -36,6 +36,7 @@ let testModules = [
   'object/object_deprecated',
   'object/set_property',
   'promise',
+  'threadsafe_function/threadsafe_function_ptr',
   'threadsafe_function/threadsafe_function',
   'typedarray',
   'typedarray-bigint',
@@ -64,6 +65,7 @@ if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
 
 if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
     (process.env.npm_config_NAPI_VERSION < 4)) {
+  testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_ptr'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function'), 1);
 }
 
diff --git a/test/threadsafe_function/threadsafe_function_ptr.cc b/test/threadsafe_function/threadsafe_function_ptr.cc
new file mode 100644
index 000000000..00e8559b8
--- /dev/null
+++ b/test/threadsafe_function/threadsafe_function_ptr.cc
@@ -0,0 +1,26 @@
+#include "napi.h"
+
+#if (NAPI_VERSION > 3)
+
+using namespace Napi;
+
+namespace {
+
+static Value Test(const CallbackInfo& info) {
+  Object resource = info[0].As<Object>();
+  Function cb = info[1].As<Function>();
+  ThreadSafeFunction tsfn = ThreadSafeFunction::New(info.Env(), cb, resource, "Test", 1, 1);
+  tsfn.Release();
+  return info.Env().Undefined();
+}
+
+}
+
+Object InitThreadSafeFunctionPtr(Env env) {
+  Object exports = Object::New(env);
+  exports["test"] = Function::New(env, Test);
+
+  return exports;
+}
+
+#endif
diff --git a/test/threadsafe_function/threadsafe_function_ptr.js b/test/threadsafe_function/threadsafe_function_ptr.js
new file mode 100644
index 000000000..535b5d642
--- /dev/null
+++ b/test/threadsafe_function/threadsafe_function_ptr.js
@@ -0,0 +1,10 @@
+'use strict';
+
+const buildType = process.config.target_defaults.default_configuration;
+
+test(require(`../build/${buildType}/binding.node`));
+test(require(`../build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  binding.threadsafe_function_ptr.test({}, () => {});
+}