From 1af6489a78581d16fae8dade23387a0fab39f14c Mon Sep 17 00:00:00 2001 From: Hitesh Kanwathirtha Date: Thu, 16 Mar 2017 08:17:12 +0000 Subject: [PATCH 1/2] 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 --- src/node_jsvmapi.cc | 44 +++++++++++++++---- src/node_jsvmapi.h | 4 ++ src/node_jsvmapi_types.h | 2 +- test/addons-abi/6_object_wrap/myobject.cc | 3 +- test/addons-abi/6_object_wrap/myobject.h | 2 +- test/addons-abi/7_factory_wrap/myobject.cc | 3 +- test/addons-abi/7_factory_wrap/myobject.h | 2 +- test/addons-abi/8_passing_wrapped/myobject.cc | 3 +- test/addons-abi/8_passing_wrapped/myobject.h | 2 +- test/addons-abi/test_buffer/test_buffer.cc | 12 +++-- .../test_typedarray/test_typedarray.cc | 7 ++- 11 files changed, 64 insertions(+), 20 deletions(-) diff --git a/src/node_jsvmapi.cc b/src/node_jsvmapi.cc index df0b8d0085..f17c26fec7 100644 --- a/src/node_jsvmapi.cc +++ b/src/node_jsvmapi.cc @@ -141,13 +141,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( @@ -209,7 +211,8 @@ class Reference { bool deleteSelf = reference->_deleteSelf; if (reference->_finalizeCallback != nullptr) { - reference->_finalizeCallback(reference->_finalizeData); + reference->_finalizeCallback(reference->_finalizeData, + reference->_finalizeHint); } if (deleteSelf) { @@ -223,6 +226,7 @@ class Reference { bool _deleteSelf; napi_finalize _finalizeCallback; void* _finalizeData; + void* _finalizeHint; }; class TryCatch : public v8::TryCatch { @@ -1733,6 +1737,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); @@ -1751,7 +1756,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) { @@ -1761,7 +1772,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(reference); } @@ -1789,6 +1800,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); @@ -1799,7 +1811,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); @@ -2126,6 +2144,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); @@ -2134,7 +2153,7 @@ napi_status napi_create_external_buffer(napi_env e, static_cast(data), size, (node::Buffer::FreeCallback)finalize_cb, - nullptr); + finalize_hint); CHECK_MAYBE_EMPTY(maybe, napi_generic_failure); @@ -2220,6 +2239,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); @@ -2231,7 +2251,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 index aaa7dbc25e..d632ba43ed 100644 --- a/src/node_jsvmapi.h +++ b/src/node_jsvmapi.h @@ -310,6 +310,7 @@ 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, @@ -317,6 +318,7 @@ NODE_EXTERN napi_status napi_unwrap(napi_env e, 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 v, @@ -393,6 +395,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, @@ -417,6 +420,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 index 6e88355eb3..79d641f8b4 100644 --- a/src/node_jsvmapi_types.h +++ b/src/node_jsvmapi_types.h @@ -15,7 +15,7 @@ typedef struct napi_propertyname__ *napi_propertyname; 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 index c633a421ec..ba559c9b6d 100644 --- 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(nativeObject)->~MyObject(); } @@ -70,6 +70,7 @@ void MyObject::New(napi_env env, napi_callback_info info) { jsthis, reinterpret_cast(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 index 7972ec1b19..5409aead7b 100644 --- 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 index 3e6652909c..0443163692 100644 --- 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(nativeObject)->~MyObject(); } @@ -56,6 +56,7 @@ void MyObject::New(napi_env env, napi_callback_info info) { jsthis, reinterpret_cast(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 index 9aec7d3c4a..fbadd7671b 100644 --- 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 index de1fb31b40..792eaabb99 100644 --- 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(nativeObject)->~MyObject(); } @@ -53,6 +53,7 @@ void MyObject::New(napi_env env, napi_callback_info info) { jsthis, reinterpret_cast(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 index 8af4450c4a..248b3e5dda 100644 --- 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 index a7ff0a4e2c..14c6872b48 100644 --- 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(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, // finalize_hint + nullptr, + &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(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 index 99ae9b879f..b7869c2a73 100644 --- 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; From 58f13cfaf71d862c1c47201517d5a65541f83119 Mon Sep 17 00:00:00 2001 From: Hitesh Kanwathirtha Date: Thu, 16 Mar 2017 21:04:24 +0000 Subject: [PATCH 2/2] Fix comment position --- test/addons-abi/test_buffer/test_buffer.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/addons-abi/test_buffer/test_buffer.cc b/test/addons-abi/test_buffer/test_buffer.cc index 14c6872b48..72f30c0623 100644 --- a/test/addons-abi/test_buffer/test_buffer.cc +++ b/test/addons-abi/test_buffer/test_buffer.cc @@ -55,8 +55,8 @@ void newExternalBuffer(napi_env env, napi_callback_info info) { env, sizeof(theText), theCopy, - deleteTheText, // finalize_hint - nullptr, + deleteTheText, + nullptr, // finalize_hint &theBuffer)); NAPI_CALL(env, napi_set_return_value(env, info, theBuffer)); }