Skip to content

Commit 3974ba4

Browse files
bnoordhuisandrew749
bnoordhuis
authored and
andrew749
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: nodejs/node#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>
1 parent ce0e522 commit 3974ba4

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
@@ -351,13 +351,9 @@
351351
'return require("vm").runInThisContext(' +
352352
`${JSON.stringify(body)}, { filename: ` +
353353
`${JSON.stringify(name)}, displayErrors: true });\n`;
354-
// Defer evaluation for a tick. This is a workaround for deferred
355-
// events not firing when evaluating scripts from the command line,
356-
// see https://github.com/nodejs/node/issues/1600.
357-
setImmediate(function() {
358-
const result = module._compile(script, `${name}-wrapper`);
359-
if (process._print_eval) console.log(result);
360-
});
354+
const result = module._compile(script, `${name}-wrapper`);
355+
if (process._print_eval) console.log(result);
356+
process._tickCallback();
361357
}
362358

363359
// 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
@@ -144,3 +144,22 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`,
144144
assert.strictEqual(proc.stderr, '');
145145
assert.strictEqual(proc.stdout, 'start\nbeforeExit\nexit\n');
146146
}
147+
148+
// Regression test for https://github.com/nodejs/node/issues/11948.
149+
{
150+
const script = `
151+
process.on('message', (message) => {
152+
if (message === 'ping') process.send('pong');
153+
if (message === 'exit') process.disconnect();
154+
});
155+
`;
156+
const proc = child.fork('-e', [script]);
157+
proc.on('exit', common.mustCall((exitCode, signalCode) => {
158+
assert.strictEqual(exitCode, 0);
159+
assert.strictEqual(signalCode, null);
160+
}));
161+
proc.on('message', (message) => {
162+
if (message === 'pong') proc.send('exit');
163+
});
164+
proc.send('ping');
165+
}

0 commit comments

Comments
 (0)