Skip to content

Commit

Permalink
lib: fix beforeExit not working with -e
Browse files Browse the repository at this point in the history
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: #8534
PR-URL: #8821
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
bnoordhuis authored and evanlucas committed Nov 2, 2016
1 parent 06b4140 commit 9372aee
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 25 deletions.
2 changes: 1 addition & 1 deletion lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
28 changes: 16 additions & 12 deletions test/message/eval_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ SyntaxError: Strict mode code may not include a with statement
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
42
42
[eval]:1
Expand All @@ -20,9 +21,10 @@ Error: hello
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
[eval]:1
throw new Error("hello")
^
Expand All @@ -32,9 +34,10 @@ Error: hello
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (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;
Expand All @@ -45,9 +48,10 @@ ReferenceError: y is not defined
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
[eval]:1
var ______________________________________________; throw 10
^
Expand Down
28 changes: 16 additions & 12 deletions test/message/stdin_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ SyntaxError: Strict mode code may not include a with statement
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
42
42

Expand All @@ -22,9 +23,10 @@ Error: hello
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)

[stdin]:1
throw new Error("hello")
Expand All @@ -35,9 +37,10 @@ Error: hello
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
100

[stdin]:1
Expand All @@ -49,9 +52,10 @@ ReferenceError: y is not defined
at Object.exports.runInThisContext (vm.js:*)
at Object.<anonymous> ([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.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)

[stdin]:1
var ______________________________________________; throw 10
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-cli-eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

0 comments on commit 9372aee

Please sign in to comment.