From 8aef370e228d65e03537aa6a5af216944b4aa3c8 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Tue, 14 Jul 2020 16:31:29 -0500 Subject: [PATCH 1/6] repl: give repl entries unique names. This is a workaround for the REPL for a problem when multiple of the entries have the same source text Fixes: https://github.com/nodejs/node/issues/1337 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10284 --- lib/repl.js | 8 ++++++-- test/parallel/test-repl-dynamic-import.js | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-repl-dynamic-import.js diff --git a/lib/repl.js b/lib/repl.js index cc5bf069c31295..44c07a3788341e 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -128,6 +128,10 @@ const { } = internalBinding('contextify'); const history = require('internal/repl/history'); +let nextREPLResourceNumber = 1; +function getREPLResourceName() { + return `REPL${nextREPLResourceNumber++}`; +} // Lazy-loaded. let processTopLevelAwait; @@ -767,7 +771,7 @@ function REPLServer(prompt, const evalCmd = self[kBufferedCommandSymbol] + cmd + '\n'; debug('eval %j', evalCmd); - self.eval(evalCmd, self.context, 'repl', finish); + self.eval(evalCmd, self.context, getREPLResourceName(), finish); function finish(e, ret) { debug('finish', e, ret); @@ -1248,7 +1252,7 @@ function complete(line, callback) { const memberGroups = []; const evalExpr = `try { ${expr} } catch {}`; - this.eval(evalExpr, this.context, 'repl', (e, obj) => { + this.eval(evalExpr, this.context, getREPLResourceName(), (e, obj) => { try { let p; if ((typeof obj === 'object' && obj !== null) || diff --git a/test/parallel/test-repl-dynamic-import.js b/test/parallel/test-repl-dynamic-import.js new file mode 100644 index 00000000000000..a162a7574fe9c7 --- /dev/null +++ b/test/parallel/test-repl-dynamic-import.js @@ -0,0 +1,20 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const child = child_process.spawn(process.execPath, [ + '--interactive', + '--expose-gc' +], { + stdio: 'pipe' +}); +child.stdin.write('\nimport("fs");\n_.then(gc);\n'); +// Wait for concurrent GC to finish +setTimeout(() => { + child.stdin.write('\nimport("fs");\n'); + child.stdin.write('\nprocess.exit(0);\n'); +}, 500); +child.on('exit', (code, signal) => { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +}); From 84349753c717002a394052d06c087bd621dd68d4 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 15 Jul 2020 09:08:56 -0500 Subject: [PATCH 2/6] fix tests --- lib/repl.js | 4 ++-- test/parallel/test-repl-pretty-custom-stack.js | 12 ++++++------ test/parallel/test-repl-pretty-stack.js | 14 +++++++------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 44c07a3788341e..899b5469276a97 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -572,10 +572,10 @@ function REPLServer(prompt, if (e.name === 'SyntaxError') { // Remove stack trace. e.stack = e.stack - .replace(/^repl:\d+\r?\n/, '') + .replace(/^REPL\d+:\d+\r?\n/, '') .replace(/^\s+at\s.*\n?/gm, ''); } else if (self.replMode === module.exports.REPL_MODE_STRICT) { - e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/, + e.stack = e.stack.replace(/(\s+at\s+REPL\d+:)(\d+)/, (_, pre, line) => pre + (line - 1)); } } diff --git a/test/parallel/test-repl-pretty-custom-stack.js b/test/parallel/test-repl-pretty-custom-stack.js index 8f633a4d4808c5..d04a394a2e249e 100644 --- a/test/parallel/test-repl-pretty-custom-stack.js +++ b/test/parallel/test-repl-pretty-custom-stack.js @@ -5,7 +5,7 @@ const fixtures = require('../common/fixtures'); const assert = require('assert'); const repl = require('repl'); -const stackRegExp = /repl:[0-9]+:[0-9]+/g; +const stackRegExp = /(REPL\d+):[0-9]+:[0-9]+/g; function run({ command, expected }) { let accum = ''; @@ -25,8 +25,8 @@ function run({ command, expected }) { r.write(`${command}\n`); assert.strictEqual( - accum.replace(stackRegExp, 'repl:*:*'), - expected.replace(stackRegExp, 'repl:*:*') + accum.replace(stackRegExp, '$1:*:*'), + expected.replace(stackRegExp, '$1:*:*') ); r.close(); } @@ -48,8 +48,8 @@ const tests = [ { // test .load for a file that throws command: `.load ${fixtures.path('repl-pretty-stack.js')}`, - expected: 'Uncaught Error: Whoops!--->\nrepl:*:*--->\nd (repl:*:*)' + - '--->\nc (repl:*:*)--->\nb (repl:*:*)--->\na (repl:*:*)\n' + expected: 'Uncaught Error: Whoops!--->\nREPL1:*:*--->\nd (REPL1:*:*)' + + '--->\nc (REPL1:*:*)--->\nb (REPL1:*:*)--->\na (REPL1:*:*)\n' }, { command: 'let x y;', @@ -67,7 +67,7 @@ const tests = [ // test anonymous IIFE { command: '(function() { throw new Error(\'Whoops!\'); })()', - expected: 'Uncaught Error: Whoops!--->\nrepl:*:*\n' + expected: 'Uncaught Error: Whoops!--->\nREPL5:*:*\n' } ]; diff --git a/test/parallel/test-repl-pretty-stack.js b/test/parallel/test-repl-pretty-stack.js index 456e866b7b20f9..8ab3fef2aaa033 100644 --- a/test/parallel/test-repl-pretty-stack.js +++ b/test/parallel/test-repl-pretty-stack.js @@ -5,7 +5,7 @@ const fixtures = require('../common/fixtures'); const assert = require('assert'); const repl = require('repl'); -const stackRegExp = /(at .*repl:)[0-9]+:[0-9]+/g; +const stackRegExp = /(at .*REPL\d+:)[0-9]+:[0-9]+/g; function run({ command, expected, ...extraREPLOptions }, i) { let accum = ''; @@ -37,9 +37,9 @@ const tests = [ { // Test .load for a file that throws. command: `.load ${fixtures.path('repl-pretty-stack.js')}`, - expected: 'Uncaught Error: Whoops!\n at repl:*:*\n' + - ' at d (repl:*:*)\n at c (repl:*:*)\n' + - ' at b (repl:*:*)\n at a (repl:*:*)\n' + expected: 'Uncaught Error: Whoops!\n at REPL1:*:*\n' + + ' at d (REPL1:*:*)\n at c (REPL1:*:*)\n' + + ' at b (REPL1:*:*)\n at a (REPL1:*:*)\n' }, { command: 'let x y;', @@ -53,12 +53,12 @@ const tests = [ { command: '(() => { const err = Error(\'Whoops!\'); ' + 'err.foo = \'bar\'; throw err; })()', - expected: "Uncaught Error: Whoops!\n at repl:*:* {\n foo: 'bar'\n}\n", + expected: "Uncaught Error: Whoops!\n at REPL4:*:* {\n foo: 'bar'\n}\n", }, { command: '(() => { const err = Error(\'Whoops!\'); ' + 'err.foo = \'bar\'; throw err; })()', - expected: 'Uncaught Error: Whoops!\n at repl:*:* {\n foo: ' + + expected: 'Uncaught Error: Whoops!\n at REPL5:*:* {\n foo: ' + "\u001b[32m'bar'\u001b[39m\n}\n", useColors: true }, @@ -69,7 +69,7 @@ const tests = [ // Test anonymous IIFE. { command: '(function() { throw new Error(\'Whoops!\'); })()', - expected: 'Uncaught Error: Whoops!\n at repl:*:*\n' + expected: 'Uncaught Error: Whoops!\n at REPL7:*:*\n' } ]; From f24f521a2a0da5e7a2078be3a685038beed036b6 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 15 Jul 2020 09:44:33 -0500 Subject: [PATCH 3/6] Update lib/repl.js --- lib/repl.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/repl.js b/lib/repl.js index 899b5469276a97..c05996128094ea 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -129,6 +129,8 @@ const { const history = require('internal/repl/history'); let nextREPLResourceNumber = 1; +// this prevents v8 code cache from getting confused and using a different +// cache from a resource of the same name function getREPLResourceName() { return `REPL${nextREPLResourceNumber++}`; } From b116249615dec517d9b9160d2c4dcdd119d32071 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 15 Jul 2020 09:45:25 -0500 Subject: [PATCH 4/6] linter... --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index c05996128094ea..18f26a1bb975c5 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -129,7 +129,7 @@ const { const history = require('internal/repl/history'); let nextREPLResourceNumber = 1; -// this prevents v8 code cache from getting confused and using a different +// This prevents v8 code cache from getting confused and using a different // cache from a resource of the same name function getREPLResourceName() { return `REPL${nextREPLResourceNumber++}`; From b40623f28a59944275d0a28e157c57e44d952b39 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 29 Jul 2020 10:11:34 -0500 Subject: [PATCH 5/6] Update test/parallel/test-repl-dynamic-import.js Co-authored-by: Ruben Bridgewater --- test/parallel/test-repl-dynamic-import.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-repl-dynamic-import.js b/test/parallel/test-repl-dynamic-import.js index a162a7574fe9c7..a722f39fce3289 100644 --- a/test/parallel/test-repl-dynamic-import.js +++ b/test/parallel/test-repl-dynamic-import.js @@ -13,7 +13,7 @@ child.stdin.write('\nimport("fs");\n_.then(gc);\n'); setTimeout(() => { child.stdin.write('\nimport("fs");\n'); child.stdin.write('\nprocess.exit(0);\n'); -}, 500); +}, common.platformTimeout(50)); child.on('exit', (code, signal) => { assert.strictEqual(code, 0); assert.strictEqual(signal, null); From 98ffe4a81bef02c89085b72fd0486f8f41f90a5b Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 29 Jul 2020 10:21:12 -0500 Subject: [PATCH 6/6] lint --- test/parallel/test-repl-dynamic-import.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-repl-dynamic-import.js b/test/parallel/test-repl-dynamic-import.js index a722f39fce3289..1f7a01575aa89b 100644 --- a/test/parallel/test-repl-dynamic-import.js +++ b/test/parallel/test-repl-dynamic-import.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const child_process = require('child_process'); const child = child_process.spawn(process.execPath, [