From 9ff7ed23cd822dc810ddd99d63d4e2ca68635474 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 21 Mar 2017 11:30:59 +0100 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/11958 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel Reviewed-By: Michael Dawson Reviewed-By: Daijiro Wachi --- lib/internal/bootstrap_node.js | 10 ++----- test/message/eval_messages.out | 53 ++++++++++++++++++--------------- test/message/stdin_messages.out | 37 +++++++++++++---------- test/parallel/test-cli-eval.js | 19 ++++++++++++ 4 files changed, 72 insertions(+), 47 deletions(-) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index f29f7a647a4..51b7f928cb2 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -400,13 +400,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 diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index ba0c35431c6..340e760dc69 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -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. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (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 @@ -19,28 +20,31 @@ 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. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (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. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (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; @@ -48,14 +52,15 @@ 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. ([eval]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (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 diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index b4c51d7ad56..ad1688f15d0 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -7,10 +7,12 @@ SyntaxError: Strict mode code may not include a with statement at Object.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (bootstrap_node.js:*:*) - at runCallback (timers.js:*:*) - at tryOnImmediate (timers.js:*:*) - at processImmediate [as _immediateCallback] (timers.js:*:*) + at evalScript (bootstrap_node.js:*:*) + at Socket. (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 @@ -23,10 +25,11 @@ Error: hello at Object.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (bootstrap_node.js:*:*) - at runCallback (timers.js:*:*) - at tryOnImmediate (timers.js:*:*) - at processImmediate [as _immediateCallback] (timers.js:*:*) + at evalScript (bootstrap_node.js:*:*) + at Socket. (bootstrap_node.js:*:*) + at emitNone (events.js:*:*) + at Socket.emit (events.js:*:*) + at endReadableNT (_stream_readable.js:*:*) [stdin]:1 throw new Error("hello") ^ @@ -37,10 +40,11 @@ Error: hello at Object.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (bootstrap_node.js:*:*) - at runCallback (timers.js:*:*) - at tryOnImmediate (timers.js:*:*) - at processImmediate [as _immediateCallback] (timers.js:*:*) + at evalScript (bootstrap_node.js:*:*) + at Socket. (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; @@ -52,10 +56,11 @@ ReferenceError: y is not defined at Object.runInThisContext (vm.js:*) at Object. ([stdin]-wrapper:*:*) at Module._compile (module.js:*:*) - at Immediate. (bootstrap_node.js:*:*) - at runCallback (timers.js:*:*) - at tryOnImmediate (timers.js:*:*) - at processImmediate [as _immediateCallback] (timers.js:*:*) + at evalScript (bootstrap_node.js:*:*) + at Socket. (bootstrap_node.js:*:*) + at emitNone (events.js:*:*) + at Socket.emit (events.js:*:*) + at endReadableNT (_stream_readable.js:*:*) [stdin]:1 var ______________________________________________; throw 10 diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js index 6d21b5d50be..34681bd235c 100644 --- a/test/parallel/test-cli-eval.js +++ b/test/parallel/test-cli-eval.js @@ -171,6 +171,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', '--',