Skip to content

Commit 6c803db

Browse files
bnoordhuisMylesBorins
bnoordhuis
authored and
MylesBorins
committed
lib: fix event race condition with -e
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]>
1 parent 3d21bfe commit 6c803db

File tree

4 files changed

+72
-47
lines changed

4 files changed

+72
-47
lines changed

‎lib/internal/bootstrap_node.js

+3-7
Original file line numberDiff line numberDiff line change
@@ -385,13 +385,9 @@
385385
'return require("vm").runInThisContext(' +
386386
`${JSON.stringify(body)}, { filename: ` +
387387
`${JSON.stringify(name)}, displayErrors: true });\n`;
388-
// Defer evaluation for a tick. This is a workaround for deferred
389-
// events not firing when evaluating scripts from the command line,
390-
// see https://github.com/nodejs/node/issues/1600.
391-
setImmediate(function() {
392-
const result = module._compile(script, `${name}-wrapper`);
393-
if (process._print_eval) console.log(result);
394-
});
388+
const result = module._compile(script, `${name}-wrapper`);
389+
if (process._print_eval) console.log(result);
390+
process._tickCallback();
395391
}
396392

397393
// Load preload modules

‎test/message/eval_messages.out

+29-24
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
with(this){__filename}
44
^^^^
55
SyntaxError: Strict mode code may not include a with statement
6-
at createScript (vm.js:*)
7-
at Object.runInThisContext (vm.js:*)
6+
at createScript (vm.js:*:*)
7+
at Object.runInThisContext (vm.js:*:*)
88
at Object.<anonymous> ([eval]-wrapper:*:*)
99
at Module._compile (module.js:*:*)
10-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
11-
at runCallback (timers.js:*:*)
12-
at tryOnImmediate (timers.js:*:*)
13-
at processImmediate [as _immediateCallback] (timers.js:*:*)
10+
at evalScript (bootstrap_node.js:*:*)
11+
at run (bootstrap_node.js:*:*)
12+
at run (bootstrap_node.js:*:*)
13+
at startup (bootstrap_node.js:*:*)
14+
at bootstrap_node.js:*:*
1415
42
1516
42
1617
[eval]:1
@@ -19,43 +20,47 @@ throw new Error("hello")
1920

2021
Error: hello
2122
at [eval]:1:7
22-
at ContextifyScript.Script.runInThisContext (vm.js:*)
23-
at Object.runInThisContext (vm.js:*)
23+
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
24+
at Object.runInThisContext (vm.js:*:*)
2425
at Object.<anonymous> ([eval]-wrapper:*:*)
2526
at Module._compile (module.js:*:*)
26-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
27-
at runCallback (timers.js:*:*)
28-
at tryOnImmediate (timers.js:*:*)
29-
at processImmediate [as _immediateCallback] (timers.js:*:*)
27+
at evalScript (bootstrap_node.js:*:*)
28+
at run (bootstrap_node.js:*:*)
29+
at run (bootstrap_node.js:*:*)
30+
at startup (bootstrap_node.js:*:*)
31+
at bootstrap_node.js:*:*
32+
3033
[eval]:1
3134
throw new Error("hello")
3235
^
3336

3437
Error: hello
3538
at [eval]:1:7
36-
at ContextifyScript.Script.runInThisContext (vm.js:*)
37-
at Object.runInThisContext (vm.js:*)
39+
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
40+
at Object.runInThisContext (vm.js:*:*)
3841
at Object.<anonymous> ([eval]-wrapper:*:*)
3942
at Module._compile (module.js:*:*)
40-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
41-
at runCallback (timers.js:*:*)
42-
at tryOnImmediate (timers.js:*:*)
43-
at processImmediate [as _immediateCallback] (timers.js:*:*)
43+
at evalScript (bootstrap_node.js:*:*)
44+
at run (bootstrap_node.js:*:*)
45+
at run (bootstrap_node.js:*:*)
46+
at startup (bootstrap_node.js:*:*)
47+
at bootstrap_node.js:*:*
4448
100
4549
[eval]:1
4650
var x = 100; y = x;
4751
^
4852

4953
ReferenceError: y is not defined
5054
at [eval]:1:16
51-
at ContextifyScript.Script.runInThisContext (vm.js:*)
52-
at Object.runInThisContext (vm.js:*)
55+
at ContextifyScript.Script.runInThisContext (vm.js:*:*)
56+
at Object.runInThisContext (vm.js:*:*)
5357
at Object.<anonymous> ([eval]-wrapper:*:*)
5458
at Module._compile (module.js:*:*)
55-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
56-
at runCallback (timers.js:*:*)
57-
at tryOnImmediate (timers.js:*:*)
58-
at processImmediate [as _immediateCallback] (timers.js:*:*)
59+
at evalScript (bootstrap_node.js:*:*)
60+
at run (bootstrap_node.js:*:*)
61+
at run (bootstrap_node.js:*:*)
62+
at startup (bootstrap_node.js:*:*)
63+
at bootstrap_node.js:*:*
5964

6065
[eval]:1
6166
var ______________________________________________; throw 10

‎test/message/stdin_messages.out

+21-16
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ SyntaxError: Strict mode code may not include a with statement
77
at Object.runInThisContext (vm.js:*)
88
at Object.<anonymous> ([stdin]-wrapper:*:*)
99
at Module._compile (module.js:*:*)
10-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
11-
at runCallback (timers.js:*:*)
12-
at tryOnImmediate (timers.js:*:*)
13-
at processImmediate [as _immediateCallback] (timers.js:*:*)
10+
at evalScript (bootstrap_node.js:*:*)
11+
at Socket.<anonymous> (bootstrap_node.js:*:*)
12+
at emitNone (events.js:*:*)
13+
at Socket.emit (events.js:*:*)
14+
at endReadableNT (_stream_readable.js:*:*)
15+
at _combinedTickCallback (internal/process/next_tick.js:*:*)
1416
42
1517
42
1618
[stdin]:1
@@ -23,10 +25,11 @@ Error: hello
2325
at Object.runInThisContext (vm.js:*)
2426
at Object.<anonymous> ([stdin]-wrapper:*:*)
2527
at Module._compile (module.js:*:*)
26-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
27-
at runCallback (timers.js:*:*)
28-
at tryOnImmediate (timers.js:*:*)
29-
at processImmediate [as _immediateCallback] (timers.js:*:*)
28+
at evalScript (bootstrap_node.js:*:*)
29+
at Socket.<anonymous> (bootstrap_node.js:*:*)
30+
at emitNone (events.js:*:*)
31+
at Socket.emit (events.js:*:*)
32+
at endReadableNT (_stream_readable.js:*:*)
3033
[stdin]:1
3134
throw new Error("hello")
3235
^
@@ -37,10 +40,11 @@ Error: hello
3740
at Object.runInThisContext (vm.js:*)
3841
at Object.<anonymous> ([stdin]-wrapper:*:*)
3942
at Module._compile (module.js:*:*)
40-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
41-
at runCallback (timers.js:*:*)
42-
at tryOnImmediate (timers.js:*:*)
43-
at processImmediate [as _immediateCallback] (timers.js:*:*)
43+
at evalScript (bootstrap_node.js:*:*)
44+
at Socket.<anonymous> (bootstrap_node.js:*:*)
45+
at emitNone (events.js:*:*)
46+
at Socket.emit (events.js:*:*)
47+
at endReadableNT (_stream_readable.js:*:*)
4448
100
4549
[stdin]:1
4650
var x = 100; y = x;
@@ -52,10 +56,11 @@ ReferenceError: y is not defined
5256
at Object.runInThisContext (vm.js:*)
5357
at Object.<anonymous> ([stdin]-wrapper:*:*)
5458
at Module._compile (module.js:*:*)
55-
at Immediate.<anonymous> (bootstrap_node.js:*:*)
56-
at runCallback (timers.js:*:*)
57-
at tryOnImmediate (timers.js:*:*)
58-
at processImmediate [as _immediateCallback] (timers.js:*:*)
59+
at evalScript (bootstrap_node.js:*:*)
60+
at Socket.<anonymous> (bootstrap_node.js:*:*)
61+
at emitNone (events.js:*:*)
62+
at Socket.emit (events.js:*:*)
63+
at endReadableNT (_stream_readable.js:*:*)
5964

6065
[stdin]:1
6166
var ______________________________________________; throw 10

‎test/parallel/test-cli-eval.js

+19
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,25 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`,
150150
assert.strictEqual(proc.stdout, 'start\nbeforeExit\nexit\n');
151151
}
152152

153+
// Regression test for https://github.com/nodejs/node/issues/11948.
154+
{
155+
const script = `
156+
process.on('message', (message) => {
157+
if (message === 'ping') process.send('pong');
158+
if (message === 'exit') process.disconnect();
159+
});
160+
`;
161+
const proc = child.fork('-e', [script]);
162+
proc.on('exit', common.mustCall((exitCode, signalCode) => {
163+
assert.strictEqual(exitCode, 0);
164+
assert.strictEqual(signalCode, null);
165+
}));
166+
proc.on('message', (message) => {
167+
if (message === 'pong') proc.send('exit');
168+
});
169+
proc.send('ping');
170+
}
171+
153172
[ '-arg1',
154173
'-arg1 arg2 --arg3',
155174
'--',

0 commit comments

Comments
 (0)