From de62467497bd242d20ae8d543c7cea58faf03ef5 Mon Sep 17 00:00:00 2001 From: ywave620 Date: Fri, 25 Nov 2022 15:17:41 +0800 Subject: [PATCH] process,worker: ensure code after exit() effectless Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() --- lib/internal/process/per_thread.js | 12 ++++++++++ test/cctest/test_environment.cc | 4 +++- test/node-api/test_worker_terminate/test.js | 4 ++-- .../test_worker_terminate.c | 2 -- ...-async-hooks-worker-asyncfn-terminate-4.js | 1 + ...r-voluntarily-exit-followed-by-addition.js | 18 +++++++++++++++ ...rker-voluntarily-exit-followed-by-throw.js | 23 +++++++++++++++++++ 7 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-worker-voluntarily-exit-followed-by-addition.js create mode 100644 test/parallel/test-worker-voluntarily-exit-followed-by-throw.js diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index b2a15a46484255..ed1b82d13e77cc 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -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 { @@ -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) { diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 812962cd5c1a71..547c8ddbffe243 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -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); } diff --git a/test/node-api/test_worker_terminate/test.js b/test/node-api/test_worker_terminate/test.js index 7c7224c073dda3..eefb974af5a669 100644 --- a/test/node-api/test_worker_terminate/test.js +++ b/test/node-api/test_worker_terminate/test.js @@ -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(); diff --git a/test/node-api/test_worker_terminate/test_worker_terminate.c b/test/node-api/test_worker_terminate/test_worker_terminate.c index 3c428195b9a571..48e3e0fa675ed3 100644 --- a/test/node-api/test_worker_terminate/test_worker_terminate.c +++ b/test/node-api/test_worker_terminate/test_worker_terminate.c @@ -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); diff --git a/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js b/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js index dc641471b691e0..c522091006cf5a 100644 --- a/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js +++ b/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js @@ -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(); diff --git a/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js b/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js new file mode 100644 index 00000000000000..9bb706bdd8d4c7 --- /dev/null +++ b/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; +} diff --git a/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js b/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js new file mode 100644 index 00000000000000..d5a599c2bed5bb --- /dev/null +++ b/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; + } +}