From 8ff16fd8c0933696e5164449689a28d0efb1f06c Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 18 Nov 2022 17:31:25 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/45406 Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson --- .../test_reference_double_free.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/js-native-api/test_reference_double_free/test_reference_double_free.c b/test/js-native-api/test_reference_double_free/test_reference_double_free.c index 32f08c8e66adf1..f491d237fded3e 100644 --- a/test/js-native-api/test_reference_double_free/test_reference_double_free.c +++ b/test/js-native-api/test_reference_double_free/test_reference_double_free.c @@ -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; @@ -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; }