From 5efb3c373a9fbda9c451a13607fc6d550e9ff717 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 fc3121b243..100b5063da 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -353,7 +353,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 7fbf51f7e0..44965be374 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 62c43b02a8..828bee92cb 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 f22084bc69..3c38afd2ac 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'); +}