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.
nodejs · Feb 11, 2018 · 33f4a51 · 33f4a51
1 parent 3e8af96
commit 33f4a51
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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');