diff --git a/doc/api/n-api.md b/doc/api/n-api.md index c26a1222e19be2..be06a5ab3bed7a 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3152,7 +3152,9 @@ Afterward, additional manipulation of the wrapper's prototype chain may cause *Note*: Calling `napi_wrap()` a second time on an object that already has a native instance associated with it by virtue of a previous call to -`napi_wrap()` will cause an error to be returned. +`napi_wrap()` will cause an error to be returned. If you wish to associate +another native instance with the given object, call `napi_remove_wrap()` on it +first. ### *napi_unwrap* +```C +napi_status napi_remove_wrap(napi_env env, + napi_value js_object, + void** result); +``` + + - `[in] env`: The environment that the API is invoked under. + - `[in] js_object`: The object associated with the native instance. + - `[out] result`: Pointer to the wrapped native instance. + +Returns `napi_ok` if the API succeeded. + +Retrieves a native instance that was previously wrapped in the JavaScript +object `js_object` using `napi_wrap()` and removes the wrapping, thereby +restoring the JavaScript object's prototype chain. If a finalize callback was +associated with the wrapping, it will no longer be called when the JavaScript +object becomes garbage-collected. + ## Asynchronous Operations Addon modules often need to leverage async helpers from libuv as part of their diff --git a/src/node_api.cc b/src/node_api.cc index 2caa85aee5fc2c..db66ea0bab4550 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -679,6 +679,8 @@ v8::Local CreateAccessorCallbackData(napi_env env, return cbdata; } +int kWrapperFields = 3; + // Pointer used to identify items wrapped by N-API. Used by FindWrapper and // napi_wrap(). const char napi_wrap_name[] = "N-API Wrapper"; @@ -687,7 +689,8 @@ const char napi_wrap_name[] = "N-API Wrapper"; // wrapper would be the first in the chain, but it is OK for other objects to // be inserted in the prototype chain. bool FindWrapper(v8::Local obj, - v8::Local* result = nullptr) { + v8::Local* result = nullptr, + v8::Local* parent = nullptr) { v8::Local wrapper = obj; do { @@ -695,8 +698,11 @@ bool FindWrapper(v8::Local obj, if (proto.IsEmpty() || !proto->IsObject()) { return false; } + if (parent != nullptr) { + *parent = wrapper; + } wrapper = proto.As(); - if (wrapper->InternalFieldCount() == 2) { + if (wrapper->InternalFieldCount() == kWrapperFields) { v8::Local external = wrapper->GetInternalField(1); if (external->IsExternal() && external.As()->Value() == v8impl::napi_wrap_name) { @@ -750,6 +756,29 @@ napi_env GetEnv(v8::Local context) { return result; } +napi_status Unwrap(napi_env env, + napi_value js_object, + void** result, + v8::Local* wrapper, + v8::Local* parent = nullptr) { + CHECK_ARG(env, js_object); + CHECK_ARG(env, result); + + v8::Local value = v8impl::V8LocalValueFromJsValue(js_object); + RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg); + v8::Local obj = value.As(); + + RETURN_STATUS_IF_FALSE( + env, v8impl::FindWrapper(obj, wrapper, parent), napi_invalid_arg); + + v8::Local unwrappedValue = (*wrapper)->GetInternalField(0); + RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg); + + *result = unwrappedValue.As()->Value(); + + return napi_ok; +} + } // end of namespace v8impl // Intercepts the Node-V8 module registration callback. Converts parameters @@ -2269,62 +2298,78 @@ napi_status napi_wrap(napi_env env, // Create a wrapper object with an internal field to hold the wrapped pointer // and a second internal field to identify the owner as N-API. v8::Local wrapper_template; - ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 2); + ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, v8impl::kWrapperFields); auto maybe_object = wrapper_template->NewInstance(context); CHECK_MAYBE_EMPTY(env, maybe_object, napi_generic_failure); - v8::Local wrapper = maybe_object.ToLocalChecked(); - wrapper->SetInternalField(1, v8::External::New(isolate, - reinterpret_cast(const_cast(v8impl::napi_wrap_name)))); // Store the pointer as an external in the wrapper. wrapper->SetInternalField(0, v8::External::New(isolate, native_object)); + wrapper->SetInternalField(1, v8::External::New(isolate, + reinterpret_cast(const_cast(v8impl::napi_wrap_name)))); // Insert the wrapper into the object's prototype chain. v8::Local proto = obj->GetPrototype(); CHECK(wrapper->SetPrototype(context, proto).FromJust()); CHECK(obj->SetPrototype(context, wrapper).FromJust()); + 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); - v8impl::Reference* reference = v8impl::Reference::New( + reference = v8impl::Reference::New( env, obj, 0, false, finalize_cb, native_object, finalize_hint); *result = reinterpret_cast(reference); } else if (finalize_cb != nullptr) { // Create a self-deleting reference just for the finalize callback. - v8impl::Reference::New( + reference = v8impl::Reference::New( env, obj, 0, true, finalize_cb, native_object, finalize_hint); } + if (reference != nullptr) { + wrapper->SetInternalField(2, v8::External::New(isolate, reference)); + } + return GET_RETURN_STATUS(env); } -napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) { +napi_status napi_unwrap(napi_env env, napi_value obj, void** result) { // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw // JS exceptions. CHECK_ENV(env); - CHECK_ARG(env, js_object); - CHECK_ARG(env, result); - - v8::Local value = v8impl::V8LocalValueFromJsValue(js_object); - RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg); - v8::Local obj = value.As(); + v8::Local wrapper; + return napi_set_last_error(env, v8impl::Unwrap(env, obj, result, &wrapper)); +} +napi_status napi_remove_wrap(napi_env env, napi_value obj, void** result) { + NAPI_PREAMBLE(env); v8::Local wrapper; - RETURN_STATUS_IF_FALSE( - env, v8impl::FindWrapper(obj, &wrapper), napi_invalid_arg); + v8::Local parent; + napi_status status = v8impl::Unwrap(env, obj, result, &wrapper, &parent); + if (status != napi_ok) { + return napi_set_last_error(env, status); + } - v8::Local unwrappedValue = wrapper->GetInternalField(0); - RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg); + v8::Local external = wrapper->GetInternalField(2); + if (external->IsExternal()) { + v8impl::Reference::Delete( + static_cast(external.As()->Value())); + } - *result = unwrappedValue.As()->Value(); + if (!parent.IsEmpty()) { + v8::Maybe maybe = parent->SetPrototype( + env->isolate->GetCurrentContext(), wrapper->GetPrototype()); + CHECK_MAYBE_NOTHING(env, maybe, napi_generic_failure); + if (!maybe.FromMaybe(false)) { + return napi_set_last_error(env, napi_generic_failure); + } + } - return napi_clear_last_error(env); + return GET_RETURN_STATUS(env); } napi_status napi_create_external(napi_env env, diff --git a/src/node_api.h b/src/node_api.h index 0cf0ba046997d3..e52e2016d733b9 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -362,6 +362,9 @@ NAPI_EXTERN napi_status napi_wrap(napi_env env, NAPI_EXTERN napi_status napi_unwrap(napi_env env, napi_value js_object, void** result); +NAPI_EXTERN napi_status napi_remove_wrap(napi_env env, + napi_value js_object, + void** result); NAPI_EXTERN napi_status napi_create_external(napi_env env, void* data, napi_finalize finalize_cb, diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index 5dfdf6e8539c5a..10cf8b6e40132b 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -1,4 +1,5 @@ 'use strict'; +// Flags: --expose-gc const common = require('../../common'); const test_general = require(`./build/${common.buildType}/test_general`); @@ -56,10 +57,37 @@ assert.strictEqual(release, process.release.name); // for null assert.strictEqual(test_general.testNapiTypeof(null), 'null'); -const x = {}; +// Ensure that garbage collecting an object with a wrapped native item results +// in the finalize callback being called. +let w = {}; +test_general.wrap(w, []); +w = null; +global.gc(); +assert.strictEqual(test_general.derefItemWasCalled(), true, + 'deref_item() was called upon garbage collecting a ' + + 'wrapped object'); // Assert that wrapping twice fails. +const x = {}; test_general.wrap(x, 25); assert.throws(function() { test_general.wrap(x, 'Blah'); }, Error); + +// Ensure that wrapping, removing the wrap, and then wrapping again works. +const y = {}; +test_general.wrap(y, -12); +test_general.removeWrap(y); +assert.doesNotThrow(function() { + test_general.wrap(y, 're-wrap!'); +}, Error, 'Wrapping twice succeeds if a remove_wrap() separates the instances'); + +// Ensure that removing a wrap and garbage collecting does not fire the +// finalize callback. +let z = {}; +test_general.testFinalizeWrap(z); +test_general.removeWrap(z); +z = null; +global.gc(); +assert.strictEqual(test_general.finalizeWasCalled(), false, + 'finalize callback was not called upon garbage collection'); diff --git a/test/addons-napi/test_general/test_general.c b/test/addons-napi/test_general/test_general.c index da347bd68bcc9a..ecec3e014ba0b1 100644 --- a/test/addons-napi/test_general/test_general.c +++ b/test/addons-napi/test_general/test_general.c @@ -138,17 +138,29 @@ napi_value testNapiTypeof(napi_env env, napi_callback_info info) { return result; } +static bool deref_item_called = false; static void deref_item(napi_env env, void* data, void* hint) { (void) hint; + deref_item_called = true; NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, (napi_ref)data)); } +napi_value deref_item_was_called(napi_env env, napi_callback_info info) { + napi_value it_was_called; + + NAPI_CALL(env, napi_get_boolean(env, deref_item_called, &it_was_called)); + + return it_was_called; +} + napi_value wrap(napi_env env, napi_callback_info info) { size_t argc = 2; napi_value argv[2]; napi_ref payload; + deref_item_called = false; + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); NAPI_CALL(env, napi_create_reference(env, argv[1], 1, &payload)); NAPI_CALL(env, napi_wrap(env, argv[0], payload, deref_item, NULL, NULL)); @@ -156,6 +168,43 @@ napi_value wrap(napi_env env, napi_callback_info info) { return NULL; } +napi_value remove_wrap(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_remove_wrap(env, wrapped, &data)); + if (data != NULL) { + NAPI_CALL(env, napi_delete_reference(env, (napi_ref)data)); + } + + return NULL; +} + +static bool finalize_called = false; +static void test_finalize(napi_env env, void* data, void* hint) { + finalize_called = true; +} + +napi_value test_finalize_wrap(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value js_object; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &js_object, NULL, NULL)); + NAPI_CALL(env, napi_wrap(env, js_object, NULL, test_finalize, NULL, NULL)); + + return NULL; +} + +napi_value finalize_was_called(napi_env env, napi_callback_info info) { + napi_value it_was_called; + + NAPI_CALL(env, napi_get_boolean(env, finalize_called, &it_was_called)); + + return it_was_called; +} + void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_property_descriptor descriptors[] = { DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals), @@ -169,6 +218,10 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErrorCleanup), DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof), DECLARE_NAPI_PROPERTY("wrap", wrap), + DECLARE_NAPI_PROPERTY("removeWrap", remove_wrap), + DECLARE_NAPI_PROPERTY("testFinalizeWrap", test_finalize_wrap), + DECLARE_NAPI_PROPERTY("finalizeWasCalled", finalize_was_called), + DECLARE_NAPI_PROPERTY("derefItemWasCalled", deref_item_was_called), }; NAPI_CALL_RETURN_VOID(env, napi_define_properties(