napi: add finalize hint support (#155)

napi: add finalize hint support Change napi_finalize_callback to support a hint parameter APIs that take in a finalize callback now also take a hint This hint is passed back in when the finalize callback is called Fixes: #111 Reviewed-By: @jasongin @boingoing @mhdawson PR-Url: #155
nodejs · Mar 16, 2017 · 7597d8e · 7597d8e
1 parent 2791cf9
commit 7597d8e
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 20 deletions.
diff --git a/src/node_jsvmapi.cc b/src/node_jsvmapi.cc
@@ -119,13 +119,15 @@ class Reference {
             int initialRefcount,
             bool deleteSelf,
             napi_finalize finalizeCallback = nullptr,
-            void* finalizeData = nullptr)
-      : _isolate(isolate),
+            void* finalizeData = nullptr,
+            void* finalizeHint = nullptr)
+       : _isolate(isolate),
         _persistent(isolate, value),
         _refcount(initialRefcount),
         _deleteSelf(deleteSelf),
         _finalizeCallback(finalizeCallback),
-        _finalizeData(finalizeData) {
+        _finalizeData(finalizeData),
+        _finalizeHint(finalizeHint) {
     if (initialRefcount == 0) {
       if (_finalizeCallback != nullptr || _deleteSelf) {
         _persistent.SetWeak(
@@ -187,7 +189,8 @@ class Reference {
     bool deleteSelf = reference->_deleteSelf;
 
     if (reference->_finalizeCallback != nullptr) {
-      reference->_finalizeCallback(reference->_finalizeData);
+      reference->_finalizeCallback(reference->_finalizeData,
+          reference->_finalizeHint);
     }
 
     if (deleteSelf) {
@@ -201,6 +204,7 @@ class Reference {
   bool _deleteSelf;
   napi_finalize _finalizeCallback;
   void* _finalizeData;
+  void* _finalizeHint;
 };
 
 class TryCatch : public v8::TryCatch {
@@ -1782,6 +1786,7 @@ napi_status napi_wrap(napi_env e,
                       napi_value jsObject,
                       void* nativeObj,
                       napi_finalize finalize_cb,
+                      void* finalize_hint,
                       napi_ref* result) {
   NAPI_PREAMBLE(e);
   CHECK_ARG(jsObject);
@@ -1800,7 +1805,13 @@ napi_status napi_wrap(napi_env e,
     // Create a separate self-deleting reference for the finalizer callback.
     // This ensures the finalizer is not dependent on the lifetime of the
     // returned reference.
-    new v8impl::Reference(isolate, obj, 0, true, finalize_cb, nativeObj);
+    new v8impl::Reference(isolate,
+        obj,
+        0,
+        true,
+        finalize_cb,
+        nativeObj,
+        finalize_hint);
   }
 
   if (result != nullptr) {
@@ -1810,7 +1821,7 @@ napi_status napi_wrap(napi_env e,
     // deleted via
     // napi_delete_reference() when it is no longer needed.
     v8impl::Reference* reference =
-        new v8impl::Reference(isolate, obj, 0, false, nullptr, nullptr);
+        new v8impl::Reference(isolate, obj, 0, false);
     *result = reinterpret_cast<napi_ref>(reference);
   }
 
@@ -1838,6 +1849,7 @@ napi_status napi_unwrap(napi_env e, napi_value jsObject, void** result) {
 napi_status napi_create_external(napi_env e,
                                  void* data,
                                  napi_finalize finalize_cb,
+                                 void* finalize_hint,
                                  napi_value* result) {
   NAPI_PREAMBLE(e);
   CHECK_ARG(result);
@@ -1848,7 +1860,13 @@ napi_status napi_create_external(napi_env e,
 
   // The Reference object will delete itself after invoking the finalizer
   // callback.
-  new v8impl::Reference(isolate, externalValue, 0, true, finalize_cb, data);
+  new v8impl::Reference(isolate,
+      externalValue,
+      0,
+      true,
+      finalize_cb,
+      data,
+      finalize_hint);
 
   *result = v8impl::JsValueFromV8LocalValue(externalValue);
 
@@ -2177,6 +2195,7 @@ napi_status napi_create_external_buffer(napi_env e,
                                         size_t size,
                                         void* data,
                                         napi_finalize finalize_cb,
+                                        void* finalize_hint,
                                         napi_value* result) {
   NAPI_PREAMBLE(e);
   CHECK_ARG(result);
@@ -2185,7 +2204,7 @@ napi_status napi_create_external_buffer(napi_env e,
                                  static_cast<char*>(data),
                                  size,
                                  (node::Buffer::FreeCallback)finalize_cb,
-                                 nullptr);
+                                 finalize_hint);
 
   CHECK_MAYBE_EMPTY(maybe, napi_generic_failure);
 
@@ -2271,6 +2290,7 @@ napi_status napi_create_external_arraybuffer(napi_env e,
                                              void* external_data,
                                              size_t byte_length,
                                              napi_finalize finalize_cb,
+                                             void* finalize_hint,
                                              napi_value* result) {
   NAPI_PREAMBLE(e);
   CHECK_ARG(result);
@@ -2282,7 +2302,13 @@ napi_status napi_create_external_arraybuffer(napi_env e,
   if (finalize_cb != nullptr) {
     // Create a self-deleting weak reference that invokes the finalizer
     // callback.
-    new v8impl::Reference(isolate, buffer, 0, true, finalize_cb, external_data);
+    new v8impl::Reference(isolate,
+        buffer,
+        0,
+        true,
+        finalize_cb,
+        external_data,
+        finalize_hint);
   }
 
   *result = v8impl::JsValueFromV8LocalValue(buffer);

diff --git a/src/node_jsvmapi.h b/src/node_jsvmapi.h
@@ -321,13 +321,15 @@ NODE_EXTERN napi_status napi_wrap(napi_env e,
                                   napi_value jsObject,
                                   void* nativeObj,
                                   napi_finalize finalize_cb,
+                                  void* finalize_hint,
                                   napi_ref* result);
 NODE_EXTERN napi_status napi_unwrap(napi_env e,
                                     napi_value jsObject,
                                     void** result);
 NODE_EXTERN napi_status napi_create_external(napi_env e,
                                              void* data,
                                              napi_finalize finalize_cb,
+                                             void* finalize_hint,
                                              napi_value* result);
 NODE_EXTERN napi_status napi_get_value_external(napi_env e,
                                                 napi_value value,
@@ -408,6 +410,7 @@ NODE_EXTERN napi_status napi_create_external_buffer(napi_env e,
                                                     size_t size,
                                                     void* data,
                                                     napi_finalize finalize_cb,
+                                                    void* finalize_hint,
                                                     napi_value* result);
 NODE_EXTERN napi_status napi_create_buffer_copy(napi_env e,
                                                 const void* data,
@@ -434,6 +437,7 @@ napi_create_external_arraybuffer(napi_env env,
                                  void* external_data,
                                  size_t byte_length,
                                  napi_finalize finalize_cb,
+                                 void* finalize_hint,
                                  napi_value* result);
 NODE_EXTERN napi_status napi_get_arraybuffer_info(napi_env env,
                                                   napi_value arraybuffer,

diff --git a/src/node_jsvmapi_types.h b/src/node_jsvmapi_types.h
@@ -14,7 +14,7 @@ typedef struct napi_escapable_handle_scope__ *napi_escapable_handle_scope;
 typedef struct napi_callback_info__ *napi_callback_info;
 
 typedef void (*napi_callback)(napi_env, napi_callback_info);
-typedef void (*napi_finalize)(void* v);
+typedef void (*napi_finalize)(void* finalize_data, void* finalize_hint);
 
 enum napi_property_attributes {
   napi_default = 0,

diff --git a/test/addons-abi/6_object_wrap/myobject.cc b/test/addons-abi/6_object_wrap/myobject.cc
@@ -7,7 +7,7 @@ MyObject::MyObject(double value)
 
 MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
 
-void MyObject::Destructor(void* nativeObject) {
+void MyObject::Destructor(void* nativeObject, void* /*finalize_hint*/) {
   reinterpret_cast<MyObject*>(nativeObject)->~MyObject();
 }
 
@@ -66,6 +66,7 @@ void MyObject::New(napi_env env, napi_callback_info info) {
                        jsthis,
                        reinterpret_cast<void*>(obj),
                        MyObject::Destructor,
+                       nullptr,  // finalize_hint
                        &obj->wrapper_);
     if (status != napi_ok) return;
 

diff --git a/test/addons-abi/6_object_wrap/myobject.h b/test/addons-abi/6_object_wrap/myobject.h
@@ -6,7 +6,7 @@
 class MyObject {
  public:
   static void Init(napi_env env, napi_value exports);
-  static void Destructor(void* nativeObject);
+  static void Destructor(void* nativeObject, void* finalize_hint);
 
  private:
   explicit MyObject(double value_ = 0);

diff --git a/test/addons-abi/7_factory_wrap/myobject.cc b/test/addons-abi/7_factory_wrap/myobject.cc
@@ -4,7 +4,7 @@ MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {}
 
 MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
 
-void MyObject::Destructor(void* nativeObject) {
+void MyObject::Destructor(void* nativeObject, void* /*finalize_hint*/) {
   reinterpret_cast<MyObject*>(nativeObject)->~MyObject();
 }
 
@@ -56,6 +56,7 @@ void MyObject::New(napi_env env, napi_callback_info info) {
                      jsthis,
                      reinterpret_cast<void*>(obj),
                      MyObject::Destructor,
+                     nullptr, /* finalize_hint */
                      &obj->wrapper_);
   if (status != napi_ok) return;
 

diff --git a/test/addons-abi/7_factory_wrap/myobject.h b/test/addons-abi/7_factory_wrap/myobject.h
@@ -6,7 +6,7 @@
 class MyObject {
  public:
   static napi_status Init(napi_env env);
-  static void Destructor(void* nativeObject);
+  static void Destructor(void* nativeObject, void* /*finalize_hint*/);
   static napi_status NewInstance(napi_env env,
                                  napi_value arg,
                                  napi_value* instance);

diff --git a/test/addons-abi/8_passing_wrapped/myobject.cc b/test/addons-abi/8_passing_wrapped/myobject.cc
@@ -5,7 +5,7 @@ MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {}
 
 MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
 
-void MyObject::Destructor(void* nativeObject) {
+void MyObject::Destructor(void* nativeObject, void* /*finalize_hint*/) {
   reinterpret_cast<MyObject*>(nativeObject)->~MyObject();
 }
 
@@ -53,6 +53,7 @@ void MyObject::New(napi_env env, napi_callback_info info) {
                      jsthis,
                      reinterpret_cast<void*>(obj),
                      MyObject::Destructor,
+                     nullptr,  // finalize_hint
                      &obj->wrapper_);
   if (status != napi_ok) return;
 

diff --git a/test/addons-abi/8_passing_wrapped/myobject.h b/test/addons-abi/8_passing_wrapped/myobject.h
@@ -6,7 +6,7 @@
 class MyObject {
  public:
   static napi_status Init(napi_env env);
-  static void Destructor(void* nativeObject);
+  static void Destructor(void* nativeObject, void* finalize_hint);
   static napi_status NewInstance(napi_env env,
                                  napi_value arg,
                                  napi_value* instance);

diff --git a/test/addons-abi/test_buffer/test_buffer.cc b/test/addons-abi/test_buffer/test_buffer.cc
@@ -23,12 +23,12 @@ static const char theText[] =
     "Lorem ipsum dolor sit amet, consectetur adipiscing elit.";
 
 static int deleterCallCount = 0;
-static void deleteTheText(void *data) {
+static void deleteTheText(void *data, void* /*finalize_hint*/) {
   delete reinterpret_cast<char *>(data);
   deleterCallCount++;
 }
 
-static void noopDeleter(void *data) { deleterCallCount++; }
+static void noopDeleter(void *data, void* /*finalize_hint*/) { deleterCallCount++; }
 
 void newBuffer(napi_env env, napi_callback_info info) {
   napi_value theBuffer;
@@ -52,7 +52,12 @@ void newExternalBuffer(napi_env env, napi_callback_info info) {
   JS_ASSERT(env, theCopy, "Failed to copy static text for newExternalBuffer");
   NAPI_CALL(env,
             napi_create_external_buffer(
-                env, sizeof(theText), theCopy, deleteTheText, &theBuffer));
+                env,
+                sizeof(theText),
+                theCopy,
+                deleteTheText,
+                nullptr,  // finalize_hint
+                &theBuffer));
   NAPI_CALL(env, napi_set_return_value(env, info, theBuffer));
 }
 
@@ -116,6 +121,7 @@ void staticBuffer(napi_env env, napi_callback_info info) {
                                   sizeof(theText),
                                   const_cast<char *>(theText),
                                   noopDeleter,
+                                  nullptr,  // finalize_hint
                                   &theBuffer));
   NAPI_CALL(env, napi_set_return_value(env, info, theBuffer));
 }

diff --git a/test/addons-abi/test_typedarray/test_typedarray.cc b/test/addons-abi/test_typedarray/test_typedarray.cc
@@ -105,7 +105,12 @@ void External(napi_env env, napi_callback_info info) {
 
   napi_value output_buffer;
   napi_status status = napi_create_external_arraybuffer(
-      env, externalData, sizeof(externalData), nullptr, &output_buffer);
+      env,
+      externalData,
+      sizeof(externalData),
+      nullptr,  // finalize_callback
+      nullptr,  // finalize_hint
+      &output_buffer);
   if (status != napi_ok) return;
 
   napi_value output_array;