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

process,worker: ensure code after exit() effectless · nodejs/node@ea73702 · GitHub

/ node Public

Commit ea73702

Browse files
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

10 files changed

Lines changed: 76 additions & 5 deletions

‎lib/internal/process/per_thread.js‎

Lines 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) {

‎src/js_native_api_v8.cc‎

Lines 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

‎src/js_native_api_v8.h‎

Lines 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;

‎src/node_api.cc‎

Lines 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) {

‎test/cctest/test_environment.cc‎

Lines 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

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

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

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

Lines 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

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

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

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

Lines 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+
}

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

Lines 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)
Back | FazBrowse Home | New Git URL