Skip to content

Commit 1ce5e63

Browse files
addaleaxrvagg
authored andcommitted
n-api: do not call into JS when that is not allowed
Check whether calling into JS is allowed before doing so. PR-URL: #26127 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent cbd3cf0 commit 1ce5e63

File tree

5 files changed

+84
-5
lines changed

5 files changed

+84
-5
lines changed

src/js_native_api_v8.h

+9-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ struct napi_env__ {
1111
context_persistent(isolate, context) {
1212
CHECK_EQ(isolate, context->GetIsolate());
1313
}
14+
virtual ~napi_env__() {}
1415
v8::Isolate* const isolate; // Shortcut for context()->GetIsolate()
1516
v8impl::Persistent<v8::Context> context_persistent;
1617

@@ -21,6 +22,8 @@ struct napi_env__ {
2122
inline void Ref() { refs++; }
2223
inline void Unref() { if ( --refs == 0) delete this; }
2324

25+
virtual bool can_call_into_js() const { return true; }
26+
2427
v8impl::Persistent<v8::Value> last_exception;
2528
napi_extended_error_info last_error;
2629
int open_handle_scopes = 0;
@@ -68,11 +71,12 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code,
6871
RETURN_STATUS_IF_FALSE((env), !((maybe).IsEmpty()), (status))
6972

7073
// NAPI_PREAMBLE is not wrapped in do..while: try_catch must have function scope
71-
#define NAPI_PREAMBLE(env) \
72-
CHECK_ENV((env)); \
73-
RETURN_STATUS_IF_FALSE((env), (env)->last_exception.IsEmpty(), \
74-
napi_pending_exception); \
75-
napi_clear_last_error((env)); \
74+
#define NAPI_PREAMBLE(env) \
75+
CHECK_ENV((env)); \
76+
RETURN_STATUS_IF_FALSE((env), \
77+
(env)->last_exception.IsEmpty() && (env)->can_call_into_js(), \
78+
napi_pending_exception); \
79+
napi_clear_last_error((env)); \
7680
v8impl::TryCatch try_catch((env))
7781

7882
#define CHECK_TO_TYPE(env, type, context, result, src, status) \

src/node_api.cc

+5
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ struct node_napi_env__ : public napi_env__ {
1212
napi_env__(context) {
1313
CHECK_NOT_NULL(node_env());
1414
}
15+
1516
inline node::Environment* node_env() const {
1617
return node::Environment::GetCurrent(context());
1718
}
19+
20+
bool can_call_into_js() const override {
21+
return node_env()->can_call_into_js();
22+
}
1823
};
1924

2025
typedef node_napi_env__* node_napi_env;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"targets": [
3+
{
4+
"target_name": "test_worker_terminate",
5+
"sources": [ "test_worker_terminate.c" ]
6+
}
7+
]
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../../common');
3+
const assert = require('assert');
4+
const { Worker, isMainThread, workerData } = require('worker_threads');
5+
6+
if (isMainThread) {
7+
const counter = new Int32Array(new SharedArrayBuffer(4));
8+
const worker = new Worker(__filename, { workerData: { counter } });
9+
worker.on('exit', common.mustCall(() => {
10+
assert.strictEqual(counter[0], 1);
11+
}));
12+
worker.on('error', common.mustNotCall());
13+
} else {
14+
const { Test } = require(`./build/${common.buildType}/test_worker_terminate`);
15+
16+
const { counter } = workerData;
17+
// Test() tries to call a function twice and asserts that the second call does
18+
// not work because of a pending exception.
19+
Test(() => {
20+
Atomics.add(counter, 0, 1);
21+
process.exit();
22+
});
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#include <stdio.h>
2+
#include <node_api.h>
3+
#include <assert.h>
4+
#include "../../js-native-api/common.h"
5+
6+
napi_value Test(napi_env env, napi_callback_info info) {
7+
size_t argc = 1;
8+
napi_value recv;
9+
napi_value argv[1];
10+
napi_status status;
11+
12+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, &recv, NULL));
13+
NAPI_ASSERT(env, argc >= 1, "Not enough arguments, expected 1.");
14+
15+
napi_valuetype t;
16+
NAPI_CALL(env, napi_typeof(env, argv[0], &t));
17+
NAPI_ASSERT(env, t == napi_function,
18+
"Wrong first argument, function expected.");
19+
20+
status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
21+
assert(status == napi_ok);
22+
status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
23+
assert(status == napi_pending_exception);
24+
25+
return NULL;
26+
}
27+
28+
napi_value Init(napi_env env, napi_value exports) {
29+
napi_property_descriptor properties[] = {
30+
DECLARE_NAPI_PROPERTY("Test", Test)
31+
};
32+
33+
NAPI_CALL(env, napi_define_properties(
34+
env, exports, sizeof(properties) / sizeof(*properties), properties));
35+
36+
return exports;
37+
}
38+
39+
NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)

0 commit comments

Comments
 (0)