From ed7042cbb63d5b43be8e4b303baabf60bea81af0 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 13 Apr 2020 23:29:38 -0700 Subject: [PATCH] add hinted finalizer --- napi-inl.h | 23 ++++++++++++++++++++--- napi.h | 8 ++++++++ test/addon_data.cc | 26 +++++++++++++++++++++++--- test/addon_data.js | 38 ++++++++++++++++++++++---------------- 4 files changed, 73 insertions(+), 22 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index c07b4ca22..b29ac49f0 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -326,9 +326,21 @@ inline Value Env::RunScript(String script) { template fini> inline void Env::SetInstanceData(T* data) { napi_status status = - napi_set_instance_data(_env, data, [](napi_env env, void* data, void*) { - fini(env, static_cast(data)); - }, nullptr); + napi_set_instance_data(_env, data, [](napi_env env, void* data, void*) { + fini(env, static_cast(data)); + }, nullptr); + NAPI_THROW_IF_FAILED_VOID(_env, status); +} + +template fini> +inline void Env::SetInstanceData(DataType* data, HintType* hint) { + napi_status status = + napi_set_instance_data(_env, data, + [](napi_env env, void* data, void* hint) { + fini(env, static_cast(data), static_cast(hint)); + }, hint); NAPI_THROW_IF_FAILED_VOID(_env, status); } @@ -345,6 +357,11 @@ inline T* Env::GetInstanceData() { template void Env::DefaultFini(Env, T* data) { delete data; } + +template +void Env::DefaultFiniWithHint(Env, DataType* data, HintType*) { + delete data; +} #endif // NAPI_VERSION > 5 //////////////////////////////////////////////////////////////////////////////// diff --git a/napi.h b/napi.h index 59a02f51c..926a65fd4 100644 --- a/napi.h +++ b/napi.h @@ -200,6 +200,14 @@ namespace Napi { template using Finalizer = void (*)(Env, T*); template fini = Env::DefaultFini> void SetInstanceData(T* data); + + template + using FinalizerWithHint = void (*)(Env, DataType*, HintType*); + template fini = + Env::DefaultFiniWithHint> + void SetInstanceData(DataType* data, HintType* hint); #endif // NAPI_VERSION > 5 private: diff --git a/test/addon_data.cc b/test/addon_data.cc index 021f23f90..d160a5946 100644 --- a/test/addon_data.cc +++ b/test/addon_data.cc @@ -56,8 +56,22 @@ class Addon { } } - static Napi::Object Init(Napi::Env env) { - env.SetInstanceData(new Addon(env)); + static void DeleteAddon(Napi::Env, Addon* addon, uint32_t* hint) { + delete addon; + fprintf(stderr, "hint: %d\n", *hint); + delete hint; + } + + static Napi::Object Init(Napi::Env env, Napi::Value jshint) { + if (!jshint.IsNumber()) { + NAPI_THROW(Napi::Error::New(env, "Expected number"), Napi::Object()); + } + uint32_t hint = jshint.As(); + if (hint == 0) + env.SetInstanceData(new Addon(env)); + else + env.SetInstanceData(new Addon(env), + new uint32_t(hint)); Napi::Object result = Napi::Object::New(env); result.DefineProperties({ Napi::PropertyDescriptor::Accessor("verbose"), @@ -71,7 +85,13 @@ class Addon { Napi::FunctionReference VerboseIndicator; }; +// We use an addon factory so we can cover both the case where there is an +// instance data hint and the case where there isn't. +static Napi::Value AddonFactory(const Napi::CallbackInfo& info) { + return Addon::Init(info.Env(), info[0]); +} + Napi::Object InitAddonData(Napi::Env env) { - return Addon::Init(env); + return Napi::Function::New(env, AddonFactory); } #endif // (NAPI_VERSION > 5) diff --git a/test/addon_data.js b/test/addon_data.js index 24edcbb03..d363921ec 100644 --- a/test/addon_data.js +++ b/test/addon_data.js @@ -8,28 +8,34 @@ const path = require('path'); test(path.resolve(__dirname, `./build/${buildType}/binding.node`)); test(path.resolve(__dirname, `./build/${buildType}/binding_noexcept.node`)); -function test(bindingName) { - const binding = require(bindingName); - - // Make sure it is possible to get/set instance data. - assert.strictEqual(binding.addon_data.verbose.verbose, false); - binding.addon_data.verbose = true; - assert.strictEqual(binding.addon_data.verbose.verbose, true); - binding.addon_data.verbose = false; - assert.strictEqual(binding.addon_data.verbose.verbose, false); - - // Make sure the instance data finalizer is called at process exit. +// Make sure the instance data finalizer is called at process exit. If the hint +// is non-zero, it will be printed out by the child process. +function testFinalizer(bindingName, hint, expected) { const child = spawn(process.execPath, [ '-e', - `require('${bindingName}').addon_data.verbose = true;` + `require('${bindingName}').addon_data(${hint}).verbose = true;` ]); - let foundMessage = false; + const actual = []; readline .createInterface({ input: child.stderr }) .on('line', (line) => { - if (line.match('addon_data: Addon::~Addon')) { - foundMessage = true; + if (expected.indexOf(line) >= 0) { + actual.push(line); } }) - .on('close', () => assert.strictEqual(foundMessage, true)); + .on('close', () => assert.deepStrictEqual(expected, actual)); +} + +function test(bindingName) { + const binding = require(bindingName).addon_data(0); + + // Make sure it is possible to get/set instance data. + assert.strictEqual(binding.verbose.verbose, false); + binding.verbose = true; + assert.strictEqual(binding.verbose.verbose, true); + binding.verbose = false; + assert.strictEqual(binding.verbose.verbose, false); + + testFinalizer(bindingName, 0, ['addon_data: Addon::~Addon']); + testFinalizer(bindingName, 42, ['addon_data: Addon::~Addon', 'hint: 42']); }