Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ea73702

Browse filesBrowse files
ywave620juanarbol
ywave620
authored and
juanarbol
committed
process,worker: ensure code after exit() effectless
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 16ee02f commit ea73702
Copy full SHA for ea73702

File tree

Expand file treeCollapse file tree

10 files changed

+76
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+76
-5
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/per_thread.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/per_thread.js
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ function hrtimeBigInt() {
103103
return hrBigintValues[0];
104104
}
105105

106+
function nop() {}
107+
106108
// The execution of this function itself should not cause any side effects.
107109
function wrapProcessMethods(binding) {
108110
const {
@@ -193,6 +195,16 @@ function wrapProcessMethods(binding) {
193195
// in the user land. Either document it, or deprecate it in favor of a
194196
// better public alternative.
195197
process.reallyExit(process.exitCode || 0);
198+
199+
// If this is a worker, v8::Isolate::TerminateExecution() is called above.
200+
// That function spoofs the stack pointer to cause the stack guard
201+
// check to throw the termination exception. Because v8 performs
202+
// stack guard check upon every function call, we give it a chance.
203+
//
204+
// Without this, user code after `process.exit()` would take effect.
205+
// test/parallel/test-worker-voluntarily-exit-followed-by-addition.js
206+
// test/parallel/test-worker-voluntarily-exit-followed-by-throw.js
207+
nop();
196208
}
197209

198210
function kill(pid, sig) {
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ class CallbackWrapperBase : public CallbackWrapper {
314314
env->CallIntoModule([&](napi_env env) { result = cb(env, cbinfo_wrapper); },
315315
[&](napi_env env, v8::Local<v8::Value> value) {
316316
exceptionOccurred = true;
317+
if (env->terminatedOrTerminating()) {
318+
return;
319+
}
317320
env->isolate->ThrowException(value);
318321
});
319322

Collapse file

‎src/js_native_api_v8.h‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.h
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,20 @@ struct napi_env__ {
7272
}
7373

7474
static inline void HandleThrow(napi_env env, v8::Local<v8::Value> value) {
75+
if (env->terminatedOrTerminating()) {
76+
return;
77+
}
7578
env->isolate->ThrowException(value);
7679
}
7780

81+
// i.e. whether v8 exited or is about to exit
82+
inline bool terminatedOrTerminating() {
83+
return this->isolate->IsExecutionTerminating() || !can_call_into_js();
84+
}
85+
86+
// v8 uses a special exception to indicate termination, the
87+
// `handle_exception` callback should identify such case using
88+
// terminatedOrTerminating() before actually handle the exception
7889
template <typename T, typename U = decltype(HandleThrow)>
7990
inline void CallIntoModule(T&& call, U&& handle_exception = HandleThrow) {
8091
int open_handle_scopes_before = open_handle_scopes;
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ template <bool enforceUncaughtExceptionPolicy, typename T>
9595
void node_napi_env__::CallbackIntoModule(T&& call) {
9696
CallIntoModule(call, [](napi_env env_, v8::Local<v8::Value> local_err) {
9797
node_napi_env__* env = static_cast<node_napi_env__*>(env_);
98+
if (env->terminatedOrTerminating()) {
99+
return;
100+
}
98101
node::Environment* node_env = env->node_env();
99102
if (!node_env->options()->force_node_api_uncaught_exceptions_policy &&
100103
!enforceUncaughtExceptionPolicy) {
Collapse file

‎test/cctest/test_environment.cc‎

Copy file name to clipboardExpand all lines: test/cctest/test_environment.cc
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,9 @@ TEST_F(EnvironmentTest, ExitHandlerTest) {
553553
callback_calls++;
554554
node::Stop(*env);
555555
});
556-
node::LoadEnvironment(*env, "process.exit(42)").ToLocalChecked();
556+
// When terminating, v8 throws makes the current embedder call bail out
557+
// with MaybeLocal<>()
558+
EXPECT_TRUE(node::LoadEnvironment(*env, "process.exit(42)").IsEmpty());
557559
EXPECT_EQ(callback_calls, 1);
558560
}
559561

Collapse file

‎test/node-api/test_worker_terminate/test.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_worker_terminate/test.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ if (isMainThread) {
1919
const { Test } = require(`./build/${common.buildType}/test_worker_terminate`);
2020

2121
const { counter } = workerData;
22-
// Test() tries to call a function twice and asserts that the second call does
23-
// not work because of a pending exception.
22+
// Test() tries to call a function and asserts it fails because of a
23+
// pending termination exception.
2424
Test(() => {
2525
Atomics.add(counter, 0, 1);
2626
process.exit();
Collapse file

‎test/node-api/test_worker_terminate/test_worker_terminate.c‎

Copy file name to clipboardExpand all lines: test/node-api/test_worker_terminate/test_worker_terminate.c
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ napi_value Test(napi_env env, napi_callback_info info) {
1717
NODE_API_ASSERT(env, t == napi_function,
1818
"Wrong first argument, function expected.");
1919

20-
status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
21-
assert(status == napi_ok);
2220
status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
2321
assert(status == napi_pending_exception);
2422

Collapse file

‎test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { Worker } = require('worker_threads');
1212
const workerData = new Int32Array(new SharedArrayBuffer(4));
1313
const w = new Worker(`
1414
const { createHook } = require('async_hooks');
15+
const { workerData } = require('worker_threads');
1516
1617
setImmediate(async () => {
1718
createHook({ init() {} }).enable();
Collapse file

‎test/parallel/test-worker-voluntarily-exit-followed-by-addition.js‎

Copy file name to clipboard
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { Worker, isMainThread } = require('worker_threads');
5+
6+
if (isMainThread) {
7+
const workerData = new Int32Array(new SharedArrayBuffer(4));
8+
new Worker(__filename, {
9+
workerData,
10+
});
11+
process.on('beforeExit', common.mustCall(() => {
12+
assert.strictEqual(workerData[0], 0);
13+
}));
14+
} else {
15+
const { workerData } = require('worker_threads');
16+
process.exit();
17+
workerData[0] = 1;
18+
}
Collapse file

‎test/parallel/test-worker-voluntarily-exit-followed-by-throw.js‎

Copy file name to clipboard
+23Lines changed: 23 additions & 0 deletions
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 } = require('worker_threads');
5+
6+
if (isMainThread) {
7+
const workerData = new Int32Array(new SharedArrayBuffer(4));
8+
new Worker(__filename, {
9+
workerData,
10+
});
11+
process.on('beforeExit', common.mustCall(() => {
12+
assert.strictEqual(workerData[0], 0);
13+
}));
14+
} else {
15+
const { workerData } = require('worker_threads');
16+
try {
17+
process.exit();
18+
throw new Error('xxx');
19+
// eslint-disable-next-line no-unused-vars
20+
} catch (err) {
21+
workerData[0] = 1;
22+
}
23+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.