Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
node-api: fix immediate napi_remove_wrap test
As documented in napi_wrap section, the returned reference must be
deleted with `napi_delete_reference` in response to the finalize
callback, in which `napi_unwrap` and `napi_remove_wrap` is not
available.

When the reference needs to be deleted early, it should be
deleted after the wrapped value is not accessed with `napi_unwrap`
and `napi_remove_wrap` too.

This test is previously added in response to duplicating the test
https://github.com/nodejs/node-addon-api/blob/main/test/objectwrap_constructor_exception.cc
in the node-addon-api. As Napi::ObjectWrap<> is a subclass of
Napi::Reference<>, napi_remove_wrap in the destructor of
Napi::ObjectWrap<> is called before napi_delete_reference in the
destructor of Napi::Reference<>.

PR-URL: #45406
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
  • Loading branch information
legendecas authored and ruyadorno committed Nov 21, 2022
1 parent 2b760c3 commit 8ff16fd
Showing 1 changed file with 8 additions and 1 deletion.
Expand Up @@ -44,6 +44,13 @@ static napi_value New(napi_env env, napi_callback_info info) {

static void NoopDeleter(napi_env env, void* data, void* hint) {}

// Tests that calling napi_remove_wrap and napi_delete_reference consecutively
// doesn't crash the process.
// This is analogous to the test https://github.com/nodejs/node-addon-api/blob/main/test/objectwrap_constructor_exception.cc.
// In which the Napi::ObjectWrap<> is being destructed immediately after napi_wrap.
// As Napi::ObjectWrap<> is a subclass of Napi::Reference<>, napi_remove_wrap
// in the destructor of Napi::ObjectWrap<> is called before napi_delete_reference
// in the destructor of Napi::Reference<>.
static napi_value DeleteImmediately(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value js_obj;
Expand All @@ -56,8 +63,8 @@ static napi_value DeleteImmediately(napi_env env, napi_callback_info info) {
NODE_API_ASSERT(env, type == napi_object, "Expected object parameter");

NODE_API_CALL(env, napi_wrap(env, js_obj, NULL, NoopDeleter, NULL, &ref));
NODE_API_CALL(env, napi_delete_reference(env, ref));
NODE_API_CALL(env, napi_remove_wrap(env, js_obj, NULL));
NODE_API_CALL(env, napi_delete_reference(env, ref));

return NULL;
}
Expand Down

0 comments on commit 8ff16fd

Please sign in to comment.