diff --git a/test/js-native-api/6_object_wrap/binding.gyp b/test/js-native-api/6_object_wrap/binding.gyp index 2be24c9ec171a9..49dd929fc747a0 100644 --- a/test/js-native-api/6_object_wrap/binding.gyp +++ b/test/js-native-api/6_object_wrap/binding.gyp @@ -1,17 +1,28 @@ { "targets": [ { - "target_name": "6_object_wrap", + "target_name": "myobject", "sources": [ - "6_object_wrap.cc" + "myobject.cc", + "myobject.h", ] }, { - "target_name": "6_object_wrap_basic_finalizer", + "target_name": "myobject_basic_finalizer", "defines": [ "NAPI_EXPERIMENTAL" ], "sources": [ - "6_object_wrap.cc" + "myobject.cc", + "myobject.h", ] - } + }, + { + "target_name": "nested_wrap", + # Test without basic finalizers as it schedules differently. + "defines": [ "NAPI_VERSION=10" ], + "sources": [ + "nested_wrap.cc", + "nested_wrap.h", + ], + }, ] } diff --git a/test/js-native-api/6_object_wrap/6_object_wrap.cc b/test/js-native-api/6_object_wrap/myobject.cc similarity index 83% rename from test/js-native-api/6_object_wrap/6_object_wrap.cc rename to test/js-native-api/6_object_wrap/myobject.cc index 8a380e3caa20bb..750d4f450a3cdd 100644 --- a/test/js-native-api/6_object_wrap/6_object_wrap.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -1,7 +1,7 @@ +#include "myobject.h" #include "../common.h" #include "../entry_point.h" #include "assert.h" -#include "myobject.h" typedef int32_t FinalizerData; @@ -10,7 +10,9 @@ napi_ref MyObject::constructor; MyObject::MyObject(double value) : value_(value), env_(nullptr), wrapper_(nullptr) {} -MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); } +MyObject::~MyObject() { + napi_delete_reference(env_, wrapper_); +} void MyObject::Destructor(node_api_basic_env env, void* nativeObject, @@ -26,24 +28,36 @@ void MyObject::Destructor(node_api_basic_env env, void MyObject::Init(napi_env env, napi_value exports) { napi_property_descriptor properties[] = { - { "value", nullptr, nullptr, GetValue, SetValue, 0, napi_default, 0 }, - { "valueReadonly", nullptr, nullptr, GetValue, nullptr, 0, napi_default, - 0 }, - DECLARE_NODE_API_PROPERTY("plusOne", PlusOne), - DECLARE_NODE_API_PROPERTY("multiply", Multiply), + {"value", nullptr, nullptr, GetValue, SetValue, 0, napi_default, 0}, + {"valueReadonly", + nullptr, + nullptr, + GetValue, + nullptr, + 0, + napi_default, + 0}, + DECLARE_NODE_API_PROPERTY("plusOne", PlusOne), + DECLARE_NODE_API_PROPERTY("multiply", Multiply), }; napi_value cons; - NODE_API_CALL_RETURN_VOID(env, napi_define_class( - env, "MyObject", -1, New, nullptr, - sizeof(properties) / sizeof(napi_property_descriptor), - properties, &cons)); + NODE_API_CALL_RETURN_VOID( + env, + napi_define_class(env, + "MyObject", + -1, + New, + nullptr, + sizeof(properties) / sizeof(napi_property_descriptor), + properties, + &cons)); NODE_API_CALL_RETURN_VOID(env, - napi_create_reference(env, cons, 1, &constructor)); + napi_create_reference(env, cons, 1, &constructor)); - NODE_API_CALL_RETURN_VOID(env, - napi_set_named_property(env, exports, "MyObject", cons)); + NODE_API_CALL_RETURN_VOID( + env, napi_set_named_property(env, exports, "MyObject", cons)); } napi_value MyObject::New(napi_env env, napi_callback_info info) { @@ -71,8 +85,12 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) { obj->env_ = env; NODE_API_CALL(env, - napi_wrap(env, _this, obj, MyObject::Destructor, - nullptr /* finalize_hint */, &obj->wrapper_)); + napi_wrap(env, + _this, + obj, + MyObject::Destructor, + nullptr /* finalize_hint */, + &obj->wrapper_)); return _this; } @@ -93,7 +111,7 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) { napi_value MyObject::GetValue(napi_env env, napi_callback_info info) { napi_value _this; NODE_API_CALL(env, - napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr)); + napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr)); MyObject* obj; NODE_API_CALL(env, napi_unwrap(env, _this, reinterpret_cast(&obj))); @@ -121,7 +139,7 @@ napi_value MyObject::SetValue(napi_env env, napi_callback_info info) { napi_value MyObject::PlusOne(napi_env env, napi_callback_info info) { napi_value _this; NODE_API_CALL(env, - napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr)); + napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr)); MyObject* obj; NODE_API_CALL(env, napi_unwrap(env, _this, reinterpret_cast(&obj))); diff --git a/test/js-native-api/6_object_wrap/nested_wrap.cc b/test/js-native-api/6_object_wrap/nested_wrap.cc new file mode 100644 index 00000000000000..f854a35658669a --- /dev/null +++ b/test/js-native-api/6_object_wrap/nested_wrap.cc @@ -0,0 +1,99 @@ +#include "nested_wrap.h" +#include "../common.h" +#include "../entry_point.h" + +napi_ref NestedWrap::constructor{}; +static int finalization_count = 0; + +NestedWrap::NestedWrap() {} + +NestedWrap::~NestedWrap() { + napi_delete_reference(env_, wrapper_); + + // Delete the nested reference as well. + napi_delete_reference(env_, nested_); +} + +void NestedWrap::Destructor(node_api_basic_env env, + void* nativeObject, + void* /*finalize_hint*/) { + // Once this destructor is called, it cancels all pending + // finalizers for the object by deleting the references. + NestedWrap* obj = static_cast(nativeObject); + delete obj; + + finalization_count++; +} + +void NestedWrap::Init(napi_env env, napi_value exports) { + napi_value cons; + NODE_API_CALL_RETURN_VOID( + env, + napi_define_class( + env, "NestedWrap", -1, New, nullptr, 0, nullptr, &cons)); + + NODE_API_CALL_RETURN_VOID(env, + napi_create_reference(env, cons, 1, &constructor)); + + NODE_API_CALL_RETURN_VOID( + env, napi_set_named_property(env, exports, "NestedWrap", cons)); +} + +napi_value NestedWrap::New(napi_env env, napi_callback_info info) { + napi_value new_target; + NODE_API_CALL(env, napi_get_new_target(env, info, &new_target)); + bool is_constructor = (new_target != nullptr); + NODE_API_BASIC_ASSERT_BASE( + is_constructor, "Constructor called without new", nullptr); + + napi_value this_val; + NODE_API_CALL(env, + napi_get_cb_info(env, info, 0, nullptr, &this_val, nullptr)); + + NestedWrap* obj = new NestedWrap(); + + obj->env_ = env; + NODE_API_CALL(env, + napi_wrap(env, + this_val, + obj, + NestedWrap::Destructor, + nullptr /* finalize_hint */, + &obj->wrapper_)); + + // Create a second napi_ref to be deleted in the destructor. + NODE_API_CALL(env, + napi_add_finalizer(env, + this_val, + obj, + NestedWrap::Destructor, + nullptr /* finalize_hint */, + &obj->nested_)); + + return this_val; +} + +static napi_value GetFinalizerCallCount(napi_env env, napi_callback_info info) { + napi_value result; + NODE_API_CALL(env, napi_create_int32(env, finalization_count, &result)); + return result; +} + +EXTERN_C_START +napi_value Init(napi_env env, napi_value exports) { + NestedWrap::Init(env, exports); + + napi_property_descriptor descriptors[] = { + DECLARE_NODE_API_PROPERTY("getFinalizerCallCount", GetFinalizerCallCount), + }; + + NODE_API_CALL( + env, + napi_define_properties(env, + exports, + sizeof(descriptors) / sizeof(*descriptors), + descriptors)); + + return exports; +} +EXTERN_C_END diff --git a/test/js-native-api/6_object_wrap/nested_wrap.h b/test/js-native-api/6_object_wrap/nested_wrap.h new file mode 100644 index 00000000000000..7e7729c3375c9a --- /dev/null +++ b/test/js-native-api/6_object_wrap/nested_wrap.h @@ -0,0 +1,33 @@ +#ifndef TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_ +#define TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_ + +#include + +/** + * Test that an napi_ref can be nested inside another ObjectWrap. + * + * This test shows a critical case where a finalizer deletes an napi_ref + * whose finalizer is also scheduled. + */ + +class NestedWrap { + public: + static void Init(napi_env env, napi_value exports); + static void Destructor(node_api_basic_env env, + void* nativeObject, + void* finalize_hint); + + private: + explicit NestedWrap(); + ~NestedWrap(); + + static napi_value New(napi_env env, napi_callback_info info); + + static napi_ref constructor; + + napi_env env_{}; + napi_ref wrapper_{}; + napi_ref nested_{}; +}; + +#endif // TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_ diff --git a/test/js-native-api/6_object_wrap/nested_wrap.js b/test/js-native-api/6_object_wrap/nested_wrap.js new file mode 100644 index 00000000000000..2d99b54c7d2754 --- /dev/null +++ b/test/js-native-api/6_object_wrap/nested_wrap.js @@ -0,0 +1,20 @@ +// Flags: --expose-gc + +'use strict'; +const common = require('../../common'); +const { gcUntil } = require('../../common/gc'); +const assert = require('assert'); +const addon = require(`./build/${common.buildType}/nested_wrap`); + +// This test verifies that ObjectWrap and napi_ref can be nested and finalized +// correctly with a non-basic finalizer. +(() => { + let obj = new addon.NestedWrap(); + obj = null; + // Silent eslint about unused variables. + assert.strictEqual(obj, null); +})(); + +gcUntil('object-wrap-ref', () => { + return addon.getFinalizerCallCount() === 1; +}); diff --git a/test/js-native-api/6_object_wrap/test-basic-finalizer.js b/test/js-native-api/6_object_wrap/test-basic-finalizer.js index 46b5672c5fa4b9..631c1cb96a50eb 100644 --- a/test/js-native-api/6_object_wrap/test-basic-finalizer.js +++ b/test/js-native-api/6_object_wrap/test-basic-finalizer.js @@ -3,7 +3,7 @@ 'use strict'; const common = require('../../common'); const assert = require('assert'); -const addon = require(`./build/${common.buildType}/6_object_wrap_basic_finalizer`); +const addon = require(`./build/${common.buildType}/myobject_basic_finalizer`); // This test verifies that ObjectWrap can be correctly finalized with a node_api_basic_finalizer // in the current JS loop tick diff --git a/test/js-native-api/6_object_wrap/test-object-wrap-ref.js b/test/js-native-api/6_object_wrap/test-object-wrap-ref.js index a7d866a6869515..874c1304b26867 100644 --- a/test/js-native-api/6_object_wrap/test-object-wrap-ref.js +++ b/test/js-native-api/6_object_wrap/test-object-wrap-ref.js @@ -2,7 +2,7 @@ 'use strict'; const common = require('../../common'); -const addon = require(`./build/${common.buildType}/6_object_wrap`); +const addon = require(`./build/${common.buildType}/myobject`); const { gcUntil } = require('../../common/gc'); (function scope() { diff --git a/test/js-native-api/6_object_wrap/test.js b/test/js-native-api/6_object_wrap/test.js index c113a47299f95c..e30289e9bd437b 100644 --- a/test/js-native-api/6_object_wrap/test.js +++ b/test/js-native-api/6_object_wrap/test.js @@ -1,7 +1,7 @@ 'use strict'; const common = require('../../common'); const assert = require('assert'); -const addon = require(`./build/${common.buildType}/6_object_wrap`); +const addon = require(`./build/${common.buildType}/myobject`); const getterOnlyErrorRE = /^TypeError: Cannot set property .* of #<.*> which has only a getter$/;