From f8691398e444e60cc1d0f1a9c05452b2878c4c97 Mon Sep 17 00:00:00 2001 From: killagu Date: Sun, 11 Feb 2018 11:50:31 +0800 Subject: [PATCH] tools, test: fix prof polyfill readline `node --prof foo.js` may not print the full profile log file, leaving the last line broken (for example `tick,`. When that happens, `readline` will be stuck in an infinite loop. This patch fixes it. Also introduced `common.isCPPSymbolsNotMapped` to avoid duplicated code on tick-processor tests. Backport-PR-URL: https://github.com/nodejs/node/pull/18901 PR-URL: https://github.com/nodejs/node/pull/18641 Reviewed-By: Khaidi Chu Reviewed-By: Matheus Marchini Reviewed-By: Ruben Bridgewater --- lib/internal/v8_prof_polyfill.js | 7 +++ test/common/README.md | 5 ++ test/common/index.js | 6 ++ .../test-tick-processor-builtin.js | 7 +-- .../test-tick-processor-cpp-core.js | 7 +-- ...test-tick-processor-polyfill-brokenfile.js | 62 +++++++++++++++++++ .../test-tick-processor-preprocess-flag.js | 7 +-- 7 files changed, 86 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..43ccc0e5d8bfac 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 ${logFile} is broken`, { + code: 'BROKEN_PROFILE_FILE', + detail: `${JSON.stringify(line)} at the file end is broken` + }); + return ''; + } } } diff --git a/test/common/README.md b/test/common/README.md index f1e4c329ed1b9b..c6742deb691587 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. +### isCPPSymbolsNotMapped +* [<Boolean>] + +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 30b6aca88e05e3..ce07d91c80224c 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -832,3 +832,9 @@ exports.firstInvalidFD = function firstInvalidFD() { } catch (e) {} return fd; }; + +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 f94964813ac76a..3d4e1b9d236030 100644 --- a/test/tick-processor/test-tick-processor-builtin.js +++ b/test/tick-processor/test-tick-processor-builtin.js @@ -4,12 +4,9 @@ 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.isCPPSymbolsNotMapped) { 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 496d06b555f3e8..7775dcdbd6184a 100644 --- a/test/tick-processor/test-tick-processor-cpp-core.js +++ b/test/tick-processor/test-tick-processor-cpp-core.js @@ -4,12 +4,9 @@ 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.isCPPSymbolsNotMapped) { 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..3348b6f11b2e67 --- /dev/null +++ b/test/tick-processor/test-tick-processor-polyfill-brokenfile.js @@ -0,0 +1,62 @@ +'use strict'; +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +if (!common.enoughTestCpu) + common.skip('test is CPU-intensive'); + +if (common.isCPPSymbolsNotMapped) { + 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/; +const WARN_DETAIL_REG_EXP = /".*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(WARN_DETAIL_REG_EXP.test(child.stderr.toString())); + assert.strictEqual(child.status, 0); +} + +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 52d642a3ae3fc0..8b852d6a83d180 100644 --- a/test/tick-processor/test-tick-processor-preprocess-flag.js +++ b/test/tick-processor/test-tick-processor-preprocess-flag.js @@ -4,12 +4,9 @@ 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.isCPPSymbolsNotMapped) { common.skip('C++ symbols are not mapped for this os.'); +} const base = require('./tick-processor-base.js');