FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

node-api: add status napi_cannot_run_js · nodejs/node@53c02b2 · GitHub

/ node Public

Commit 53c02b2

Browse files
gabrielschulhof
authored and
MoLow
committed
node-api: add status napi_cannot_run_js
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. PR-URL: #47986 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Signed-off-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
1 parent 98c6e4b commit 53c02b2

7 files changed

Lines changed: 131 additions & 8 deletions

File tree

‎doc/api/n-api.md‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,8 @@ typedef enum {
580580
napi_arraybuffer_expected,
581581
napi_detachable_arraybuffer_expected,
582582
napi_would_deadlock, /* unused */
583-
napi_no_external_buffers_allowed
583+
napi_no_external_buffers_allowed,
584+
napi_cannot_run_js
584585
} napi_status;
585586
```
586587

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

818+
Since these functions may be called while the JavaScript engine is in a state
819+
where it cannot execute JavaScript code, some Node-API calls may return
820+
`napi_pending_exception` even when there is no exception pending.
821+
822+
Change History:
823+
824+
* experimental (`NAPI_EXPERIMENTAL` is defined):
825+
826+
Node-API calls made from a finalizer will return `napi_cannot_run_js` when
827+
the JavaScript engine is unable to execute JavaScript, and will return
828+
`napi_exception_pending` if there is a pending exception.
829+
817830
#### `napi_async_execute_callback`
818831

819832
<!-- YAML
@@ -1950,11 +1963,11 @@ from [`napi_add_async_cleanup_hook`][].
19501963
The Node.js environment may be torn down at an arbitrary time as soon as
19511964
possible with JavaScript execution disallowed, like on the request of
19521965
[`worker.terminate()`][]. When the environment is being torn down, the
1953-
registered `napi_finalize` callbacks of JavaScript objects, Thread-safe
1966+
registered `napi_finalize` callbacks of JavaScript objects, thread-safe
19541967
functions and environment instance data are invoked immediately and
19551968
independently.
19561969

1957-
The invocation of `napi_finalize` callbacks are scheduled after the manually
1970+
The invocation of `napi_finalize` callbacks is scheduled after the manually
19581971
registered cleanup hooks. In order to ensure a proper order of addon
19591972
finalization during environment shutdown to avoid use-after-free in the
19601973
`napi_finalize` callback, addons should register a cleanup hook with

‎src/js_native_api_types.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ typedef enum {
9999
napi_arraybuffer_expected,
100100
napi_detachable_arraybuffer_expected,
101101
napi_would_deadlock, // unused
102-
napi_no_external_buffers_allowed
102+
napi_no_external_buffers_allowed,
103+
napi_cannot_run_js,
103104
} napi_status;
104105
// Note: when adding a new enum value to `napi_status`, please also update
105106
// * `const int last_status` in the definition of `napi_get_last_error_info()'

‎src/js_native_api_v8.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,7 @@ static const char* error_messages[] = {
682682
"A detachable arraybuffer was expected",
683683
"Main thread would deadlock",
684684
"External buffers are not allowed",
685+
"Cannot run JavaScript",
685686
};
686687

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

698699
static_assert(NAPI_ARRAYSIZE(error_messages) == last_status + 1,
699700
"Count of error messages must match count of error values");

‎src/js_native_api_v8.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,12 @@ inline napi_status napi_set_last_error(napi_env env,
212212
#define NAPI_PREAMBLE(env) \
213213
CHECK_ENV((env)); \
214214
RETURN_STATUS_IF_FALSE( \
215-
(env), \
216-
(env)->last_exception.IsEmpty() && (env)->can_call_into_js(), \
217-
napi_pending_exception); \
215+
(env), (env)->last_exception.IsEmpty(), napi_pending_exception); \
216+
RETURN_STATUS_IF_FALSE((env), \
217+
(env)->can_call_into_js(), \
218+
(env->module_api_version == NAPI_VERSION_EXPERIMENTAL \
219+
? napi_cannot_run_js \
220+
: napi_pending_exception)); \
218221
napi_clear_last_error((env)); \
219222
v8impl::TryCatch try_catch((env))
220223

‎test/js-native-api/test_cannot_run_js/binding.gyp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"targets": [
3+
{
4+
"target_name": "copy_entry_point",
5+
"type": "none",
6+
"copies": [
7+
{
8+
"destination": ".",
9+
"files": [ "../entry_point.c" ]
10+
}
11+
]
12+
},
13+
{
14+
"target_name": "test_cannot_run_js",
15+
"sources": [
16+
"entry_point.c",
17+
"test_cannot_run_js.c"
18+
],
19+
"defines": [ "NAPI_EXPERIMENTAL" ],
20+
"dependencies": [ "copy_entry_point" ],
21+
},
22+
{
23+
"target_name": "test_pending_exception",
24+
"sources": [
25+
"entry_point.c",
26+
"test_cannot_run_js.c"
27+
],
28+
"defines": [ "NAPI_VERSION=8" ],
29+
"dependencies": [ "copy_entry_point" ],
30+
}
31+
]
32+
}

‎test/js-native-api/test_cannot_run_js/test.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
3+
// Test that `napi_call_function()` returns `napi_cannot_run_js` in experimental
4+
// mode and `napi_pending_exception` otherwise. This test calls the add-on's
5+
// `createRef()` method, which creates a strong reference to a JS function. When
6+
// the process exits, it calls all reference finalizers. The finalizer for the
7+
// strong reference created herein will attempt to call `napi_get_property()` on
8+
// a property of the global object and will abort the process if the API doesn't
9+
// return the correct status.
10+
11+
const { buildType, mustNotCall } = require('../../common');
12+
const addon_v8 = require(`./build/${buildType}/test_pending_exception`);
13+
const addon_new = require(`./build/${buildType}/test_cannot_run_js`);
14+
15+
function runTests(addon, isVersion8) {
16+
addon.createRef(mustNotCall());
17+
}
18+
19+
function runAllTests() {
20+
runTests(addon_v8, /* isVersion8 */ true);
21+
runTests(addon_new, /* isVersion8 */ false);
22+
}
23+
24+
runAllTests();

‎test/js-native-api/test_cannot_run_js/test_cannot_run_js.c‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include <js_native_api.h>
2+
#include "../common.h"
3+
#include "stdlib.h"
4+
5+
static void Finalize(napi_env env, void* data, void* hint) {
6+
napi_value global, set_timeout;
7+
napi_ref* ref = data;
8+
#ifdef NAPI_EXPERIMENTAL
9+
napi_status expected_status = napi_cannot_run_js;
10+
#else
11+
napi_status expected_status = napi_pending_exception;
12+
#endif // NAPI_EXPERIMENTAL
13+
14+
if (napi_delete_reference(env, *ref) != napi_ok) abort();
15+
if (napi_get_global(env, &global) != napi_ok) abort();
16+
if (napi_get_named_property(env, global, "setTimeout", &set_timeout) !=
17+
expected_status)
18+
abort();
19+
free(ref);
20+
}
21+
22+
static napi_value CreateRef(napi_env env, napi_callback_info info) {
23+
size_t argc = 1;
24+
napi_value cb;
25+
napi_valuetype value_type;
26+
napi_ref* ref = malloc(sizeof(*ref));
27+
NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, &cb, NULL, NULL));
28+
NODE_API_ASSERT(env, argc == 1, "Function takes only one argument");
29+
NODE_API_CALL(env, napi_typeof(env, cb, &value_type));
30+
NODE_API_ASSERT(
31+
env, value_type == napi_function, "argument must be function");
32+
NODE_API_CALL(env, napi_add_finalizer(env, cb, ref, Finalize, NULL, ref));
33+
return cb;
34+
}
35+
36+
EXTERN_C_START
37+
napi_value Init(napi_env env, napi_value exports) {
38+
napi_property_descriptor properties[] = {
39+
DECLARE_NODE_API_PROPERTY("createRef", CreateRef),
40+
};
41+
42+
NODE_API_CALL(
43+
env,
44+
napi_define_properties(
45+
env, exports, sizeof(properties) / sizeof(*properties), properties));
46+
47+
return exports;
48+
}
49+
EXTERN_C_END

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL