Skip to content

Commit

Permalink
node-api: enable uncaught exceptions policy by default
Browse files Browse the repository at this point in the history 
This enables the option `--force-node-api-uncaught-exceptions-policy`
for a specific Node-API addon when it is compiled with
`NAPI_EXPERIMENTAL` (and this would be the default behavior when
`NAPI_VERSION` 10 releases). This would not break existing Node-API
addons.

PR-URL: #49313
Refs: #36510
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
  • Loading branch information
@legendecas @ruyadorno
legendecas authored and ruyadorno committed Sep 28, 2023
1 parent 6a88c6a commit 9600928
Show file tree
Hide file tree
Showing 16 changed files with 307 additions and 133 deletions.
8 changes: 8 additions & 0 deletions doc/api/n-api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6245,6 +6245,13 @@ napi_create_threadsafe_function(napi_env env,
[`napi_threadsafe_function_call_js`][] provides more details.
* `[out] result`: The asynchronous thread-safe JavaScript function.

**Change History:**

* Experimental (`NAPI_EXPERIMENTAL` is defined):

Uncaught exceptions thrown in `call_js_cb` are handled with the
[`'uncaughtException'`][] event, instead of being ignored.

### `napi_get_threadsafe_function_context`

<!-- YAML
Expand Down Expand Up @@ -6475,6 +6482,7 @@ the add-on's file name during loading.
[Visual Studio]: https://visualstudio.microsoft.com
[Working with JavaScript properties]: #working-with-javascript-properties
[Xcode]: https://developer.apple.com/xcode/
[`'uncaughtException'`]: process.md#event-uncaughtexception
[`Number.MAX_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.max_safe_integer
[`Number.MIN_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.min_safe_integer
[`Worker`]: worker_threads.md#class-worker
Expand Down
16 changes: 10 additions & 6 deletions src/node_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,8 @@ void node_napi_env__::trigger_fatal_exception(v8::Local<v8::Value> local_err) {
node::errors::TriggerUncaughtException(isolate, local_err, local_msg);
}

// option enforceUncaughtExceptionPolicy is added for not breaking existing
// running n-api add-ons, and should be deprecated in the next major Node.js
// release.
// The option enforceUncaughtExceptionPolicy is added for not breaking existing
// running Node-API add-ons.
template <bool enforceUncaughtExceptionPolicy, typename T>
void node_napi_env__::CallbackIntoModule(T&& call) {
CallIntoModule(call, [](napi_env env_, v8::Local<v8::Value> local_err) {
Expand All @@ -93,19 +92,24 @@ void node_napi_env__::CallbackIntoModule(T&& call) {
return;
}
node::Environment* node_env = env->node_env();
if (!node_env->options()->force_node_api_uncaught_exceptions_policy &&
// If the module api version is less than NAPI_VERSION_EXPERIMENTAL,
// and the option --force-node-api-uncaught-exceptions-policy is not
// specified, emit a warning about the uncaught exception instead of
// triggering uncaught exception event.
if (env->module_api_version < NAPI_VERSION_EXPERIMENTAL &&
!node_env->options()->force_node_api_uncaught_exceptions_policy &&
!enforceUncaughtExceptionPolicy) {
ProcessEmitDeprecationWarning(
node_env,
"Uncaught N-API callback exception detected, please run node "
"with option --force-node-api-uncaught-exceptions-policy=true"
"with option --force-node-api-uncaught-exceptions-policy=true "
"to handle those exceptions properly.",
"DEP0168");
return;
}
// If there was an unhandled exception in the complete callback,
// report it as a fatal exception. (There is no JavaScript on the
// callstack that can possibly handle it.)
// call stack that can possibly handle it.)
env->trigger_fatal_exception(local_err);
});
}
Expand Down
6 changes: 6 additions & 0 deletions test/js-native-api/test_reference/binding.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
"sources": [
"test_reference.c"
]
},
{
"target_name": "test_finalizer",
"sources": [
"test_finalizer.c"
]
}
]
}
71 changes: 71 additions & 0 deletions test/js-native-api/test_reference/test_finalizer.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#include <assert.h>
#include <js_native_api.h>
#include <stdlib.h>
#include "../common.h"
#include "../entry_point.h"

static int test_value = 1;
static int finalize_count = 0;

static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) {
int* actual_value = data;
NODE_API_ASSERT_RETURN_VOID(
env,
actual_value == &test_value,
"The correct pointer was passed to the finalizer");

napi_ref finalizer_ref = (napi_ref)hint;
napi_value js_finalizer;
napi_value recv;
NODE_API_CALL_RETURN_VOID(
env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
NODE_API_CALL_RETURN_VOID(
env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
}

static napi_value CreateExternalWithJsFinalize(napi_env env,
napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
napi_value finalizer = args[0];
napi_valuetype finalizer_valuetype;
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
NODE_API_ASSERT(env,
finalizer_valuetype == napi_function,
"Wrong type of first argument");
napi_ref finalizer_ref;
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));

napi_value result;
NODE_API_CALL(env,
napi_create_external(env,
&test_value,
FinalizeExternalCallJs,
finalizer_ref, /* finalize_hint */
&result));

finalize_count = 0;
return result;
}

EXTERN_C_START
napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize",
CreateExternalWithJsFinalize),
};

NODE_API_CALL(
env,
napi_define_properties(env,
exports,
sizeof(descriptors) / sizeof(*descriptors),
descriptors));

return exports;
}
EXTERN_C_END
4 changes: 2 additions & 2 deletions test/js-native-api/test_reference/test_finalizer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Flags: --expose-gc --force-node-api-uncaught-exceptions-policy

const common = require('../../common');
const test_reference = require(`./build/${common.buildType}/test_reference`);
const binding = require(`./build/${common.buildType}/test_finalizer`);
const assert = require('assert');

process.on('uncaughtException', common.mustCall((err) => {
Expand All @@ -11,7 +11,7 @@ process.on('uncaughtException', common.mustCall((err) => {

(async function() {
{
test_reference.createExternalWithJsFinalize(
binding.createExternalWithJsFinalize(
common.mustCall(() => {
throw new Error('finalizer error');
}));
Expand Down
75 changes: 18 additions & 57 deletions test/js-native-api/test_reference/test_reference.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,6 @@ static void FinalizeExternal(napi_env env, void* data, void* hint) {
finalize_count++;
}

static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) {
int *actual_value = data;
NODE_API_ASSERT_RETURN_VOID(env, actual_value == &test_value,
"The correct pointer was passed to the finalizer");

napi_ref finalizer_ref = (napi_ref)hint;
napi_value js_finalizer;
napi_value recv;
NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
}

static napi_value CreateExternal(napi_env env, napi_callback_info info) {
int* data = &test_value;

Expand Down Expand Up @@ -118,31 +104,6 @@ CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
return result;
}

static napi_value
CreateExternalWithJsFinalize(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
napi_value finalizer = args[0];
napi_valuetype finalizer_valuetype;
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument");
napi_ref finalizer_ref;
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));

napi_value result;
NODE_API_CALL(env,
napi_create_external(env,
&test_value,
FinalizeExternalCallJs,
finalizer_ref, /* finalize_hint */
&result));

finalize_count = 0;
return result;
}

static napi_value CheckExternal(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value arg;
Expand Down Expand Up @@ -263,24 +224,24 @@ static napi_value ValidateDeleteBeforeFinalize(napi_env env, napi_callback_info
EXTERN_C_START
napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount),
DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal),
DECLARE_NODE_API_PROPERTY("createExternalWithFinalize",
CreateExternalWithFinalize),
DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize",
CreateExternalWithJsFinalize),
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString", CreateSymbolForEmptyString),
DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength", CreateSymbolForIncorrectLength),
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),
DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue),
DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize",
ValidateDeleteBeforeFinalize),
DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount),
DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal),
DECLARE_NODE_API_PROPERTY("createExternalWithFinalize",
CreateExternalWithFinalize),
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString",
CreateSymbolForEmptyString),
DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength",
CreateSymbolForIncorrectLength),
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),
DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue),
DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize",
ValidateDeleteBeforeFinalize),
};

NODE_API_CALL(env, napi_define_properties(
Expand Down
4 changes: 4 additions & 0 deletions test/node-api/test_buffer/binding.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
{
"target_name": "test_buffer",
"sources": [ "test_buffer.c" ]
},
{
"target_name": "test_finalizer",
"sources": [ "test_finalizer.c" ]
}
]
}
50 changes: 7 additions & 43 deletions test/node-api/test_buffer/test_buffer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ static void noopDeleter(napi_env env, void* data, void* finalize_hint) {
deleterCallCount++;
}

static void malignDeleter(napi_env env, void* data, void* finalize_hint) {
NODE_API_ASSERT_RETURN_VOID(env, data != NULL && strcmp(data, theText) == 0, "invalid data");
napi_ref finalizer_ref = (napi_ref)finalize_hint;
napi_value js_finalizer;
napi_value recv;
NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer));
NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv));
NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL));
NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref));
}

static napi_value newBuffer(napi_env env, napi_callback_info info) {
napi_value theBuffer;
char* theCopy;
Expand Down Expand Up @@ -118,30 +107,6 @@ static napi_value staticBuffer(napi_env env, napi_callback_info info) {
return theBuffer;
}

static napi_value malignFinalizerBuffer(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments");
napi_value finalizer = args[0];
napi_valuetype finalizer_valuetype;
NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype));
NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument");
napi_ref finalizer_ref;
NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref));

napi_value theBuffer;
NODE_API_CALL(
env,
napi_create_external_buffer(env,
sizeof(theText),
(void*)theText,
malignDeleter,
finalizer_ref, // finalize_hint
&theBuffer));
return theBuffer;
}

static napi_value Init(napi_env env, napi_value exports) {
napi_value theValue;

Expand All @@ -151,14 +116,13 @@ static napi_value Init(napi_env env, napi_value exports) {
napi_set_named_property(env, exports, "theText", theValue));

napi_property_descriptor methods[] = {
DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer),
DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer),
DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount),
DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer),
DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance),
DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo),
DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer),
DECLARE_NODE_API_PROPERTY("malignFinalizerBuffer", malignFinalizerBuffer),
DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer),
DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer),
DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount),
DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer),
DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance),
DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo),
DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer),
};

NODE_API_CALL(env, napi_define_properties(
Expand Down
Loading

0 comments on commit 9600928

Please sign in to comment.