From 82177ad69634983b2cafb4dea88d2fa042bc4365 Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Sat, 30 Aug 2025 10:14:39 +0530 Subject: [PATCH 1/6] node-api: make napi_delete_reference use node_api_basic_env --- src/js_native_api.h | 2 +- src/js_native_api_v8.cc | 6 +++++- test/cctest/test_linked_binding.cc | 2 +- test/js-native-api/6_object_wrap/myobject.cc | 4 ++-- test/js-native-api/6_object_wrap/nested_wrap.cc | 4 ++-- test/js-native-api/7_factory_wrap/myobject.cc | 2 +- test/js-native-api/8_passing_wrapped/myobject.cc | 2 +- test/node-api/test_uv_loop/test_uv_loop.cc | 2 +- 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/js_native_api.h b/src/js_native_api.h index 8177ace3acbb89..6b53a2c28de461 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -364,7 +364,7 @@ napi_create_reference(napi_env env, // Deletes a reference. The referenced value is released, and may // be GC'd unless there are other references to it. -NAPI_EXTERN napi_status NAPI_CDECL napi_delete_reference(napi_env env, +NAPI_EXTERN napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, napi_ref ref); // Increments the reference count, optionally returning the resulting count. diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 260a572ce71a82..64186c3114367c 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2773,7 +2773,11 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env, // there are other references to it. // For a napi_reference returned from `napi_wrap`, this must be called in the // finalizer. -napi_status NAPI_CDECL napi_delete_reference(napi_env env, napi_ref ref) { +// Deletes a reference. The referenced value is released, and may be GC'd unless +// there are other references to it. +// For a napi_reference returned from `napi_wrap`, this must be called in the +// finalizer. +napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, napi_ref ref) { // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw // JS exceptions. CHECK_ENV(env); diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 19ea2b8f228ebb..3f1c60e560b1ad 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -171,7 +171,7 @@ napi_value InitializeLocalNapiRefBinding(napi_env env, napi_value exports) { napi_ref ref{}; if (node_api_version == NAPI_VERSION_EXPERIMENTAL) { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_ok); - CHECK_EQ(napi_delete_reference(env, ref), napi_ok); + CHECK_EQ(napi_delete_reference(static_cast(env), ref), napi_ok); } else { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_invalid_arg); } diff --git a/test/js-native-api/6_object_wrap/myobject.cc b/test/js-native-api/6_object_wrap/myobject.cc index 750d4f450a3cdd..5e192930524907 100644 --- a/test/js-native-api/6_object_wrap/myobject.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -11,7 +11,7 @@ MyObject::MyObject(double value) : value_(value), env_(nullptr), wrapper_(nullptr) {} MyObject::~MyObject() { - napi_delete_reference(env_, wrapper_); + napi_delete_reference(static_cast(env_), wrapper_); } void MyObject::Destructor(node_api_basic_env env, @@ -215,7 +215,7 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env, if (out == nullptr) { // If the napi_ref has been invalidated, delete it. - NODE_API_CALL(env, napi_delete_reference(env, dangling_ref)); + NODE_API_CALL(env, napi_delete_reference(static_cast(env), dangling_ref)); NODE_API_CALL(env, napi_get_boolean(env, true, &ret)); } else { // The dangling napi_ref is still valid. diff --git a/test/js-native-api/6_object_wrap/nested_wrap.cc b/test/js-native-api/6_object_wrap/nested_wrap.cc index f854a35658669a..a91d72228d0a98 100644 --- a/test/js-native-api/6_object_wrap/nested_wrap.cc +++ b/test/js-native-api/6_object_wrap/nested_wrap.cc @@ -8,10 +8,10 @@ static int finalization_count = 0; NestedWrap::NestedWrap() {} NestedWrap::~NestedWrap() { - napi_delete_reference(env_, wrapper_); + napi_delete_reference(static_cast(env_), wrapper_); // Delete the nested reference as well. - napi_delete_reference(env_, nested_); + napi_delete_reference(static_cast(env_), nested_); } void NestedWrap::Destructor(node_api_basic_env env, diff --git a/test/js-native-api/7_factory_wrap/myobject.cc b/test/js-native-api/7_factory_wrap/myobject.cc index a1a00972138a7f..a27ec67432fe32 100644 --- a/test/js-native-api/7_factory_wrap/myobject.cc +++ b/test/js-native-api/7_factory_wrap/myobject.cc @@ -5,7 +5,7 @@ static int finalize_count = 0; MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} -MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); } +MyObject::~MyObject() { napi_delete_reference(static_cast(env_), wrapper_); } void MyObject::Destructor(napi_env env, void* nativeObject, diff --git a/test/js-native-api/8_passing_wrapped/myobject.cc b/test/js-native-api/8_passing_wrapped/myobject.cc index 6ca61bb491ffe0..cc71b30e1cc0f4 100644 --- a/test/js-native-api/8_passing_wrapped/myobject.cc +++ b/test/js-native-api/8_passing_wrapped/myobject.cc @@ -7,7 +7,7 @@ MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} MyObject::~MyObject() { finalize_count++; - napi_delete_reference(env_, wrapper_); + napi_delete_reference(static_cast(env_), wrapper_); } void MyObject::Destructor( diff --git a/test/node-api/test_uv_loop/test_uv_loop.cc b/test/node-api/test_uv_loop/test_uv_loop.cc index 68c3b71f933592..b9c080f026dde3 100644 --- a/test/node-api/test_uv_loop/test_uv_loop.cc +++ b/test/node-api/test_uv_loop/test_uv_loop.cc @@ -65,7 +65,7 @@ napi_value SetImmediateBinding(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_open_handle_scope(env, &scope)); NODE_API_CALL(env, napi_get_undefined(env, &undefined)); NODE_API_CALL(env, napi_get_reference_value(env, cbref, &callback)); - NODE_API_CALL(env, napi_delete_reference(env, cbref)); + NODE_API_CALL(env, napi_delete_reference(static_cast(env), cbref)); NODE_API_CALL(env, napi_call_function(env, undefined, callback, 0, nullptr, nullptr)); NODE_API_CALL(env, napi_close_handle_scope(env, scope)); From 001894c09abd4875cad2861d49a191a668c82e90 Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Sun, 31 Aug 2025 23:13:00 +0530 Subject: [PATCH 2/6] fix: address review comments, remove duplicate comments, wrap long lines for linter compliance --- src/js_native_api_v8.cc | 13 +++++-------- test/cctest/test_linked_binding.cc | 3 ++- test/js-native-api/6_object_wrap/myobject.cc | 4 +++- test/js-native-api/7_factory_wrap/myobject.cc | 4 +++- test/node-api/test_uv_loop/test_uv_loop.cc | 4 +++- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 64186c3114367c..9f1a94aa17abae 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2769,14 +2769,11 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env, return napi_clear_last_error(env); } -// Deletes a reference. The referenced value is released, and may be GC'd unless -// there are other references to it. -// For a napi_reference returned from `napi_wrap`, this must be called in the -// finalizer. -// Deletes a reference. The referenced value is released, and may be GC'd unless -// there are other references to it. -// For a napi_reference returned from `napi_wrap`, this must be called in the -// finalizer. +napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, + napi_ref ref) { + // Deletes a reference. The referenced value is released, and may be GC'd + // unless there are other references to it. For a napi_reference returned + // from `napi_wrap`, this must be called in the finalizer. napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, napi_ref ref) { // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw // JS exceptions. diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 3f1c60e560b1ad..f1320fd03f1dbe 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -171,7 +171,8 @@ napi_value InitializeLocalNapiRefBinding(napi_env env, napi_value exports) { napi_ref ref{}; if (node_api_version == NAPI_VERSION_EXPERIMENTAL) { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_ok); - CHECK_EQ(napi_delete_reference(static_cast(env), ref), napi_ok); + CHECK_EQ(napi_delete_reference(static_cast(env), ref), + napi_ok); } else { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_invalid_arg); } diff --git a/test/js-native-api/6_object_wrap/myobject.cc b/test/js-native-api/6_object_wrap/myobject.cc index 5e192930524907..4be468687ceb31 100644 --- a/test/js-native-api/6_object_wrap/myobject.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -215,7 +215,9 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env, if (out == nullptr) { // If the napi_ref has been invalidated, delete it. - NODE_API_CALL(env, napi_delete_reference(static_cast(env), dangling_ref)); + NODE_API_CALL(env, + napi_delete_reference(static_cast(env), + dangling_ref)); NODE_API_CALL(env, napi_get_boolean(env, true, &ret)); } else { // The dangling napi_ref is still valid. diff --git a/test/js-native-api/7_factory_wrap/myobject.cc b/test/js-native-api/7_factory_wrap/myobject.cc index a27ec67432fe32..8035ff29ac01c7 100644 --- a/test/js-native-api/7_factory_wrap/myobject.cc +++ b/test/js-native-api/7_factory_wrap/myobject.cc @@ -5,7 +5,9 @@ static int finalize_count = 0; MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} -MyObject::~MyObject() { napi_delete_reference(static_cast(env_), wrapper_); } +MyObject::~MyObject() { + napi_delete_reference(static_cast(env_), wrapper_); +} void MyObject::Destructor(napi_env env, void* nativeObject, diff --git a/test/node-api/test_uv_loop/test_uv_loop.cc b/test/node-api/test_uv_loop/test_uv_loop.cc index b9c080f026dde3..8cd74979d4e207 100644 --- a/test/node-api/test_uv_loop/test_uv_loop.cc +++ b/test/node-api/test_uv_loop/test_uv_loop.cc @@ -65,7 +65,9 @@ napi_value SetImmediateBinding(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_open_handle_scope(env, &scope)); NODE_API_CALL(env, napi_get_undefined(env, &undefined)); NODE_API_CALL(env, napi_get_reference_value(env, cbref, &callback)); - NODE_API_CALL(env, napi_delete_reference(static_cast(env), cbref)); + NODE_API_CALL( + env, + napi_delete_reference(static_cast(env), cbref)); NODE_API_CALL(env, napi_call_function(env, undefined, callback, 0, nullptr, nullptr)); NODE_API_CALL(env, napi_close_handle_scope(env, scope)); From 26c06d6d3b2091b40e442fd2e5ce40a484e5ef9b Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Mon, 1 Sep 2025 21:45:07 +0530 Subject: [PATCH 3/6] fix: remove duplicate function definition and fix formatting --- src/js_native_api_v8.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 9f1a94aa17abae..6dfcd534799b39 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2769,12 +2769,12 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env, return napi_clear_last_error(env); } +// Deletes a reference. The referenced value is released, and may be GC'd unless +// there are other references to it. +// For a napi_reference returned from `napi_wrap`, this must be called in the +// finalizer. napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, napi_ref ref) { - // Deletes a reference. The referenced value is released, and may be GC'd - // unless there are other references to it. For a napi_reference returned - // from `napi_wrap`, this must be called in the finalizer. -napi_status NAPI_CDECL napi_delete_reference(node_api_basic_env env, napi_ref ref) { // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw // JS exceptions. CHECK_ENV(env); From 6a54a143c464ac94c1b7737558875ef43e102428 Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Mon, 1 Sep 2025 22:58:16 +0530 Subject: [PATCH 4/6] Format-cpp and lint fix --- test/js-native-api/6_object_wrap/myobject.cc | 6 +++--- test/node-api/test_uv_loop/test_uv_loop.cc | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/js-native-api/6_object_wrap/myobject.cc b/test/js-native-api/6_object_wrap/myobject.cc index 4be468687ceb31..f25a0946e640e1 100644 --- a/test/js-native-api/6_object_wrap/myobject.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -215,9 +215,9 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env, if (out == nullptr) { // If the napi_ref has been invalidated, delete it. - NODE_API_CALL(env, - napi_delete_reference(static_cast(env), - dangling_ref)); + NODE_API_CALL(env, + napi_delete_reference(static_cast(env), + dangling_ref)); NODE_API_CALL(env, napi_get_boolean(env, true, &ret)); } else { // The dangling napi_ref is still valid. diff --git a/test/node-api/test_uv_loop/test_uv_loop.cc b/test/node-api/test_uv_loop/test_uv_loop.cc index 8cd74979d4e207..13c923ea82d23d 100644 --- a/test/node-api/test_uv_loop/test_uv_loop.cc +++ b/test/node-api/test_uv_loop/test_uv_loop.cc @@ -65,9 +65,9 @@ napi_value SetImmediateBinding(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_open_handle_scope(env, &scope)); NODE_API_CALL(env, napi_get_undefined(env, &undefined)); NODE_API_CALL(env, napi_get_reference_value(env, cbref, &callback)); - NODE_API_CALL( - env, - napi_delete_reference(static_cast(env), cbref)); + NODE_API_CALL( + env, + napi_delete_reference(static_cast(env), cbref)); NODE_API_CALL(env, napi_call_function(env, undefined, callback, 0, nullptr, nullptr)); NODE_API_CALL(env, napi_close_handle_scope(env, scope)); From dde1ea8831fd21775a9ea758df9fd11a6ab87223 Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Tue, 2 Sep 2025 15:00:47 +0530 Subject: [PATCH 5/6] removed static_cast --- test/cctest/test_linked_binding.cc | 3 +-- test/js-native-api/6_object_wrap/myobject.cc | 4 +--- test/js-native-api/6_object_wrap/nested_wrap.cc | 4 ++-- test/js-native-api/7_factory_wrap/myobject.cc | 2 +- test/js-native-api/8_passing_wrapped/myobject.cc | 2 +- test/node-api/test_uv_loop/test_uv_loop.cc | 4 +--- 6 files changed, 7 insertions(+), 12 deletions(-) diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index f1320fd03f1dbe..19ea2b8f228ebb 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -171,8 +171,7 @@ napi_value InitializeLocalNapiRefBinding(napi_env env, napi_value exports) { napi_ref ref{}; if (node_api_version == NAPI_VERSION_EXPERIMENTAL) { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_ok); - CHECK_EQ(napi_delete_reference(static_cast(env), ref), - napi_ok); + CHECK_EQ(napi_delete_reference(env, ref), napi_ok); } else { CHECK_EQ(napi_create_reference(env, key, 1, &ref), napi_invalid_arg); } diff --git a/test/js-native-api/6_object_wrap/myobject.cc b/test/js-native-api/6_object_wrap/myobject.cc index f25a0946e640e1..f4c2a90c154236 100644 --- a/test/js-native-api/6_object_wrap/myobject.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -215,9 +215,7 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env, if (out == nullptr) { // If the napi_ref has been invalidated, delete it. - NODE_API_CALL(env, - napi_delete_reference(static_cast(env), - dangling_ref)); + NODE_API_CALL(env, napi_delete_reference(env, dangling_ref)); NODE_API_CALL(env, napi_get_boolean(env, true, &ret)); } else { // The dangling napi_ref is still valid. diff --git a/test/js-native-api/6_object_wrap/nested_wrap.cc b/test/js-native-api/6_object_wrap/nested_wrap.cc index a91d72228d0a98..f854a35658669a 100644 --- a/test/js-native-api/6_object_wrap/nested_wrap.cc +++ b/test/js-native-api/6_object_wrap/nested_wrap.cc @@ -8,10 +8,10 @@ static int finalization_count = 0; NestedWrap::NestedWrap() {} NestedWrap::~NestedWrap() { - napi_delete_reference(static_cast(env_), wrapper_); + napi_delete_reference(env_, wrapper_); // Delete the nested reference as well. - napi_delete_reference(static_cast(env_), nested_); + napi_delete_reference(env_, nested_); } void NestedWrap::Destructor(node_api_basic_env env, diff --git a/test/js-native-api/7_factory_wrap/myobject.cc b/test/js-native-api/7_factory_wrap/myobject.cc index 8035ff29ac01c7..12dcb6e0b27819 100644 --- a/test/js-native-api/7_factory_wrap/myobject.cc +++ b/test/js-native-api/7_factory_wrap/myobject.cc @@ -6,7 +6,7 @@ static int finalize_count = 0; MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} MyObject::~MyObject() { - napi_delete_reference(static_cast(env_), wrapper_); + napi_delete_reference(env_, wrapper_); } void MyObject::Destructor(napi_env env, diff --git a/test/js-native-api/8_passing_wrapped/myobject.cc b/test/js-native-api/8_passing_wrapped/myobject.cc index cc71b30e1cc0f4..6ca61bb491ffe0 100644 --- a/test/js-native-api/8_passing_wrapped/myobject.cc +++ b/test/js-native-api/8_passing_wrapped/myobject.cc @@ -7,7 +7,7 @@ MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} MyObject::~MyObject() { finalize_count++; - napi_delete_reference(static_cast(env_), wrapper_); + napi_delete_reference(env_, wrapper_); } void MyObject::Destructor( diff --git a/test/node-api/test_uv_loop/test_uv_loop.cc b/test/node-api/test_uv_loop/test_uv_loop.cc index 13c923ea82d23d..68c3b71f933592 100644 --- a/test/node-api/test_uv_loop/test_uv_loop.cc +++ b/test/node-api/test_uv_loop/test_uv_loop.cc @@ -65,9 +65,7 @@ napi_value SetImmediateBinding(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_open_handle_scope(env, &scope)); NODE_API_CALL(env, napi_get_undefined(env, &undefined)); NODE_API_CALL(env, napi_get_reference_value(env, cbref, &callback)); - NODE_API_CALL( - env, - napi_delete_reference(static_cast(env), cbref)); + NODE_API_CALL(env, napi_delete_reference(env, cbref)); NODE_API_CALL(env, napi_call_function(env, undefined, callback, 0, nullptr, nullptr)); NODE_API_CALL(env, napi_close_handle_scope(env, scope)); From f58f962a1ac24f8708812830b215d95c3737f56e Mon Sep 17 00:00:00 2001 From: Jeetu Suthar Date: Wed, 3 Sep 2025 22:31:44 +0530 Subject: [PATCH 6/6] remove unnecessary static_cast and formatting --- test/js-native-api/6_object_wrap/myobject.cc | 2 +- test/js-native-api/7_factory_wrap/myobject.cc | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/test/js-native-api/6_object_wrap/myobject.cc b/test/js-native-api/6_object_wrap/myobject.cc index f4c2a90c154236..750d4f450a3cdd 100644 --- a/test/js-native-api/6_object_wrap/myobject.cc +++ b/test/js-native-api/6_object_wrap/myobject.cc @@ -11,7 +11,7 @@ MyObject::MyObject(double value) : value_(value), env_(nullptr), wrapper_(nullptr) {} MyObject::~MyObject() { - napi_delete_reference(static_cast(env_), wrapper_); + napi_delete_reference(env_, wrapper_); } void MyObject::Destructor(node_api_basic_env env, diff --git a/test/js-native-api/7_factory_wrap/myobject.cc b/test/js-native-api/7_factory_wrap/myobject.cc index 12dcb6e0b27819..a1a00972138a7f 100644 --- a/test/js-native-api/7_factory_wrap/myobject.cc +++ b/test/js-native-api/7_factory_wrap/myobject.cc @@ -5,9 +5,7 @@ static int finalize_count = 0; MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} -MyObject::~MyObject() { - napi_delete_reference(env_, wrapper_); -} +MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); } void MyObject::Destructor(napi_env env, void* nativeObject,