From fca1a54efeff167d82748b5bf474a59eb32a1447 Mon Sep 17 00:00:00 2001 From: Kenvin Davies Date: Fri, 26 Feb 2021 19:45:00 +0800 Subject: [PATCH] src: allow references to be copyable in APIs allow non-copyable callbacks be finalizer parameters Fixes: https://github.com/nodejs/node-addon-api/issues/301 PR-URL: https://github.com/nodejs/node-addon-api/pull/915 Reviewed-By: Michael Dawson --- napi-inl.h | 24 ++++++++++++++++-------- test/binding.cc | 2 ++ test/binding.gyp | 1 + test/movable_callbacks.cc | 23 +++++++++++++++++++++++ test/movable_callbacks.js | 27 +++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 test/movable_callbacks.cc create mode 100644 test/movable_callbacks.js diff --git a/napi-inl.h b/napi-inl.h index 41c7638..b12ec4e 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1345,7 +1345,8 @@ inline bool Object::InstanceOf(const Function& constructor) const { template inline void Object::AddFinalizer(Finalizer finalizeCallback, T* data) { details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, nullptr }); + new details::FinalizeData( + {std::move(finalizeCallback), nullptr}); napi_status status = details::AttachData(_env, *this, @@ -1363,7 +1364,8 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback, T* data, Hint* finalizeHint) { details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, finalizeHint }); + new details::FinalizeData( + {std::move(finalizeCallback), finalizeHint}); napi_status status = details::AttachData(_env, *this, @@ -1395,7 +1397,8 @@ inline External External::New(napi_env env, Finalizer finalizeCallback) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, nullptr }); + new details::FinalizeData( + {std::move(finalizeCallback), nullptr}); napi_status status = napi_create_external( env, data, @@ -1417,7 +1420,8 @@ inline External External::New(napi_env env, Hint* finalizeHint) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, finalizeHint }); + new details::FinalizeData( + {std::move(finalizeCallback), finalizeHint}); napi_status status = napi_create_external( env, data, @@ -1509,7 +1513,8 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env, Finalizer finalizeCallback) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, nullptr }); + new details::FinalizeData( + {std::move(finalizeCallback), nullptr}); napi_status status = napi_create_external_arraybuffer( env, externalData, @@ -1533,7 +1538,8 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env, Hint* finalizeHint) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, finalizeHint }); + new details::FinalizeData( + {std::move(finalizeCallback), finalizeHint}); napi_status status = napi_create_external_arraybuffer( env, externalData, @@ -2153,7 +2159,8 @@ inline Buffer Buffer::New(napi_env env, Finalizer finalizeCallback) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, nullptr }); + new details::FinalizeData( + {std::move(finalizeCallback), nullptr}); napi_status status = napi_create_external_buffer( env, length * sizeof (T), @@ -2177,7 +2184,8 @@ inline Buffer Buffer::New(napi_env env, Hint* finalizeHint) { napi_value value; details::FinalizeData* finalizeData = - new details::FinalizeData({ finalizeCallback, finalizeHint }); + new details::FinalizeData( + {std::move(finalizeCallback), finalizeHint}); napi_status status = napi_create_external_buffer( env, length * sizeof (T), diff --git a/test/binding.cc b/test/binding.cc index 03661da..fa37a8d 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -34,6 +34,7 @@ Object InitError(Env env); Object InitExternal(Env env); Object InitFunction(Env env); Object InitHandleScope(Env env); +Object InitMovableCallbacks(Env env); Object InitMemoryManagement(Env env); Object InitName(Env env); Object InitObject(Env env); @@ -101,6 +102,7 @@ Object Init(Env env, Object exports) { exports.Set("function", InitFunction(env)); exports.Set("name", InitName(env)); exports.Set("handlescope", InitHandleScope(env)); + exports.Set("movable_callbacks", InitMovableCallbacks(env)); exports.Set("memory_management", InitMemoryManagement(env)); exports.Set("object", InitObject(env)); #ifndef NODE_ADDON_API_DISABLE_DEPRECATED diff --git a/test/binding.gyp b/test/binding.gyp index 90dee45..fc5b5c2 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -25,6 +25,7 @@ 'external.cc', 'function.cc', 'handlescope.cc', + 'movable_callbacks.cc', 'memory_management.cc', 'name.cc', 'object/delete_property.cc', diff --git a/test/movable_callbacks.cc b/test/movable_callbacks.cc new file mode 100644 index 0000000..9959d4d --- /dev/null +++ b/test/movable_callbacks.cc @@ -0,0 +1,23 @@ +#include "napi.h" + +using namespace Napi; + +Value createExternal(const CallbackInfo& info) { + FunctionReference ref = Reference::New(info[0].As(), 1); + auto ret = External::New( + info.Env(), + nullptr, + [ref = std::move(ref)](Napi::Env /*env*/, char* /*data*/) { + ref.Call({}); + }); + + return ret; +} + +Object InitMovableCallbacks(Env env) { + Object exports = Object::New(env); + + exports["createExternal"] = Function::New(env, createExternal); + + return exports; +} diff --git a/test/movable_callbacks.js b/test/movable_callbacks.js new file mode 100644 index 0000000..49810d5 --- /dev/null +++ b/test/movable_callbacks.js @@ -0,0 +1,27 @@ +'use strict'; +const buildType = process.config.target_defaults.default_configuration; +const common = require('./common'); +const testUtil = require('./testUtil'); + +Promise.all([ + test(require(`./build/${buildType}/binding.node`).movable_callbacks), + test(require(`./build/${buildType}/binding_noexcept.node`).movable_callbacks), +]).catch(e => { + console.error(e); + process.exitCode = 1; +}); + +async function test(binding) { + await testUtil.runGCTests([ + 'External', + () => { + const fn = common.mustCall(() => { + // noop + }, 1); + const external = binding.createExternal(fn); + }, + () => { + // noop, wait for gc + } + ]); +}