From 33f4a51aa5fd0d8650aed2ea7de93b67b4ed7d3a Mon Sep 17 00:00:00 2001 From: killagu Date: Sun, 11 Feb 2018 11:50:31 +0800 Subject: [PATCH 1/6] tools: fix prof polyfill readline `node --prof foo.js` may not print the full profile log file, the last line will broken like `tick,` and not more blank line. `readline`will stuck in infinite loop, add `bytes === 0` will fix it. --- lib/internal/v8_prof_polyfill.js | 7 ++ .../test-tick-processor-builtin.js | 13 ++-- .../test-tick-processor-cpp-core.js | 13 ++-- ...test-tick-processor-polyfill-brokenfile.js | 66 +++++++++++++++++++ .../test-tick-processor-preprocess-flag.js | 13 ++-- 5 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 test/tick-processor/test-tick-processor-polyfill-brokenfile.js diff --git a/lib/internal/v8_prof_polyfill.js b/lib/internal/v8_prof_polyfill.js index 5c6b1407120ea2..3911390bd7f804 100644 --- a/lib/internal/v8_prof_polyfill.js +++ b/lib/internal/v8_prof_polyfill.js @@ -96,6 +96,13 @@ function readline() { if (line.length === 0) { return ''; } + if (bytes === 0) { + process.emitWarning('profile file is broken', { + code: 'BROKEN_PROFILE_FILE', + detail: `${JSON.stringify(line)} at the file end is broken` + }); + return ''; + } } } diff --git a/test/tick-processor/test-tick-processor-builtin.js b/test/tick-processor/test-tick-processor-builtin.js index f94964813ac76a..78444d453b8423 100644 --- a/test/tick-processor/test-tick-processor-builtin.js +++ b/test/tick-processor/test-tick-processor-builtin.js @@ -4,12 +4,15 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD) +if ( + common.isWindows || + common.isSunOS || + common.isAIX || + common.isLinuxPPCBE || + common.isFreeBSD +) { common.skip('C++ symbols are not mapped for this os.'); +} const base = require('./tick-processor-base.js'); diff --git a/test/tick-processor/test-tick-processor-cpp-core.js b/test/tick-processor/test-tick-processor-cpp-core.js index 76407433ea55bc..3ac06206f4756d 100644 --- a/test/tick-processor/test-tick-processor-cpp-core.js +++ b/test/tick-processor/test-tick-processor-cpp-core.js @@ -4,12 +4,15 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD) +if ( + common.isWindows || + common.isSunOS || + common.isAIX || + common.isLinuxPPCBE || + common.isFreeBSD +) { common.skip('C++ symbols are not mapped for this os.'); +} const base = require('./tick-processor-base.js'); diff --git a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js new file mode 100644 index 00000000000000..1d49c4a09671ea --- /dev/null +++ b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js @@ -0,0 +1,66 @@ +'use strict'; +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +if (!common.enoughTestCpu) + common.skip('test is CPU-intensive'); + +if ( + common.isWindows || + common.isSunOS || + common.isAIX || + common.isLinuxPPCBE || + common.isFreeBSD +) { + common.skip('C++ symbols are not mapped for this os.'); +} + +//This test will produce a broken profile log. +//ensure prof-polyfill not stuck in infinite loop +//and success process + + +const assert = require('assert'); +const cp = require('child_process'); +const path = require('path'); +const fs = require('fs'); + +const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log'); +const RETRY_TIMEOUT = 150; +const BROKEN_PART = 'tick,'; +const WARN_REG_EXP = /\(node:\d+\) \[BROKEN_PROFILE_FILE\] Warning: profile file is broken[\r\n]+".*tick," at the file end is broken/; + +const code = `function f() { + this.ts = Date.now(); + setImmediate(function() { new f(); }); + }; + f();`; + +const proc = cp.spawn(process.execPath, [ + '--no_logfile_per_isolate', + '--logfile=-', + '--prof', + '-pe', code +], { + stdio: ['ignore', 'pipe', 'inherit'] +}); + +let ticks = ''; +proc.stdout.on('data', (chunk) => ticks += chunk); + + +function runPolyfill(content) { + proc.kill(); + content += BROKEN_PART; + fs.writeFileSync(LOG_FILE, content); + const child = cp.spawnSync( + `${process.execPath}`, + [ + '--prof-process', LOG_FILE + ]); + assert(WARN_REG_EXP.test(child.stderr.toString())); + assert.strictEqual(child.status, 0, 'broken file should success too'); +} + +setTimeout(() => runPolyfill(ticks), RETRY_TIMEOUT); diff --git a/test/tick-processor/test-tick-processor-preprocess-flag.js b/test/tick-processor/test-tick-processor-preprocess-flag.js index 17e89f581218fb..087d2918d7d848 100644 --- a/test/tick-processor/test-tick-processor-preprocess-flag.js +++ b/test/tick-processor/test-tick-processor-preprocess-flag.js @@ -4,12 +4,15 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD) +if ( + common.isWindows || + common.isSunOS || + common.isAIX || + common.isLinuxPPCBE || + common.isFreeBSD +) { common.skip('C++ symbols are not mapped for this os.'); +} const base = require('./tick-processor-base.js'); From 45730d5936502b1aa76e5ff7dfe5e8d015eff92c Mon Sep 17 00:00:00 2001 From: killagu Date: Sun, 11 Feb 2018 12:03:00 +0800 Subject: [PATCH 2/6] test: add common.isSymbolAvailable --- test/common/README.md | 5 +++++ test/common/index.js | 5 +++++ test/tick-processor/test-tick-processor-builtin.js | 8 +------- test/tick-processor/test-tick-processor-cpp-core.js | 8 +------- .../test-tick-processor-polyfill-brokenfile.js | 8 +------- .../tick-processor/test-tick-processor-preprocess-flag.js | 8 +------- 6 files changed, 14 insertions(+), 28 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 73779fd5372d88..c203d93bd55fa5 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -243,6 +243,11 @@ Platform check for Windows. Platform check for Windows 32-bit on Windows 64-bit. +### isSymbolAvailable +* [<Boolean>] + +Platform check for if symbol available. + ### leakedGlobals() * return [<Array>] diff --git a/test/common/index.js b/test/common/index.js index e8546db3a2f4ea..e9e14cb12bb782 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -802,3 +802,8 @@ exports.hijackStdout = hijackStdWritable.bind(null, 'stdout'); exports.hijackStderr = hijackStdWritable.bind(null, 'stderr'); exports.restoreStdout = restoreWritable.bind(null, 'stdout'); exports.restoreStderr = restoreWritable.bind(null, 'stderr'); +exports.isSymbolAvailable = exports.isWindows || + exports.isSunOS || + exports.isAIX || + exports.isLinuxPPCBE || + exports.isFreeBSD; diff --git a/test/tick-processor/test-tick-processor-builtin.js b/test/tick-processor/test-tick-processor-builtin.js index 78444d453b8423..60a5ee96e61ef7 100644 --- a/test/tick-processor/test-tick-processor-builtin.js +++ b/test/tick-processor/test-tick-processor-builtin.js @@ -4,13 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if ( - common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD -) { +if (common.isSymbolAvailable) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-cpp-core.js b/test/tick-processor/test-tick-processor-cpp-core.js index 3ac06206f4756d..5ac418fb6296d0 100644 --- a/test/tick-processor/test-tick-processor-cpp-core.js +++ b/test/tick-processor/test-tick-processor-cpp-core.js @@ -4,13 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if ( - common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD -) { +if (common.isSymbolAvailable) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js index 1d49c4a09671ea..837b15a24c91a3 100644 --- a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js +++ b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js @@ -6,13 +6,7 @@ tmpdir.refresh(); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if ( - common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD -) { +if (common.isSymbolAvailable) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-preprocess-flag.js b/test/tick-processor/test-tick-processor-preprocess-flag.js index 087d2918d7d848..71832503a7db93 100644 --- a/test/tick-processor/test-tick-processor-preprocess-flag.js +++ b/test/tick-processor/test-tick-processor-preprocess-flag.js @@ -4,13 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if ( - common.isWindows || - common.isSunOS || - common.isAIX || - common.isLinuxPPCBE || - common.isFreeBSD -) { +if (common.isSymbolAvailable) { common.skip('C++ symbols are not mapped for this os.'); } From 261018329633b9ee8479d785afa75ab6977ab893 Mon Sep 17 00:00:00 2001 From: killagu Date: Sun, 11 Feb 2018 20:46:05 +0800 Subject: [PATCH 3/6] fix: fix code style --- test/common/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index e9e14cb12bb782..d65bb7f66a725f 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -803,7 +803,7 @@ exports.hijackStderr = hijackStdWritable.bind(null, 'stderr'); exports.restoreStdout = restoreWritable.bind(null, 'stdout'); exports.restoreStderr = restoreWritable.bind(null, 'stderr'); exports.isSymbolAvailable = exports.isWindows || - exports.isSunOS || - exports.isAIX || - exports.isLinuxPPCBE || - exports.isFreeBSD; + exports.isSunOS || + exports.isAIX || + exports.isLinuxPPCBE || + exports.isFreeBSD; From 05b0391f711a1c7be656eb5404d12ee1da9fcb01 Mon Sep 17 00:00:00 2001 From: killagu Date: Mon, 12 Feb 2018 09:14:22 +0800 Subject: [PATCH 4/6] mv common.isSymbolAvailable -> common.isCPPSymbolsNotMapped --- test/common/README.md | 4 ++-- test/common/index.js | 10 +++++----- test/tick-processor/test-tick-processor-builtin.js | 2 +- test/tick-processor/test-tick-processor-cpp-core.js | 2 +- .../test-tick-processor-polyfill-brokenfile.js | 2 +- .../test-tick-processor-preprocess-flag.js | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index c203d93bd55fa5..8395d7b5c1b461 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -243,10 +243,10 @@ Platform check for Windows. Platform check for Windows 32-bit on Windows 64-bit. -### isSymbolAvailable +### isCPPSymbolsNotMapped * [<Boolean>] -Platform check for if symbol available. +Platform check for c++ symbols are mapped or not. ### leakedGlobals() * return [<Array>] diff --git a/test/common/index.js b/test/common/index.js index d65bb7f66a725f..a0086c6483e0df 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -802,8 +802,8 @@ exports.hijackStdout = hijackStdWritable.bind(null, 'stdout'); exports.hijackStderr = hijackStdWritable.bind(null, 'stderr'); exports.restoreStdout = restoreWritable.bind(null, 'stdout'); exports.restoreStderr = restoreWritable.bind(null, 'stderr'); -exports.isSymbolAvailable = exports.isWindows || - exports.isSunOS || - exports.isAIX || - exports.isLinuxPPCBE || - exports.isFreeBSD; +exports.isCPPSymbolsNotMapped = exports.isWindows || + exports.isSunOS || + exports.isAIX || + exports.isLinuxPPCBE || + exports.isFreeBSD; diff --git a/test/tick-processor/test-tick-processor-builtin.js b/test/tick-processor/test-tick-processor-builtin.js index 60a5ee96e61ef7..3d4e1b9d236030 100644 --- a/test/tick-processor/test-tick-processor-builtin.js +++ b/test/tick-processor/test-tick-processor-builtin.js @@ -4,7 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isSymbolAvailable) { +if (common.isCPPSymbolsNotMapped) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-cpp-core.js b/test/tick-processor/test-tick-processor-cpp-core.js index 5ac418fb6296d0..26daf60aa3c1ce 100644 --- a/test/tick-processor/test-tick-processor-cpp-core.js +++ b/test/tick-processor/test-tick-processor-cpp-core.js @@ -4,7 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isSymbolAvailable) { +if (common.isCPPSymbolsNotMapped) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js index 837b15a24c91a3..b39536a30579a7 100644 --- a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js +++ b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js @@ -6,7 +6,7 @@ tmpdir.refresh(); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isSymbolAvailable) { +if (common.isCPPSymbolsNotMapped) { common.skip('C++ symbols are not mapped for this os.'); } diff --git a/test/tick-processor/test-tick-processor-preprocess-flag.js b/test/tick-processor/test-tick-processor-preprocess-flag.js index 71832503a7db93..93367361aceea3 100644 --- a/test/tick-processor/test-tick-processor-preprocess-flag.js +++ b/test/tick-processor/test-tick-processor-preprocess-flag.js @@ -4,7 +4,7 @@ const common = require('../common'); if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); -if (common.isSymbolAvailable) { +if (common.isCPPSymbolsNotMapped) { common.skip('C++ symbols are not mapped for this os.'); } From 3684bc07ca599fcae13f870fc06ca5b71a884f1a Mon Sep 17 00:00:00 2001 From: killagu Date: Mon, 12 Feb 2018 10:57:52 +0800 Subject: [PATCH 5/6] fix c++ -> C++ --- test/common/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 8395d7b5c1b461..87cd19e8660d4a 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -246,7 +246,7 @@ Platform check for Windows 32-bit on Windows 64-bit. ### isCPPSymbolsNotMapped * [<Boolean>] -Platform check for c++ symbols are mapped or not. +Platform check for C++ symbols are mapped or not. ### leakedGlobals() * return [<Array>] From a43951e5460ec73611fc656ee44c489b472527fa Mon Sep 17 00:00:00 2001 From: killagu Date: Sat, 17 Feb 2018 08:48:03 +0800 Subject: [PATCH 6/6] fix code style fix warn message fix comment style --- lib/internal/v8_prof_polyfill.js | 2 +- .../test-tick-processor-polyfill-brokenfile.js | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/internal/v8_prof_polyfill.js b/lib/internal/v8_prof_polyfill.js index 3911390bd7f804..43ccc0e5d8bfac 100644 --- a/lib/internal/v8_prof_polyfill.js +++ b/lib/internal/v8_prof_polyfill.js @@ -97,7 +97,7 @@ function readline() { return ''; } if (bytes === 0) { - process.emitWarning('profile file is broken', { + process.emitWarning(`Profile file ${logFile} is broken`, { code: 'BROKEN_PROFILE_FILE', detail: `${JSON.stringify(line)} at the file end is broken` }); diff --git a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js index b39536a30579a7..3348b6f11b2e67 100644 --- a/test/tick-processor/test-tick-processor-polyfill-brokenfile.js +++ b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js @@ -7,12 +7,12 @@ if (!common.enoughTestCpu) common.skip('test is CPU-intensive'); if (common.isCPPSymbolsNotMapped) { - common.skip('C++ symbols are not mapped for this os.'); + common.skip('C++ symbols are not mapped for this OS.'); } -//This test will produce a broken profile log. -//ensure prof-polyfill not stuck in infinite loop -//and success process +// This test will produce a broken profile log. +// ensure prof-polyfill not stuck in infinite loop +// and success process const assert = require('assert'); @@ -23,7 +23,8 @@ const fs = require('fs'); const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log'); const RETRY_TIMEOUT = 150; const BROKEN_PART = 'tick,'; -const WARN_REG_EXP = /\(node:\d+\) \[BROKEN_PROFILE_FILE\] Warning: profile file is broken[\r\n]+".*tick," at the file end is broken/; +const WARN_REG_EXP = /\(node:\d+\) \[BROKEN_PROFILE_FILE] Warning: Profile file .* is broken/; +const WARN_DETAIL_REG_EXP = /".*tick," at the file end is broken/; const code = `function f() { this.ts = Date.now(); @@ -54,7 +55,8 @@ function runPolyfill(content) { '--prof-process', LOG_FILE ]); assert(WARN_REG_EXP.test(child.stderr.toString())); - assert.strictEqual(child.status, 0, 'broken file should success too'); + assert(WARN_DETAIL_REG_EXP.test(child.stderr.toString())); + assert.strictEqual(child.status, 0); } setTimeout(() => runPolyfill(ticks), RETRY_TIMEOUT);