Skip to content

Commit f869139

Browse files
killaguaddaleax
authored andcommitted
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: #18901 PR-URL: #18641 Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent 237a363 commit f869139

7 files changed

+86
-15
lines changed

lib/internal/v8_prof_polyfill.js

+7
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,13 @@ function readline() {
9696
if (line.length === 0) {
9797
return '';
9898
}
99+
if (bytes === 0) {
100+
process.emitWarning(`Profile file ${logFile} is broken`, {
101+
code: 'BROKEN_PROFILE_FILE',
102+
detail: `${JSON.stringify(line)} at the file end is broken`
103+
});
104+
return '';
105+
}
99106
}
100107
}
101108

test/common/README.md

+5
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,11 @@ Platform check for Windows.
243243

244244
Platform check for Windows 32-bit on Windows 64-bit.
245245

246+
### isCPPSymbolsNotMapped
247+
* [&lt;Boolean>]
248+
249+
Platform check for C++ symbols are mapped or not.
250+
246251
### leakedGlobals()
247252
* return [&lt;Array>]
248253

test/common/index.js

+6
Original file line numberDiff line numberDiff line change
@@ -832,3 +832,9 @@ exports.firstInvalidFD = function firstInvalidFD() {
832832
} catch (e) {}
833833
return fd;
834834
};
835+
836+
exports.isCPPSymbolsNotMapped = exports.isWindows ||
837+
exports.isSunOS ||
838+
exports.isAIX ||
839+
exports.isLinuxPPCBE ||
840+
exports.isFreeBSD;

test/tick-processor/test-tick-processor-builtin.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@ const common = require('../common');
44
if (!common.enoughTestCpu)
55
common.skip('test is CPU-intensive');
66

7-
if (common.isWindows ||
8-
common.isSunOS ||
9-
common.isAIX ||
10-
common.isLinuxPPCBE ||
11-
common.isFreeBSD)
7+
if (common.isCPPSymbolsNotMapped) {
128
common.skip('C++ symbols are not mapped for this os.');
9+
}
1310

1411
const base = require('./tick-processor-base.js');
1512

test/tick-processor/test-tick-processor-cpp-core.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@ const common = require('../common');
44
if (!common.enoughTestCpu)
55
common.skip('test is CPU-intensive');
66

7-
if (common.isWindows ||
8-
common.isSunOS ||
9-
common.isAIX ||
10-
common.isLinuxPPCBE ||
11-
common.isFreeBSD)
7+
if (common.isCPPSymbolsNotMapped) {
128
common.skip('C++ symbols are not mapped for this os.');
9+
}
1310

1411
const base = require('./tick-processor-base.js');
1512

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
'use strict';
2+
const common = require('../common');
3+
const tmpdir = require('../common/tmpdir');
4+
tmpdir.refresh();
5+
6+
if (!common.enoughTestCpu)
7+
common.skip('test is CPU-intensive');
8+
9+
if (common.isCPPSymbolsNotMapped) {
10+
common.skip('C++ symbols are not mapped for this OS.');
11+
}
12+
13+
// This test will produce a broken profile log.
14+
// ensure prof-polyfill not stuck in infinite loop
15+
// and success process
16+
17+
18+
const assert = require('assert');
19+
const cp = require('child_process');
20+
const path = require('path');
21+
const fs = require('fs');
22+
23+
const LOG_FILE = path.join(tmpdir.path, 'tick-processor.log');
24+
const RETRY_TIMEOUT = 150;
25+
const BROKEN_PART = 'tick,';
26+
const WARN_REG_EXP = /\(node:\d+\) \[BROKEN_PROFILE_FILE] Warning: Profile file .* is broken/;
27+
const WARN_DETAIL_REG_EXP = /".*tick," at the file end is broken/;
28+
29+
const code = `function f() {
30+
this.ts = Date.now();
31+
setImmediate(function() { new f(); });
32+
};
33+
f();`;
34+
35+
const proc = cp.spawn(process.execPath, [
36+
'--no_logfile_per_isolate',
37+
'--logfile=-',
38+
'--prof',
39+
'-pe', code
40+
], {
41+
stdio: ['ignore', 'pipe', 'inherit']
42+
});
43+
44+
let ticks = '';
45+
proc.stdout.on('data', (chunk) => ticks += chunk);
46+
47+
48+
function runPolyfill(content) {
49+
proc.kill();
50+
content += BROKEN_PART;
51+
fs.writeFileSync(LOG_FILE, content);
52+
const child = cp.spawnSync(
53+
`${process.execPath}`,
54+
[
55+
'--prof-process', LOG_FILE
56+
]);
57+
assert(WARN_REG_EXP.test(child.stderr.toString()));
58+
assert(WARN_DETAIL_REG_EXP.test(child.stderr.toString()));
59+
assert.strictEqual(child.status, 0);
60+
}
61+
62+
setTimeout(() => runPolyfill(ticks), RETRY_TIMEOUT);

test/tick-processor/test-tick-processor-preprocess-flag.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@ const common = require('../common');
44
if (!common.enoughTestCpu)
55
common.skip('test is CPU-intensive');
66

7-
if (common.isWindows ||
8-
common.isSunOS ||
9-
common.isAIX ||
10-
common.isLinuxPPCBE ||
11-
common.isFreeBSD)
7+
if (common.isCPPSymbolsNotMapped) {
128
common.skip('C++ symbols are not mapped for this os.');
9+
}
1310

1411
const base = require('./tick-processor-base.js');
1512

0 commit comments

Comments
 (0)