Skip to content

n-api: Enable scope and ref APIs during exception #12524

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.

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
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 40 additions & 20 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2043,15 +2043,17 @@ napi_status napi_create_reference(napi_env env,
napi_value value,
uint32_t initial_refcount,
napi_ref* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, value);
CHECK_ARG(env, result);

v8impl::Reference* reference = v8impl::Reference::New(
env, v8impl::V8LocalValueFromJsValue(value), initial_refcount, false);

*result = reinterpret_cast<napi_ref>(reference);
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

// Deletes a reference. The referenced value is released, and may be GC'd unless
Expand All @@ -2073,7 +2075,9 @@ napi_status napi_delete_reference(napi_env env, napi_ref ref) {
// Calling this when the refcount is 0 and the object is unavailable
// results in an error.
napi_status napi_reference_ref(napi_env env, napi_ref ref, uint32_t* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, ref);

v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
Expand All @@ -2083,15 +2087,17 @@ napi_status napi_reference_ref(napi_env env, napi_ref ref, uint32_t* result) {
*result = count;
}

return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

// Decrements the reference count, optionally returning the resulting count. If
// the result is 0 the reference is now weak and the object may be GC'd at any
// time if there are no other references. Calling this when the refcount is
// already 0 results in an error.
napi_status napi_reference_unref(napi_env env, napi_ref ref, uint32_t* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, ref);

v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
Expand All @@ -2106,7 +2112,7 @@ napi_status napi_reference_unref(napi_env env, napi_ref ref, uint32_t* result) {
*result = count;
}

return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

// Attempts to get a referenced value. If the reference is weak, the value might
Expand All @@ -2115,59 +2121,71 @@ napi_status napi_reference_unref(napi_env env, napi_ref ref, uint32_t* result) {
napi_status napi_get_reference_value(napi_env env,
napi_ref ref,
napi_value* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, ref);
CHECK_ARG(env, result);

v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
*result = v8impl::JsValueFromV8LocalValue(reference->Get());

return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_open_handle_scope(napi_env env, napi_handle_scope* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, result);

*result = v8impl::JsHandleScopeFromV8HandleScope(
new v8impl::HandleScopeWrapper(env->isolate));
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_close_handle_scope(napi_env env, napi_handle_scope scope) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, scope);

delete v8impl::V8HandleScopeFromJsHandleScope(scope);
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_open_escapable_handle_scope(
napi_env env,
napi_escapable_handle_scope* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, result);

*result = v8impl::JsEscapableHandleScopeFromV8EscapableHandleScope(
new v8impl::EscapableHandleScopeWrapper(env->isolate));
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_close_escapable_handle_scope(
napi_env env,
napi_escapable_handle_scope scope) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, scope);

delete v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_escape_handle(napi_env env,
napi_escapable_handle_scope scope,
napi_value escapee,
napi_value* result) {
NAPI_PREAMBLE(env);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, scope);
CHECK_ARG(env, escapee);
CHECK_ARG(env, result);
Expand All @@ -2176,7 +2194,7 @@ napi_status napi_escape_handle(napi_env env,
v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
*result = v8impl::JsValueFromV8LocalValue(
s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
return GET_RETURN_STATUS(env);
return napi_clear_last_error(env);
}

napi_status napi_new_instance(napi_env env,
Expand Down Expand Up @@ -2386,7 +2404,6 @@ napi_status napi_create_buffer(napi_env env,
void** data,
napi_value* result) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, data);
CHECK_ARG(env, result);

auto maybe = node::Buffer::New(env->isolate, length);
Expand All @@ -2396,7 +2413,10 @@ napi_status napi_create_buffer(napi_env env,
v8::Local<v8::Object> buffer = maybe.ToLocalChecked();

*result = v8impl::JsValueFromV8LocalValue(buffer);
*data = node::Buffer::Data(buffer);

if (data != nullptr) {
*data = node::Buffer::Data(buffer);
}

return GET_RETURN_STATUS(env);
}
Expand Down
7 changes: 7 additions & 0 deletions test/addons-napi/test_handle_scope/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ const testHandleScope =
require(`./build/${common.buildType}/test_handle_scope`);

testHandleScope.NewScope();

assert.ok(testHandleScope.NewScopeEscape() instanceof Object);

assert.throws(
() => {
testHandleScope.NewScopeWithException(() => { throw new RangeError(); });
},
RangeError);
25 changes: 25 additions & 0 deletions test/addons-napi/test_handle_scope/test_handle_scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,35 @@ napi_value NewScopeEscape(napi_env env, napi_callback_info info) {
return escapee;
}

napi_value NewScopeWithException(napi_env env, napi_callback_info info) {
napi_handle_scope scope;
size_t argc;
napi_value exception_function;
napi_status status;
napi_value output = NULL;

NAPI_CALL(env, napi_open_handle_scope(env, &scope));
NAPI_CALL(env, napi_create_object(env, &output));

argc = 1;
NAPI_CALL(env, napi_get_cb_info(
env, info, &argc, &exception_function, NULL, NULL));

status = napi_call_function(
env, output, exception_function, 0, NULL, NULL);
NAPI_ASSERT(env, status == napi_pending_exception,
"Function should have thrown.");

// Closing a handle scope should still work while an exception is pending.
NAPI_CALL(env, napi_close_handle_scope(env, scope));
return NULL;
}

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("NewScope", NewScope),
DECLARE_NAPI_PROPERTY("NewScopeEscape", NewScopeEscape),
DECLARE_NAPI_PROPERTY("NewScopeWithException", NewScopeWithException),
};

NAPI_CALL_RETURN_VOID(env, napi_define_properties(
Expand Down