Skip to content

Commit 197efb7

Browse files
gireeshpunathiladdaleax
authored andcommitted
child_process: close pipe ends that are re-piped
when t0 and t1 are spawned with t0's outputstream [1, 2] is piped into t1's input, a new pipe is created which uses a copy of the t0's fd. This leaves the original copy in Node parent, unattended. Net result is that when t0 produces data, it gets bifurcated into both the copies Detect the passed handle to be of 'wrap' type and close after the native spawn invocation by which time piping would have been over. Fixes: #9413 Fixes: #18016 PR-URL: #21209 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent c866b52 commit 197efb7

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

lib/internal/child_process.js

+6
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,12 @@ ChildProcess.prototype.spawn = function(options) {
383383
continue;
384384
}
385385

386+
// stream is already cloned and piped, so close
387+
if (stream.type === 'wrap') {
388+
stream.handle.close();
389+
continue;
390+
}
391+
386392
if (stream.handle) {
387393
// when i === 0 - we're dealing with stdin
388394
// (which is the only one writable pipe)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const path = require('path');
5+
const fs = require('fs');
6+
const spawn = require('child_process').spawn;
7+
const tmpdir = require('../common/tmpdir');
8+
9+
let cat, grep, wc;
10+
11+
const KB = 1024;
12+
const MB = KB * KB;
13+
14+
15+
// Make sure process chaining allows desired data flow:
16+
// check cat <file> | grep 'x' | wc -c === 1MB
17+
// This helps to make sure no data is lost between pipes.
18+
19+
{
20+
tmpdir.refresh();
21+
const file = path.resolve(tmpdir.path, 'data.txt');
22+
const buf = Buffer.alloc(MB).fill('x');
23+
24+
// Most OS commands that deal with data, attach special
25+
// meanings to new line - for example, line buffering.
26+
// So cut the buffer into lines at some points, forcing
27+
// data flow to be split in the stream.
28+
for (let i = 0; i < KB; i++)
29+
buf[i * KB] = 10;
30+
fs.writeFileSync(file, buf.toString());
31+
32+
cat = spawn('cat', [file]);
33+
grep = spawn('grep', ['x'], { stdio: [cat.stdout, 'pipe', 'pipe'] });
34+
wc = spawn('wc', ['-c'], { stdio: [grep.stdout, 'pipe', 'pipe'] });
35+
36+
wc.stdout.on('data', common.mustCall(function(data) {
37+
assert.strictEqual(data.toString().trim(), MB.toString());
38+
}));
39+
40+
cat.on('exit', common.mustCall(function(code) {
41+
assert.strictEqual(code, 0);
42+
}));
43+
44+
grep.on('exit', common.mustCall(function(code) {
45+
assert.strictEqual(code, 0);
46+
}));
47+
48+
wc.on('exit', common.mustCall(function(code) {
49+
assert.strictEqual(code, 0);
50+
}));
51+
}

0 commit comments

Comments
 (0)