Skip to content
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

node-api: add status napi_cannot_run_js #47986

Closed
19 changes: 16 additions & 3 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,8 @@ typedef enum {
napi_arraybuffer_expected,
napi_detachable_arraybuffer_expected,
napi_would_deadlock, /* unused */
napi_no_external_buffers_allowed
napi_no_external_buffers_allowed,
napi_cannot_run_js
} napi_status;
```

Expand Down Expand Up @@ -814,6 +815,18 @@ typedef void (*napi_finalize)(napi_env env,
Unless for reasons discussed in [Object Lifetime Management][], creating a
handle and/or callback scope inside the function body is not necessary.

Since these functions may be called while the JavaScript engine is in a state
where it cannot execute JavaScript code, some Node-API calls may return
`napi_pending_exception` even when there is no exception pending.

Change History:

* experimental:
gabrielschulhof marked this conversation as resolved.
Show resolved Hide resolved

Node-API calls made from a finalizer will return `napi_cannot_run_js` when
the JavaScript engine is unable to execute JavaScript, and will return
`napi_exception_pending` if there is a pending exception.

#### `napi_async_execute_callback`

<!-- YAML
Expand Down Expand Up @@ -1952,11 +1965,11 @@ from [`napi_add_async_cleanup_hook`][].
The Node.js environment may be torn down at an arbitrary time as soon as
possible with JavaScript execution disallowed, like on the request of
[`worker.terminate()`][]. When the environment is being torn down, the
registered `napi_finalize` callbacks of JavaScript objects, Thread-safe
registered `napi_finalize` callbacks of JavaScript objects, thread-safe
functions and environment instance data are invoked immediately and
independently.

The invocation of `napi_finalize` callbacks are scheduled after the manually
The invocation of `napi_finalize` callbacks is scheduled after the manually
registered cleanup hooks. In order to ensure a proper order of addon
finalization during environment shutdown to avoid use-after-free in the
`napi_finalize` callback, addons should register a cleanup hook with
Expand Down
3 changes: 2 additions & 1 deletion src/js_native_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ typedef enum {
napi_arraybuffer_expected,
napi_detachable_arraybuffer_expected,
napi_would_deadlock, // unused
napi_no_external_buffers_allowed
napi_no_external_buffers_allowed,
napi_cannot_run_js,
} napi_status;
// Note: when adding a new enum value to `napi_status`, please also update
// * `const int last_status` in the definition of `napi_get_last_error_info()'
Expand Down
3 changes: 2 additions & 1 deletion src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ static const char* error_messages[] = {
"A detachable arraybuffer was expected",
"Main thread would deadlock",
"External buffers are not allowed",
"Cannot run JavaScript",
};

napi_status NAPI_CDECL napi_get_last_error_info(
Expand All @@ -693,7 +694,7 @@ napi_status NAPI_CDECL napi_get_last_error_info(
// message in the `napi_status` enum each time a new error message is added.
// We don't have a napi_status_last as this would result in an ABI
// change each time a message was added.
const int last_status = napi_no_external_buffers_allowed;
const int last_status = napi_cannot_run_js;

static_assert(NAPI_ARRAYSIZE(error_messages) == last_status + 1,
"Count of error messages must match count of error values");
Expand Down
9 changes: 6 additions & 3 deletions src/js_native_api_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,12 @@ inline napi_status napi_set_last_error(napi_env env,
#define NAPI_PREAMBLE(env) \
CHECK_ENV((env)); \
RETURN_STATUS_IF_FALSE( \
(env), \
(env)->last_exception.IsEmpty() && (env)->can_call_into_js(), \
napi_pending_exception); \
(env), (env)->last_exception.IsEmpty(), napi_pending_exception); \
RETURN_STATUS_IF_FALSE((env), \
(env)->can_call_into_js(), \
(env->module_api_version == NAPI_VERSION_EXPERIMENTAL \
? napi_cannot_run_js \
: napi_pending_exception)); \
napi_clear_last_error((env)); \
v8impl::TryCatch try_catch((env))

Expand Down
14 changes: 14 additions & 0 deletions test/node-api/test_cannot_run_js/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
legendecas marked this conversation as resolved.
Show resolved Hide resolved
"targets": [
{
"target_name": "test_cannot_run_js",
"sources": [ "test_cannot_run_js.c" ],
"defines": [ "NAPI_EXPERIMENTAL" ],
},
{
"target_name": "test_pending_exception",
"sources": [ "test_cannot_run_js.c" ],
"defines": [ "NAPI_VERSION=8" ],
}
]
}
23 changes: 23 additions & 0 deletions test/node-api/test_cannot_run_js/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';

// Test that `napi_call_function()` returns `napi_cannot_run_js` in experimental
// mode and `napi_pending_exception` otherwise. This test calls the add-on's
// `createRef()` method, which creates a strong reference to a JS function. When
// the process exits, it calls all reference finalizers. The finalizer for the
// strong reference created herein will attempt to call `napi_call_function()`
// and will abort the process if the API doesn't return the correct status.

const { buildType } = require('../../common');
const addon_v8 = require(`./build/${buildType}/test_pending_exception`);
const addon_new = require(`./build/${buildType}/test_cannot_run_js`);

function runTests(addon, isVersion8) {
addon.createRef(runTests);
gabrielschulhof marked this conversation as resolved.
Show resolved Hide resolved
}

function runAllTests() {
runTests(addon_v8, /* isVersion8 */ true);
runTests(addon_new, /* isVersion8 */ false);
}

runAllTests();
46 changes: 46 additions & 0 deletions test/node-api/test_cannot_run_js/test_cannot_run_js.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include <node_api.h>
#include "../../js-native-api/common.h"
#include "stdlib.h"

static void Finalize(napi_env env, void* data, void* hint) {
napi_value cb;
napi_ref* ref = data;
#ifdef NAPI_EXPERIMENTAL
napi_status expected_status = napi_cannot_run_js;
#else
napi_status expected_status = napi_pending_exception;
#endif // NAPI_EXPERIMENTAL

if (napi_get_reference_value(env, *ref, &cb) != napi_ok) abort();
legendecas marked this conversation as resolved.
Show resolved Hide resolved
if (napi_delete_reference(env, *ref) != napi_ok) abort();
if (napi_call_function(env, cb, cb, 0, NULL, NULL) != expected_status)
abort();
free(ref);
}

static napi_value CreateRef(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value cb;
napi_valuetype value_type;
napi_ref* ref = malloc(sizeof(*ref));
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, &cb, NULL, NULL));
NODE_API_ASSERT(env, argc == 1, "Function takes only one argument");
NODE_API_CALL(env, napi_typeof(env, cb, &value_type));
NODE_API_ASSERT(
env, value_type == napi_function, "argument must be function");
NODE_API_CALL(env, napi_add_finalizer(env, cb, ref, Finalize, NULL, ref));
return cb;
}

NAPI_MODULE_INIT() {
napi_property_descriptor properties[] = {
DECLARE_NODE_API_PROPERTY("createRef", CreateRef),
};

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

return exports;
}