Skip to content

Commit

Permalink
process,worker: ensure code followed by exit() effectless
Browse files Browse the repository at this point in the history
Cope with the delay(to the next function call) of
v8::Isolate::TerminateExecution()
  • Loading branch information
ywave620 committed Nov 25, 2022
1 parent a01dbf1 commit c7f350d
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 5 deletions.
12 changes: 12 additions & 0 deletions lib/internal/process/per_thread.js
Expand Up @@ -104,6 +104,8 @@ function hrtimeBigInt() {
return hrBigintValues[0];
}

function nop() {}

// The execution of this function itself should not cause any side effects.
function wrapProcessMethods(binding) {
const {
Expand Down Expand Up @@ -195,6 +197,16 @@ function wrapProcessMethods(binding) {
// in the user land. Either document it, or deprecate it in favor of a
// better public alternative.
process.reallyExit(process.exitCode || kNoFailure);

// If this is a worker, v8::Isolate::TerminateExecution() is called above.
// That function spoofs the stack pointer to cause the stack guard
// check to throw the termination exception. Because v8 performs
// stack guard check upon every function call, we give it a chance.
//
// Without this, user code followed by `process.exit()` would take effect.
// test/parallel/test-worker-voluntarily-exit-followed-by-addition.js
// test/parallel/test-worker-voluntarily-exit-followed-by-throw.js
nop();
}

function kill(pid, sig) {
Expand Down
4 changes: 3 additions & 1 deletion test/cctest/test_environment.cc
Expand Up @@ -553,7 +553,9 @@ TEST_F(EnvironmentTest, ExitHandlerTest) {
callback_calls++;
node::Stop(*env);
});
node::LoadEnvironment(*env, "process.exit(42)").ToLocalChecked();
// When terminating, v8 throws makes the current embedder call bail out
// with MaybeLocal<>()
EXPECT_TRUE(node::LoadEnvironment(*env, "process.exit(42)").IsEmpty());
EXPECT_EQ(callback_calls, 1);
}

Expand Down
4 changes: 2 additions & 2 deletions test/node-api/test_worker_terminate/test.js
Expand Up @@ -19,8 +19,8 @@ if (isMainThread) {
const { Test } = require(`./build/${common.buildType}/test_worker_terminate`);

const { counter } = workerData;
// Test() tries to call a function twice and asserts that the second call does
// not work because of a pending exception.
// Test() tries to call a function and asserts it fails because of a
// pending termination exception.
Test(() => {
Atomics.add(counter, 0, 1);
process.exit();
Expand Down
2 changes: 0 additions & 2 deletions test/node-api/test_worker_terminate/test_worker_terminate.c
Expand Up @@ -17,8 +17,6 @@ napi_value Test(napi_env env, napi_callback_info info) {
NODE_API_ASSERT(env, t == napi_function,
"Wrong first argument, function expected.");

status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
assert(status == napi_ok);
status = napi_call_function(env, recv, argv[0], 0, NULL, NULL);
assert(status == napi_pending_exception);

Expand Down
Expand Up @@ -12,6 +12,7 @@ const { Worker } = require('worker_threads');
const workerData = new Int32Array(new SharedArrayBuffer(4));
const w = new Worker(`
const { createHook } = require('async_hooks');
const { workerData } = require('worker_threads');
setImmediate(async () => {
createHook({ init() {} }).enable();
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-worker-voluntarily-exit-followed-by-addition.js
@@ -0,0 +1,18 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker, isMainThread } = require('worker_threads');

if (isMainThread) {
const workerData = new Int32Array(new SharedArrayBuffer(4));
const w = new Worker(__filename, {
workerData,
});
w.on('exit', common.mustCall(() => {
assert.strictEqual(workerData[0], 0);
}));
} else {
const { workerData } = require('worker_threads');
process.exit();
workerData[0] = 1;
}
23 changes: 23 additions & 0 deletions test/parallel/test-worker-voluntarily-exit-followed-by-throw.js
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker, isMainThread } = require('worker_threads');

if (isMainThread) {
const workerData = new Int32Array(new SharedArrayBuffer(4));
const w = new Worker(__filename, {
workerData,
});
w.on('exit', common.mustCall(() => {
assert.strictEqual(workerData[0], 0);
}));
} else {
const { workerData } = require('worker_threads');
try {
process.exit();
throw new Error('xxx');
// eslint-disable-next-line no-unused-vars
} catch (err) {
workerData[0] = 1;
}
}

0 comments on commit c7f350d

Please sign in to comment.