tsfn: add error checking on GetContext (#583)

PR-URL: nodejs/node-addon-api#583 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Marlyfleitas · Nov 18, 2019 · bb3abbe · bb3abbe
1 parent d572407
commit bb3abbe
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 1 deletion.
diff --git a/napi-inl.h b/napi-inl.h
@@ -4118,7 +4118,8 @@ inline napi_status ThreadSafeFunction::Abort() {
 inline ThreadSafeFunction::ConvertibleContext
 ThreadSafeFunction::GetContext() const {
   void* context;
-  napi_get_threadsafe_function_context(_tsfn, &context);
+  napi_status status = napi_get_threadsafe_function_context(_tsfn, &context);
+  NAPI_FATAL_IF_FAILED(status, "ThreadSafeFunction::GetContext", "napi_get_threadsafe_function_context");
   return ConvertibleContext({ context });
 }
 

diff --git a/test/binding.cc b/test/binding.cc
@@ -40,6 +40,7 @@ Object InitObjectDeprecated(Env env);
 #endif // !NODE_ADDON_API_DISABLE_DEPRECATED
 Object InitPromise(Env env);
 #if (NAPI_VERSION > 3)
+Object InitThreadSafeFunctionCtx(Env env);
 Object InitThreadSafeFunctionExistingTsfn(Env env);
 Object InitThreadSafeFunctionPtr(Env env);
 Object InitThreadSafeFunctionSum(Env env);
@@ -92,6 +93,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_ctx", InitThreadSafeFunctionCtx(env));
   exports.Set("threadsafe_function_existing_tsfn", InitThreadSafeFunctionExistingTsfn(env));
   exports.Set("threadsafe_function_ptr", InitThreadSafeFunctionPtr(env));
   exports.Set("threadsafe_function_sum", InitThreadSafeFunctionSum(env));

diff --git a/test/binding.gyp b/test/binding.gyp
@@ -36,6 +36,7 @@
         'object/object.cc',
         'object/set_property.cc',
         'promise.cc',
+        'threadsafe_function/threadsafe_function_ctx.cc',
         'threadsafe_function/threadsafe_function_existing_tsfn.cc',
         'threadsafe_function/threadsafe_function_ptr.cc',
         'threadsafe_function/threadsafe_function_sum.cc',

diff --git a/test/index.js b/test/index.js
@@ -39,6 +39,7 @@ let testModules = [
   'object/object_deprecated',
   'object/set_property',
   'promise',
+  'threadsafe_function/threadsafe_function_ctx',
   'threadsafe_function/threadsafe_function_existing_tsfn',
   'threadsafe_function/threadsafe_function_ptr',
   'threadsafe_function/threadsafe_function_sum',
@@ -69,6 +70,7 @@ if (napiVersion < 3) {
 
 if (napiVersion < 4) {
   testModules.splice(testModules.indexOf('asyncprogressworker'), 1);
+  testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_ctx'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_existing_tsfn'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_ptr'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_sum'), 1);

diff --git a/test/threadsafe_function/threadsafe_function_ctx.cc b/test/threadsafe_function/threadsafe_function_ctx.cc
@@ -0,0 +1,63 @@
+#include "napi.h"
+
+#if (NAPI_VERSION > 3)
+
+using namespace Napi;
+
+namespace {
+
+class TSFNWrap : public ObjectWrap<TSFNWrap> {
+public:
+  static Object Init(Napi::Env env, Object exports);
+  TSFNWrap(const CallbackInfo &info);
+
+  Napi::Value GetContext(const CallbackInfo & /*info*/) {
+    Reference<Napi::Value> *ctx = _tsfn.GetContext();
+    return ctx->Value();
+  };
+
+  Napi::Value Release(const CallbackInfo &info) {
+    Napi::Env env = info.Env();
+    _deferred = std::unique_ptr<Promise::Deferred>(new Promise::Deferred(env));
+    _tsfn.Release();
+    return _deferred->Promise();
+  };
+
+private:
+  ThreadSafeFunction _tsfn;
+  std::unique_ptr<Promise::Deferred> _deferred;
+};
+
+Object TSFNWrap::Init(Napi::Env env, Object exports) {
+  Function func =
+      DefineClass(env, "TSFNWrap",
+                  {InstanceMethod("getContext", &TSFNWrap::GetContext),
+                   InstanceMethod("release", &TSFNWrap::Release)});
+
+  exports.Set("TSFNWrap", func);
+  return exports;
+}
+
+TSFNWrap::TSFNWrap(const CallbackInfo &info) : ObjectWrap<TSFNWrap>(info) {
+  Napi::Env env = info.Env();
+
+  Reference<Napi::Value> *_ctx = new Reference<Napi::Value>;
+  *_ctx = Persistent(info[0]);
+
+  _tsfn = ThreadSafeFunction::New(
+      info.Env(), Function::New(env, [](const CallbackInfo & /*info*/) {}),
+      Object::New(env), "Test", 1, 1, _ctx,
+      [this](Napi::Env env, Reference<Napi::Value> *ctx) {
+        _deferred->Resolve(env.Undefined());
+        ctx->Reset();
+        delete ctx;
+      });
+}
+
+} // namespace
+
+Object InitThreadSafeFunctionCtx(Env env) {
+  return TSFNWrap::Init(env, Object::New(env));
+}
+
+#endif
diff --git a/test/threadsafe_function/threadsafe_function_ctx.js b/test/threadsafe_function/threadsafe_function_ctx.js
@@ -0,0 +1,16 @@
+'use strict';
+
+const assert = require('assert');
+const buildType = process.config.target_defaults.default_configuration;
+
+module.exports = Promise.all[
+  test(require(`../build/${buildType}/binding.node`)),
+  test(require(`../build/${buildType}/binding_noexcept.node`))
+];
+
+async function test(binding) {
+  const ctx = { };
+  const tsfn = new binding.threadsafe_function_ctx.TSFNWrap(ctx);
+  assert(tsfn.getContext() === ctx);
+  await tsfn.release();
+}