Skip to content

Commit

Permalink
lib: fix event race condition with -e
Browse files Browse the repository at this point in the history 
Commit c5b07d4 ("lib: fix beforeExit not working with -e") runs the
to-be-evaluated code at a later time than before because it switches
from `process.nextTick()` to `setImmediate()`.

It affects `-e 'process.on("message", ...)'` because there is now a
larger time gap between startup and attaching the event listener,
increasing the chances of missing early messages.  I'm reasonably
sure `process.nextTick()` was also susceptible to that, only less
pronounced.

Avoid the problem altogether by evaluating the code synchronously.
Harmonizes the logic with `Module.runMain()` from lib/module.js
which also calls `process._tickCallback()` afterwards.

PR-URL: #11958
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
  • Loading branch information
@bnoordhuis @MylesBorins
bnoordhuis authored and MylesBorins committed Mar 28, 2017
1 parent 3d21bfe commit 6c803db
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 47 deletions.
10 changes: 3 additions & 7 deletions lib/internal/bootstrap_node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,9 @@
'return require("vm").runInThisContext(' +
`${JSON.stringify(body)}, { filename: ` +
`${JSON.stringify(name)}, displayErrors: true });\n`;
// 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.
setImmediate(function() {
const result = module._compile(script, `${name}-wrapper`);
if (process._print_eval) console.log(result);
});
const result = module._compile(script, `${name}-wrapper`);
if (process._print_eval) console.log(result);
process._tickCallback();
}

// Load preload modules
Expand Down
53 changes: 29 additions & 24 deletions test/message/eval_messages.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
with(this){__filename}
^^^^
SyntaxError: Strict mode code may not include a with statement
at createScript (vm.js:*)
at Object.runInThisContext (vm.js:*)
at createScript (vm.js:*:*)
at Object.runInThisContext (vm.js:*:*)
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at startup (bootstrap_node.js:*:*)
at bootstrap_node.js:*:*
42
42
[eval]:1
Expand All @@ -19,43 +20,47 @@ throw new Error("hello")

Error: hello
at [eval]:1:7
at ContextifyScript.Script.runInThisContext (vm.js:*)
at Object.runInThisContext (vm.js:*)
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
at Object.runInThisContext (vm.js:*:*)
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at startup (bootstrap_node.js:*:*)
at bootstrap_node.js:*:*

[eval]:1
throw new Error("hello")
^

Error: hello
at [eval]:1:7
at ContextifyScript.Script.runInThisContext (vm.js:*)
at Object.runInThisContext (vm.js:*)
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
at Object.runInThisContext (vm.js:*:*)
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at startup (bootstrap_node.js:*:*)
at bootstrap_node.js:*:*
100
[eval]:1
var x = 100; y = x;
^

ReferenceError: y is not defined
at [eval]:1:16
at ContextifyScript.Script.runInThisContext (vm.js:*)
at Object.runInThisContext (vm.js:*)
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
at Object.runInThisContext (vm.js:*:*)
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at run (bootstrap_node.js:*:*)
at startup (bootstrap_node.js:*:*)
at bootstrap_node.js:*:*

[eval]:1
var ______________________________________________; throw 10
Expand Down
37 changes: 21 additions & 16 deletions test/message/stdin_messages.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ SyntaxError: Strict mode code may not include a with statement
at Object.runInThisContext (vm.js:*)
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at Socket.<anonymous> (bootstrap_node.js:*:*)
at emitNone (events.js:*:*)
at Socket.emit (events.js:*:*)
at endReadableNT (_stream_readable.js:*:*)
at _combinedTickCallback (internal/process/next_tick.js:*:*)
42
42
[stdin]:1
Expand All @@ -23,10 +25,11 @@ Error: hello
at Object.runInThisContext (vm.js:*)
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at Socket.<anonymous> (bootstrap_node.js:*:*)
at emitNone (events.js:*:*)
at Socket.emit (events.js:*:*)
at endReadableNT (_stream_readable.js:*:*)
[stdin]:1
throw new Error("hello")
^
Expand All @@ -37,10 +40,11 @@ Error: hello
at Object.runInThisContext (vm.js:*)
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at Socket.<anonymous> (bootstrap_node.js:*:*)
at emitNone (events.js:*:*)
at Socket.emit (events.js:*:*)
at endReadableNT (_stream_readable.js:*:*)
100
[stdin]:1
var x = 100; y = x;
Expand All @@ -52,10 +56,11 @@ ReferenceError: y is not defined
at Object.runInThisContext (vm.js:*)
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at Immediate.<anonymous> (bootstrap_node.js:*:*)
at runCallback (timers.js:*:*)
at tryOnImmediate (timers.js:*:*)
at processImmediate [as _immediateCallback] (timers.js:*:*)
at evalScript (bootstrap_node.js:*:*)
at Socket.<anonymous> (bootstrap_node.js:*:*)
at emitNone (events.js:*:*)
at Socket.emit (events.js:*:*)
at endReadableNT (_stream_readable.js:*:*)

[stdin]:1
var ______________________________________________; throw 10
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-cli-eval.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,25 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`,
assert.strictEqual(proc.stdout, 'start\nbeforeExit\nexit\n');
}

// Regression test for https://github.com/nodejs/node/issues/11948.
{
const script = `
process.on('message', (message) => {
if (message === 'ping') process.send('pong');
if (message === 'exit') process.disconnect();
});
`;
const proc = child.fork('-e', [script]);
proc.on('exit', common.mustCall((exitCode, signalCode) => {
assert.strictEqual(exitCode, 0);
assert.strictEqual(signalCode, null);
}));
proc.on('message', (message) => {
if (message === 'pong') proc.send('exit');
});
proc.send('ping');
}

[ '-arg1',
'-arg1 arg2 --arg3',
'--',
Expand Down

0 comments on commit 6c803db

Please sign in to comment.