Skip to content

node-api: run Node-API finalizers in GC second pass #42208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vmoroz wants to merge 9 commits into from
Closed

node-api: run Node-API finalizers in GC second pass #42208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bfc9dd0
Implement finalizer_queue to run finalizers earlier
vmoroz Mar 4, 2022
63532d7
Fix C++ lint issues
vmoroz Mar 4, 2022
0a5c446
node-api: implement finalizer_queue to run finalizers earlier
vmoroz Mar 4, 2022
832409b
Fix C++ lint issues
vmoroz Mar 4, 2022
aaf43e2
Merge branch 'FinalizerQueue' of https://github.com/vmoroz/node into …
vmoroz Mar 4, 2022
d004b75
add public node_api_call_finalizers and unit tests
vmoroz Mar 11, 2022
518482f
implement node_api_set_finalizer_error_handler
vmoroz Mar 18, 2022
623d5a6
fix lint-cpp and lint-js issues
vmoroz Mar 18, 2022
66b25ac
fix lint-md issues
vmoroz Mar 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix lint-cpp and lint-js issues
  • Loading branch information
@vmoroz
vmoroz committed Mar 18, 2022
commit 623d5a6f77ed7ba34fca131f8dd272901f45cdca
8 changes: 5 additions & 3 deletions src/js_native_api_v8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@
} while (0)

void napi_env__::HandleFinalizerException(napi_env env,
v8::Local<v8::Value> exception) {
// We must reset the lats exception here to enable Node-API calls in the handler.
v8::Local<v8::Value> exception) {
// We must reset the last exception here to enable Node-API calls in the
// handler.
env->last_exception.Reset();
if (!env->finalizer_error_handler.IsEmpty()) {
bool isHandled = true;
Expand All @@ -69,7 +70,8 @@ void napi_env__::HandleFinalizerException(napi_env env,
napi_value err_value = v8impl::JsValueFromV8LocalValue(exception);
napi_value recv, result;
STATUS_CALL(napi_get_undefined(env, &recv));
STATUS_CALL(napi_call_function(env, recv, handler, 1, &err_value, &result));
STATUS_CALL(
napi_call_function(env, recv, handler, 1, &err_value, &result));
napi_valuetype result_type;
STATUS_CALL(napi_typeof(env, result, &result_type));
if (result_type == napi_boolean) {
Expand Down
3 changes: 2 additions & 1 deletion test/js-native-api/test_finalizer_exception/test_finalizer_exception.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NODE_API_GETTER("finalizeCount", TestObject::GetFinalizeCount),
DECLARE_NODE_API_PROPERTY("createObject", CreateObject),
DECLARE_NODE_API_PROPERTY("setFinalizerErrorHandler", SetFinalizerErrorHandler),
DECLARE_NODE_API_PROPERTY("setFinalizerErrorHandler",
SetFinalizerErrorHandler),
};

NODE_API_CALL(
Expand Down
2 changes: 1 addition & 1 deletion test/js-native-api/test_finalizer_exception/test_handled_exception.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function runGCTests() {
test.setFinalizerErrorHandler((err) => {
++handledExceptions;
assert.strictEqual(err.message, 'Error during Finalize');
if (handledExceptions == 2) {
if (handledExceptions === 2) {
// One time we report the error to be unhandled
return false;
}
Expand Down
12 changes: 7 additions & 5 deletions test/js-native-api/test_finalizer_exception/test_unhandled_exception.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,25 @@ function gcUntilSync(value) {
}

function runGCTests() {
let exceptionCount = 0;
let unhandledExceptions = 0;
process.on('uncaughtException', (err) => {
++exceptionCount;
++unhandledExceptions;
assert.strictEqual(err.message, 'Error during Finalize');
});

(() => {
test.createObject(/* throw on destruct */ true);
test.createObject(true /* throw on destruct */);
})();
gcUntilSync(1);
assert.strictEqual(test.finalizeCount, 1);
assert.strictEqual(unhandledExceptions, 1);

(() => {
test.createObject(/* throw on destruct */ true);
test.createObject(/* throw on destruct */ true);
test.createObject(true /* throw on destruct */);
test.createObject(true /* throw on destruct */);
})();
gcUntilSync(3);
assert.strictEqual(test.finalizeCount, 3);
assert.strictEqual(unhandledExceptions, 3);
}
runGCTests();
6 changes: 3 additions & 3 deletions test/js-native-api/test_finalizer_exception/testobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef TEST_JS_NATIVE_API_TEST_FINALIZING_QUEUE_TESTOBJECT_H_
#define TEST_JS_NATIVE_API_TEST_FINALIZING_QUEUE_TESTOBJECT_H_
#ifndef TEST_JS_NATIVE_API_TEST_FINALIZER_EXCEPTION_TESTOBJECT_H_
#define TEST_JS_NATIVE_API_TEST_FINALIZER_EXCEPTION_TESTOBJECT_H_

#include <js_native_api.h>

Expand All @@ -25,4 +25,4 @@ class TestObject {
bool throw_js_in_destructor_{false};
};

#endif // TEST_JS_NATIVE_API_TEST_FINALIZING_QUEUE_TESTOBJECT_H_
#endif // TEST_JS_NATIVE_API_TEST_FINALIZER_EXCEPTION_TESTOBJECT_H_