From cf0e881b33c841d2f89a23be281e1aaf061a58a3 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 10 Aug 2018 11:43:39 -0400 Subject: [PATCH] n-api: add generic finalizer callback Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: https://github.com/nodejs/abi-stable-node/issues/313 PR-URL: https://github.com/nodejs/node/pull/22244 Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- doc/api/n-api.md | 58 +++++++++ src/node_api.cc | 112 ++++++++++++------ src/node_api.h | 6 + .../addons-napi/test_general/testFinalizer.js | 33 ++++++ test/addons-napi/test_general/test_general.c | 41 +++++++ 5 files changed, 215 insertions(+), 35 deletions(-) create mode 100644 test/addons-napi/test_general/testFinalizer.js diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4f27d8c5522..3bee50b7f6d 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3238,6 +3238,11 @@ JavaScript functions from native code. One can either call a function like a regular JavaScript function call, or as a constructor function. +Any non-`NULL` data which is passed to this API via the `data` field of the +`napi_property_descriptor` items can be associated with `object` and freed +whenever `object` is garbage-collected by passing both `object` and the data to +[`napi_add_finalizer`][]. + ### napi_call_function +```C +napi_status napi_add_finalizer(napi_env env, + napi_value js_object, + void* native_object, + napi_finalize finalize_cb, + void* finalize_hint, + napi_ref* result); +``` + + - `[in] env`: The environment that the API is invoked under. + - `[in] js_object`: The JavaScript object to which the native data will be + attached. + - `[in] native_object`: The native data that will be attached to the JavaScript + object. + - `[in] finalize_cb`: Native callback that will be used to free the + native data when the JavaScript object is ready for garbage-collection. + - `[in] finalize_hint`: Optional contextual hint that is passed to the + finalize callback. + - `[out] result`: Optional reference to the JavaScript object. + +Returns `napi_ok` if the API succeeded. + +Adds a `napi_finalize` callback which will be called when the JavaScript object +in `js_object` is ready for garbage collection. This API is similar to +`napi_wrap()` except that +* the native data cannot be retrieved later using `napi_unwrap()`, +* nor can it be removed later using `napi_remove_wrap()`, and +* the API can be called multiple times with different data items in order to + attach each of them to the JavaScript object. + +*Caution*: The optional returned reference (if obtained) should be deleted via +[`napi_delete_reference`][] ONLY in response to the finalize callback +invocation. If it is deleted before then, then the finalize callback may never +be invoked. Therefore, when obtaining a reference a finalize callback is also +required in order to enable correct disposal of the reference. + ## Simple Asynchronous Operations Addon modules often need to leverage async helpers from libuv as part of their @@ -4559,6 +4616,7 @@ This API may only be called from the main thread. [Working with JavaScript Values]: #n_api_working_with_javascript_values [Working with JavaScript Values - Abstract Operations]: #n_api_working_with_javascript_values_abstract_operations +[`napi_add_finalizer`]: #n_api_napi_add_finalizer [`napi_async_init`]: #n_api_napi_async_init [`napi_cancel_async_work`]: #n_api_napi_cancel_async_work [`napi_close_escapable_handle_scope`]: #n_api_napi_close_escapable_handle_scope diff --git a/src/node_api.cc b/src/node_api.cc index ef4ec29708c..690579f573b 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1157,6 +1157,63 @@ class ThreadSafeFunction : public node::AsyncResource { bool handles_closing; }; +enum WrapType { + retrievable, + anonymous +}; + +template static inline +napi_status Wrap(napi_env env, + napi_value js_object, + void* native_object, + napi_finalize finalize_cb, + void* finalize_hint, + napi_ref* result) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, js_object); + + v8::Isolate* isolate = env->isolate; + v8::Local context = isolate->GetCurrentContext(); + + v8::Local value = v8impl::V8LocalValueFromJsValue(js_object); + RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg); + v8::Local obj = value.As(); + + if (wrap_type == retrievable) { + // If we've already wrapped this object, we error out. + RETURN_STATUS_IF_FALSE(env, + !obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)) + .FromJust(), + napi_invalid_arg); + } else if (wrap_type == anonymous) { + // If no finalize callback is provided, we error out. + CHECK_ARG(env, finalize_cb); + } + + v8impl::Reference* reference = nullptr; + if (result != nullptr) { + // The returned reference should be deleted via napi_delete_reference() + // ONLY in response to the finalize callback invocation. (If it is deleted + // before then, then the finalize callback will never be invoked.) + // Therefore a finalize callback is required when returning a reference. + CHECK_ARG(env, finalize_cb); + reference = v8impl::Reference::New( + env, obj, 0, false, finalize_cb, native_object, finalize_hint); + *result = reinterpret_cast(reference); + } else { + // Create a self-deleting reference. + reference = v8impl::Reference::New(env, obj, 0, true, finalize_cb, + native_object, finalize_cb == nullptr ? nullptr : finalize_hint); + } + + if (wrap_type == retrievable) { + CHECK(obj->SetPrivate(context, NAPI_PRIVATE_KEY(context, wrapper), + v8::External::New(isolate, reference)).FromJust()); + } + + return GET_RETURN_STATUS(env); +} + } // end of namespace v8impl // Intercepts the Node-V8 module registration callback. Converts parameters @@ -2859,41 +2916,12 @@ napi_status napi_wrap(napi_env env, napi_finalize finalize_cb, void* finalize_hint, napi_ref* result) { - NAPI_PREAMBLE(env); - CHECK_ARG(env, js_object); - - v8::Isolate* isolate = env->isolate; - v8::Local context = isolate->GetCurrentContext(); - - v8::Local value = v8impl::V8LocalValueFromJsValue(js_object); - RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg); - v8::Local obj = value.As(); - - // If we've already wrapped this object, we error out. - RETURN_STATUS_IF_FALSE(env, - !obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).FromJust(), - napi_invalid_arg); - - v8impl::Reference* reference = nullptr; - if (result != nullptr) { - // The returned reference should be deleted via napi_delete_reference() - // ONLY in response to the finalize callback invocation. (If it is deleted - // before then, then the finalize callback will never be invoked.) - // Therefore a finalize callback is required when returning a reference. - CHECK_ARG(env, finalize_cb); - reference = v8impl::Reference::New( - env, obj, 0, false, finalize_cb, native_object, finalize_hint); - *result = reinterpret_cast(reference); - } else { - // Create a self-deleting reference. - reference = v8impl::Reference::New(env, obj, 0, true, finalize_cb, - native_object, finalize_cb == nullptr ? nullptr : finalize_hint); - } - - CHECK(obj->SetPrivate(context, NAPI_PRIVATE_KEY(context, wrapper), - v8::External::New(isolate, reference)).FromJust()); - - return GET_RETURN_STATUS(env); + return v8impl::Wrap(env, + js_object, + native_object, + finalize_cb, + finalize_hint, + result); } napi_status napi_unwrap(napi_env env, napi_value obj, void** result) { @@ -4138,3 +4166,17 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func) { CHECK(func != nullptr); return reinterpret_cast(func)->Ref(); } + +napi_status napi_add_finalizer(napi_env env, + napi_value js_object, + void* native_object, + napi_finalize finalize_cb, + void* finalize_hint, + napi_ref* result) { + return v8impl::Wrap(env, + js_object, + native_object, + finalize_cb, + finalize_hint, + result); +} diff --git a/src/node_api.h b/src/node_api.h index 1b2a392a0a0..10a2c8ff309 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -695,6 +695,12 @@ NAPI_EXTERN napi_status napi_get_value_bigint_words(napi_env env, int* sign_bit, size_t* word_count, uint64_t* words); +NAPI_EXTERN napi_status napi_add_finalizer(napi_env env, + napi_value js_object, + void* native_object, + napi_finalize finalize_cb, + void* finalize_hint, + napi_ref* result); #endif // NAPI_EXPERIMENTAL EXTERN_C_END diff --git a/test/addons-napi/test_general/testFinalizer.js b/test/addons-napi/test_general/testFinalizer.js new file mode 100644 index 00000000000..d5df4c8c724 --- /dev/null +++ b/test/addons-napi/test_general/testFinalizer.js @@ -0,0 +1,33 @@ +'use strict'; +// Flags: --expose-gc + +const common = require('../../common'); +const test_general = require(`./build/${common.buildType}/test_general`); +const assert = require('assert'); + +let finalized = {}; +const callback = common.mustCall(2); + +// Add two items to be finalized and ensure the callback is called for each. +test_general.addFinalizerOnly(finalized, callback); +test_general.addFinalizerOnly(finalized, callback); + +// Ensure attached items cannot be retrieved. +common.expectsError(() => test_general.unwrap(finalized), + { type: Error, message: 'Invalid argument' }); + +// Ensure attached items cannot be removed. +common.expectsError(() => test_general.removeWrap(finalized), + { type: Error, message: 'Invalid argument' }); +finalized = null; +global.gc(); + +// Add an item to an object that is already wrapped, and ensure that its +// finalizer as well as the wrap finalizer gets called. +let finalizeAndWrap = {}; +test_general.wrap(finalizeAndWrap); +test_general.addFinalizerOnly(finalizeAndWrap, common.mustCall()); +finalizeAndWrap = null; +global.gc(); +assert.strictEqual(test_general.derefItemWasCalled(), true, + 'finalize callback was called'); diff --git a/test/addons-napi/test_general/test_general.c b/test/addons-napi/test_general/test_general.c index 5a363d0b168..498aec4983c 100644 --- a/test/addons-napi/test_general/test_general.c +++ b/test/addons-napi/test_general/test_general.c @@ -1,3 +1,4 @@ +#define NAPI_EXPERIMENTAL #include #include #include "../common.h" @@ -177,6 +178,17 @@ static napi_value wrap(napi_env env, napi_callback_info info) { return NULL; } +static napi_value unwrap(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value wrapped; + void* data; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &wrapped, NULL, NULL)); + NAPI_CALL(env, napi_unwrap(env, wrapped, &data)); + + return NULL; +} + static napi_value remove_wrap(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value wrapped; @@ -232,6 +244,33 @@ static napi_value testNapiRun(napi_env env, napi_callback_info info) { return result; } +static void finalizer_only_callback(napi_env env, void* data, void* hint) { + napi_ref js_cb_ref = data; + napi_value js_cb, undefined; + NAPI_CALL_RETURN_VOID(env, napi_get_reference_value(env, js_cb_ref, &js_cb)); + NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined)); + NAPI_CALL_RETURN_VOID(env, + napi_call_function(env, undefined, js_cb, 0, NULL, NULL)); + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, js_cb_ref)); +} + +static napi_value add_finalizer_only(napi_env env, napi_callback_info info) { + size_t argc = 2; + napi_value argv[2]; + napi_ref js_cb_ref; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + NAPI_CALL(env, napi_create_reference(env, argv[1], 1, &js_cb_ref)); + NAPI_CALL(env, + napi_add_finalizer(env, + argv[0], + js_cb_ref, + finalizer_only_callback, + NULL, + NULL)); + return NULL; +} + static napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals), @@ -246,7 +285,9 @@ static napi_value Init(napi_env env, napi_value exports) { DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErrorCleanup), DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof), DECLARE_NAPI_PROPERTY("wrap", wrap), + DECLARE_NAPI_PROPERTY("unwrap", unwrap), DECLARE_NAPI_PROPERTY("removeWrap", remove_wrap), + DECLARE_NAPI_PROPERTY("addFinalizerOnly", add_finalizer_only), DECLARE_NAPI_PROPERTY("testFinalizeWrap", test_finalize_wrap), DECLARE_NAPI_PROPERTY("finalizeWasCalled", finalize_was_called), DECLARE_NAPI_PROPERTY("derefItemWasCalled", deref_item_was_called),