From d57a0eaae7b405e7eb0a19fcec65fafffd041300 Mon Sep 17 00:00:00 2001 From: Roy Wright Date: Thu, 18 Jul 2019 18:16:02 -0600 Subject: [PATCH] implement virutal ObjectWrap::Finalize Adds instance method `Finalize()` to `ObjectWrap()` which gets called immediately before the native instance is freed. It receives the `Napi::Env`, so classes that override it are able to perform cleanup that involves calls into N-API. Re: https://github.com/nodejs/node-addon-api/pull/515#issuecomment-513977222 Co-authored-by: Gabriel Schulhof Co-authored-by: Michael Dawson > PR-URL: https://github.com/nodejs/node-addon-api/pull/515 Reviewed-By: Gabriel Schulhof Reviewed-By: Michael Dawson --- doc/object_wrap.md | 11 +++++++++++ napi-inl.h | 9 ++++++++- napi.h | 2 ++ test/objectwrap.cc | 17 +++++++++++++++++ test/objectwrap.js | 19 +++++++++++++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) diff --git a/doc/object_wrap.md b/doc/object_wrap.md index 31be140..082c4ea 100644 --- a/doc/object_wrap.md +++ b/doc/object_wrap.md @@ -190,6 +190,17 @@ property of the `Napi::CallbackInfo`. Returns a `Napi::Function` representing the constructor function for the class. +### Finalize + +Provides an opportunity to run cleanup code that requires access to the `Napi::Env` +before the wrapped native object instance is freed. Override to implement. + +```cpp +virtual void Finalize(Napi::Env env); +``` + +- `[in] env`: `Napi::Env`. + ### StaticMethod Creates property descriptor that represents a static method of a JavaScript class. diff --git a/napi-inl.h b/napi-inl.h index 3fb4465..2f743a2 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2888,6 +2888,9 @@ inline ObjectWrap::ObjectWrap(const Napi::CallbackInfo& callbackInfo) { *instanceRef = Reference(env, ref); } +template +inline ObjectWrap::~ObjectWrap() {} + template inline T* ObjectWrap::Unwrap(Object wrapper) { T* unwrapped; @@ -3261,6 +3264,9 @@ inline ClassPropertyDescriptor ObjectWrap::InstanceValue( return desc; } +template +inline void ObjectWrap::Finalize(Napi::Env /*env*/) {} + template inline napi_value ObjectWrap::ConstructorCallbackWrapper( napi_env env, @@ -3402,8 +3408,9 @@ inline napi_value ObjectWrap::InstanceSetterCallbackWrapper( } template -inline void ObjectWrap::FinalizeCallback(napi_env /*env*/, void* data, void* /*hint*/) { +inline void ObjectWrap::FinalizeCallback(napi_env env, void* data, void* /*hint*/) { T* instance = reinterpret_cast(data); + instance->Finalize(Napi::Env(env)); delete instance; } diff --git a/napi.h b/napi.h index 7bbea19..b5d346b 100644 --- a/napi.h +++ b/napi.h @@ -1570,6 +1570,7 @@ namespace Napi { class ObjectWrap : public Reference { public: ObjectWrap(const CallbackInfo& callbackInfo); + virtual ~ObjectWrap(); static T* Unwrap(Object wrapper); @@ -1657,6 +1658,7 @@ namespace Napi { static PropertyDescriptor InstanceValue(Symbol name, Napi::Value value, napi_property_attributes attributes = napi_default); + virtual void Finalize(Napi::Env env); private: static napi_value ConstructorCallbackWrapper(napi_env env, napi_callback_info info); diff --git a/test/objectwrap.cc b/test/objectwrap.cc index ef1cfa7..4a89530 100644 --- a/test/objectwrap.cc +++ b/test/objectwrap.cc @@ -24,6 +24,11 @@ class Test : public Napi::ObjectWrap { public: Test(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { + + if(info.Length() > 0) { + finalizeCb_ = Napi::Persistent(info[0].As()); + } + } void Setter(const Napi::CallbackInfo& /*info*/, const Napi::Value& value) { @@ -105,8 +110,20 @@ class Test : public Napi::ObjectWrap { })); } + void Finalize(Napi::Env env) { + + if(finalizeCb_.IsEmpty()) { + return; + } + + finalizeCb_.Call(env.Global(), {Napi::Boolean::New(env, true)}); + finalizeCb_.Unref(); + + } + private: std::string value_; + Napi::FunctionReference finalizeCb_; }; Napi::Object InitObjectWrap(Napi::Env env) { diff --git a/test/objectwrap.js b/test/objectwrap.js index aa6d8d3..533c05f 100644 --- a/test/objectwrap.js +++ b/test/objectwrap.js @@ -182,6 +182,24 @@ const test = (binding) => { } }; + const testFinalize = (clazz) => { + + let finalizeCalled = false; + const finalizeCb = function(called) { + finalizeCalled = called; + }; + + //Scope Test instance so that it can be gc'd. + (function() { + new Test(finalizeCb); + })(); + + global.gc(); + + assert.strictEqual(finalizeCalled, true); + + }; + const testObj = (obj, clazz) => { testValue(obj, clazz); testAccessor(obj, clazz); @@ -198,6 +216,7 @@ const test = (binding) => { testStaticMethod(clazz); testStaticEnumerables(clazz); + testFinalize(clazz); }; // `Test` is needed for accessing exposed symbols