1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

child_process: measure buffer length in bytes

Browse Source 
This change fixes a known issue where `maxBuffer` limits by characters
rather than bytes. Benchmark added to confirm no performance regression
occurs with this change.

PR-URL: https://github.com/nodejs/node/pull/6764
Fixes: https://github.com/nodejs/node/issues/1901
Reviewed-By: Brian White <mscdex@mscdex.net>
This commit is contained in:
Rich Trott 2016-05-14 15:24:34 -07:00
parent 33c7b45378
commit c9a5990a76
5 changed files with 96 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
benchmark/child_process/child-process-exec-stdout.js Normal file
View File

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

18
benchmark/child_process/child-process-read.js
View File

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

71
lib/child_process.js
View File

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

15
test/parallel/test-child-process-maxBuffer-stderr.js Normal file
View File

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

3
test/known_issues/test-child-process-max-buffer.js → test/parallel/test-child-process-maxBuffer-stdout.js
View File

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