diff options
| -rw-r--r-- | benchmark/child_process/child-process-exec-stdout.js | 30 | ||||
| -rw-r--r-- | benchmark/child_process/child-process-read.js | 18 | ||||
| -rw-r--r-- | lib/child_process.js | 71 | ||||
| -rw-r--r-- | test/parallel/test-child-process-maxBuffer-stderr.js | 15 | ||||
| -rw-r--r-- | test/parallel/test-child-process-maxBuffer-stdout.js (renamed from test/known_issues/test-child-process-max-buffer.js) | 3 |
5 files changed, 96 insertions, 41 deletions
diff --git a/benchmark/child_process/child-process-exec-stdout.js b/benchmark/child_process/child-process-exec-stdout.js new file mode 100644 index 00000000000..79efb6fadf2 --- /dev/null +++ b/benchmark/child_process/child-process-exec-stdout.js @@ -0,0 +1,30 @@ +'use strict'; +const common = require('../common.js'); +const bench = common.createBenchmark(main, { + len: [64, 256, 1024, 4096, 32768], + dur: [5] +}); + +const exec = require('child_process').exec; +function main(conf) { + bench.start(); + + const dur = +conf.dur; + const len = +conf.len; + + const msg = `"${'.'.repeat(len)}"`; + msg.match(/./); + const options = {'stdio': ['ignore', 'pipe', 'ignore']}; + // NOTE: Command below assumes bash shell. + const child = exec(`while\n echo ${msg}\ndo :; done\n`, options); + + var bytes = 0; + child.stdout.on('data', function(msg) { + bytes += msg.length; + }); + + setTimeout(function() { + child.kill(); + bench.end(bytes); + }, dur * 1000); +} diff --git a/benchmark/child_process/child-process-read.js b/benchmark/child_process/child-process-read.js index c1a7474aae3..33c268390fd 100644 --- a/benchmark/child_process/child-process-read.js +++ b/benchmark/child_process/child-process-read.js @@ -1,20 +1,20 @@ 'use strict'; -var common = require('../common.js'); -var bench = common.createBenchmark(main, { +const common = require('../common.js'); +const bench = common.createBenchmark(main, { len: [64, 256, 1024, 4096, 32768], dur: [5] }); -var spawn = require('child_process').spawn; +const spawn = require('child_process').spawn; function main(conf) { bench.start(); - var dur = +conf.dur; - var len = +conf.len; + const dur = +conf.dur; + const len = +conf.len; - var msg = '"' + Array(len).join('.') + '"'; - var options = { 'stdio': ['ignore', 'ipc', 'ignore'] }; - var child = spawn('yes', [msg], options); + const msg = '"' + Array(len).join('.') + '"'; + const options = {'stdio': ['ignore', 'ipc', 'ignore']}; + const child = spawn('yes', [msg], options); var bytes = 0; child.on('message', function(msg) { @@ -23,7 +23,7 @@ function main(conf) { setTimeout(function() { child.kill(); - var gbits = (bytes * 8) / (1024 * 1024 * 1024); + const gbits = (bytes * 8) / (1024 * 1024 * 1024); bench.end(gbits); }, dur * 1000); } diff --git a/lib/child_process.js b/lib/child_process.js index 0c6af12bc8e..5eee5228f04 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -146,15 +146,13 @@ exports.execFile = function(file /*, args, options, callback*/) { }); var encoding; - var _stdout; - var _stderr; + var stdoutState; + var stderrState; + var _stdout = []; + var _stderr = []; if (options.encoding !== 'buffer' && Buffer.isEncoding(options.encoding)) { encoding = options.encoding; - _stdout = ''; - _stderr = ''; } else { - _stdout = []; - _stderr = []; encoding = null; } var stdoutLen = 0; @@ -176,16 +174,23 @@ exports.execFile = function(file /*, args, options, callback*/) { if (!callback) return; - // merge chunks - var stdout; - var stderr; - if (!encoding) { - stdout = Buffer.concat(_stdout); - stderr = Buffer.concat(_stderr); - } else { - stdout = _stdout; - stderr = _stderr; - } + var stdout = Buffer.concat(_stdout, stdoutLen); + var stderr = Buffer.concat(_stderr, stderrLen); + + var stdoutEncoding = encoding; + var stderrEncoding = encoding; + + if (stdoutState && stdoutState.decoder) + stdoutEncoding = stdoutState.decoder.encoding; + + if (stderrState && stderrState.decoder) + stderrEncoding = stderrState.decoder.encoding; + + if (stdoutEncoding) + stdout = stdout.toString(stdoutEncoding); + + if (stderrEncoding) + stderr = stderr.toString(stderrEncoding); if (ex) { // Will be handled later @@ -245,39 +250,45 @@ exports.execFile = function(file /*, args, options, callback*/) { } if (child.stdout) { - if (encoding) - child.stdout.setEncoding(encoding); + stdoutState = child.stdout._readableState; child.stdout.addListener('data', function(chunk) { - stdoutLen += chunk.length; + // If `child.stdout.setEncoding()` happened in userland, convert string to + // Buffer. + if (stdoutState.decoder) { + chunk = Buffer.from(chunk, stdoutState.decoder.encoding); + } + + stdoutLen += chunk.byteLength; if (stdoutLen > options.maxBuffer) { ex = new Error('stdout maxBuffer exceeded'); + stdoutLen -= chunk.byteLength; kill(); } else { - if (!encoding) - _stdout.push(chunk); - else - _stdout += chunk; + _stdout.push(chunk); } }); } if (child.stderr) { - if (encoding) - child.stderr.setEncoding(encoding); + stderrState = child.stderr._readableState; child.stderr.addListener('data', function(chunk) { - stderrLen += chunk.length; + // If `child.stderr.setEncoding()` happened in userland, convert string to + // Buffer. + if (stderrState.decoder) { + chunk = Buffer.from(chunk, stderrState.decoder.encoding); + } + + stderrLen += chunk.byteLength; if (stderrLen > options.maxBuffer) { ex = new Error('stderr maxBuffer exceeded'); + stderrLen -= chunk.byteLength; kill(); } else { - if (!encoding) - _stderr.push(chunk); - else - _stderr += chunk; + _stderr.push(chunk); } }); } diff --git a/test/parallel/test-child-process-maxBuffer-stderr.js b/test/parallel/test-child-process-maxBuffer-stderr.js new file mode 100644 index 00000000000..ecaea8b152a --- /dev/null +++ b/test/parallel/test-child-process-maxBuffer-stderr.js @@ -0,0 +1,15 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const unicode = '中文测试'; // Length = 4, Byte length = 13 + +if (process.argv[2] === 'child') { + console.error(unicode); +} else { + const cmd = `${process.execPath} ${__filename} child`; + + cp.exec(cmd, {maxBuffer: 10}, common.mustCall((err, stdout, stderr) => { + assert.strictEqual(err.message, 'stderr maxBuffer exceeded'); + })); +} diff --git a/test/known_issues/test-child-process-max-buffer.js b/test/parallel/test-child-process-maxBuffer-stdout.js index 14a344c7062..122dc499f46 100644 --- a/test/known_issues/test-child-process-max-buffer.js +++ b/test/parallel/test-child-process-maxBuffer-stdout.js @@ -1,5 +1,4 @@ 'use strict'; -// Refs: https://github.com/nodejs/node/issues/1901 const common = require('../common'); const assert = require('assert'); const cp = require('child_process'); @@ -10,7 +9,7 @@ if (process.argv[2] === 'child') { } else { const cmd = `${process.execPath} ${__filename} child`; - cp.exec(cmd, { maxBuffer: 10 }, common.mustCall((err, stdout, stderr) => { + cp.exec(cmd, {maxBuffer: 10}, common.mustCall((err, stdout, stderr) => { assert.strictEqual(err.message, 'stdout maxBuffer exceeded'); })); } |