From 0eb969b13ee49122380b73f2e37525834ea6ca67 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Thu, 27 Jul 2017 20:59:29 +0300 Subject: [PATCH] n-api: add ability to remove a wrapping Calling napi_wrap() twice on the same object has the result of returning napi_invalid_arg on the second call. However, sometimes it is necessary to replace the native pointer associated with a given object. This new API allows one to remove an existing pointer, returning the object to its pristine, non-wrapped state. Fixes: https://github.com/nodejs/abi-stable-node/issues/266 --- doc/api/n-api.md | 28 ++++++- src/node_api.cc | 87 +++++++++++++++----- src/node_api.h | 3 + test/addons-napi/test_general/test.js | 11 ++- test/addons-napi/test_general/test_general.c | 13 +++ 5 files changed, 116 insertions(+), 26 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4397af96898c50..fa6035b1eae8ed 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. + ## 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 b84a33e510f264..bec98e07ce95d1 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -674,6 +674,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"; @@ -682,7 +684,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 { @@ -690,8 +693,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) { @@ -745,6 +751,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 @@ -2266,62 +2295,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 8e0740a68c45c6..26335dd3af2287 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -56,10 +56,17 @@ assert.strictEqual(release, process.release.name); // for null assert.strictEqual(test_general.testNapiTypeof(null), 'null'); -const x = {}; - // 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'); diff --git a/test/addons-napi/test_general/test_general.c b/test/addons-napi/test_general/test_general.c index da347bd68bcc9a..5c70a233a648c3 100644 --- a/test/addons-napi/test_general/test_general.c +++ b/test/addons-napi/test_general/test_general.c @@ -156,6 +156,18 @@ 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)); + NAPI_CALL(env, napi_delete_reference(env, (napi_ref)data)); + + return NULL; +} + void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_property_descriptor descriptors[] = { DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals), @@ -169,6 +181,7 @@ 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), }; NAPI_CALL_RETURN_VOID(env, napi_define_properties(