From c3c8814d2f17ea6da39ea720f5aaee778a822b75 Mon Sep 17 00:00:00 2001
From: Michael Price <mikepricedev@gmail.com>
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 <gabriel.schulhof@intel.com>
Co-authored-by: Michael Dawson <michael_dawson@ca.ibm.com>>
PR-URL: https://github.com/nodejs/node-addon-api/pull/515
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 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 31be140f0..082c4eabe 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 3fb4465bb..2f743a2d9 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -2888,6 +2888,9 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
   *instanceRef = Reference<Object>(env, ref);
 }
 
+template<typename T>
+inline ObjectWrap<T>::~ObjectWrap() {}
+
 template<typename T>
 inline T* ObjectWrap<T>::Unwrap(Object wrapper) {
   T* unwrapped;
@@ -3261,6 +3264,9 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::InstanceValue(
   return desc;
 }
 
+template <typename T>
+inline void ObjectWrap<T>::Finalize(Napi::Env /*env*/) {}
+
 template <typename T>
 inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(
     napi_env env,
@@ -3402,8 +3408,9 @@ inline napi_value ObjectWrap<T>::InstanceSetterCallbackWrapper(
 }
 
 template <typename T>
-inline void ObjectWrap<T>::FinalizeCallback(napi_env /*env*/, void* data, void* /*hint*/) {
+inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* /*hint*/) {
   T* instance = reinterpret_cast<T*>(data);
+  instance->Finalize(Napi::Env(env));
   delete instance;
 }
 
diff --git a/napi.h b/napi.h
index 7bbea198d..b5d346b89 100644
--- a/napi.h
+++ b/napi.h
@@ -1570,6 +1570,7 @@ namespace Napi {
   class ObjectWrap : public Reference<Object> {
   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 ef1cfa73e..4a89530e5 100644
--- a/test/objectwrap.cc
+++ b/test/objectwrap.cc
@@ -24,6 +24,11 @@ class Test : public Napi::ObjectWrap<Test> {
 public:
   Test(const Napi::CallbackInfo& info) :
     Napi::ObjectWrap<Test>(info) {
+
+    if(info.Length() > 0) {
+      finalizeCb_ = Napi::Persistent(info[0].As<Napi::Function>());
+    }
+
   }
 
   void Setter(const Napi::CallbackInfo& /*info*/, const Napi::Value& value) {
@@ -105,8 +110,20 @@ class Test : public Napi::ObjectWrap<Test> {
     }));
   }
 
+  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 aa6d8d3cb..533c05f75 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