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.

PR-URL: nodejs#18641
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>

Author: killagu

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 '';
+    }
   }
 }
 

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>]
 

test/common/index.js

@@ -803,3 +803,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.isCPPSymbolsNotMapped = exports.isWindows ||
+                                exports.isSunOS ||
+                                exports.isAIX ||
+                                exports.isLinuxPPCBE ||
+                                exports.isFreeBSD;

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');
 

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');
 

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);

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');