Skip to content

Commit

Permalink
node-api: add status napi_cannot_run_js
Browse files Browse the repository at this point in the history
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.
  • Loading branch information
gabrielschulhof committed May 13, 2023
1 parent 2dd6d76 commit 0e89301
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 8 deletions.
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:

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 @@
{
"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);
}

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();
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;
}

0 comments on commit 0e89301

Please sign in to comment.