From c5b07d4ec6de2cb362b887499e44655eaeb26cba Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 28 Sep 2016 12:01:48 +0200 Subject: [PATCH] lib: fix beforeExit not working with -e Commit 93a44d5 ("src: fix deferred events not working with -e") defers evaluation of the script to the next tick. A side effect of that change is that 'beforeExit' listeners run before the actual script. 'beforeExit' is emitted when the event loop is empty but process.nextTick() does not ref the event loop. Fix that by using setImmediate(). Because it is implemented in terms of a uv_check_t handle, it interacts with the event loop properly. Fixes: https://github.com/nodejs/node/issues/8534 PR-URL: https://github.com/nodejs/node/pull/8821 Reviewed-By: Colin Ihrig --- lib/internal/bootstrap_node.js | 2 +- test/message/eval_messages.out | 28 ++++++++++++++++------------ test/message/stdin_messages.out | 28 ++++++++++++++++------------ test/parallel/test-cli-eval.js | 15 +++++++++++++++ 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 8b8d066ab039e9..8db79aa33fb32a 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -341,7 +341,7 @@ // Defer evaluation for a tick. This is a workaround for deferred // events not firing when evaluating scripts from the command line, // see https://github.com/nodejs/node/issues/1600. - process.nextTick(function() { + setImmediate(function() { const result = module._compile(script, `${name}-wrapper`); if (process._print_eval) console.log(result); }); diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index 7fbf51f7e0f2f7..44965be374bac5 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -6,9 +6,10 @@ SyntaxError: Strict mode code may not include a with statement at Object.exports.runInThisContext (vm.js:*) at Object. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) 42 42 [eval]:1 @@ -20,9 +21,10 @@ Error: hello at Object.exports.runInThisContext (vm.js:*) at Object. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) [eval]:1 throw new Error("hello") ^ @@ -32,9 +34,10 @@ Error: hello at Object.exports.runInThisContext (vm.js:*) at Object. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) 100 [eval]:1 var x = 100; y = x; @@ -45,9 +48,10 @@ ReferenceError: y is not defined at Object.exports.runInThisContext (vm.js:*) at Object. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) [eval]:1 var ______________________________________________; throw 10 ^ diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index 62c43b02a8e81b..828bee92cb6f7f 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -7,9 +7,10 @@ SyntaxError: Strict mode code may not include a with statement at Object.exports.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) 42 42 @@ -22,9 +23,10 @@ Error: hello at Object.exports.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) [stdin]:1 throw new Error("hello") @@ -35,9 +37,10 @@ Error: hello at Object.exports.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) 100 [stdin]:1 @@ -49,9 +52,10 @@ ReferenceError: y is not defined at Object.exports.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at bootstrap_node.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) - at process._tickCallback (internal/process/next_tick.js:*:*) + at Immediate. (bootstrap_node.js:*:*) + at runCallback (timers.js:*:*) + at tryOnImmediate (timers.js:*:*) + at processImmediate [as _immediateCallback] (timers.js:*:*) [stdin]:1 var ______________________________________________; throw 10 diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index f22084bc690395..3c38afd2ac4524 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -97,3 +97,18 @@ child.exec(nodejs + ` -e 'require("child_process").fork("${emptyFile}")'`, assert.equal(stdout, ''); assert.equal(stderr, ''); }); + +// Regression test for https://github.com/nodejs/node/issues/8534. +{ + const script = ` + // console.log() can revive the event loop so we must be careful + // to write from a 'beforeExit' event listener only once. + process.once("beforeExit", () => console.log("beforeExit")); + process.on("exit", () => console.log("exit")); + console.log("start"); + `; + const options = { encoding: 'utf8' }; + const proc = child.spawnSync(process.execPath, ['-e', script], options); + assert.strictEqual(proc.stderr, ''); + assert.strictEqual(proc.stdout, 'start\nbeforeExit\nexit\n'); +}