Commit 7793c36
child_process: flush consuming streams
When a client calls read() with a nonzero argument
on a Socket, that Socket sets this._consuming to true.
It never sets this._consuming back to false.
ChildProcess.flushStdio() currently doesn't flush
any streams where _consuming is truthy. But, that means
that it never flushes any stream that has ever been read from.
This prevents a child process from ever closing if one of
its streams has been read from, causing issue #4049. This
commit allows consuming streams to be flushed, and the
child process to emit a close event.
Fixes: #4049
PR-URL: #4071
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>1 parent 22b0971 commit 7793c36
2 files changed
Lines changed: 18 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -217,7 +217,7 @@ util.inherits(ChildProcess, EventEmitter); | |||
| 217 | 217 | function flushStdio(subprocess) { | |
| 218 | 218 | if (subprocess.stdio == null) return; | |
| 219 | 219 | subprocess.stdio.forEach(function(stream, fd, stdio) { | |
| 220 | - if (!stream || !stream.readable || stream._consuming) | ||
| 220 | + if (!stream || !stream.readable) | ||
| 221 | 221 | return; | |
| 222 | 222 | stream.resume(); | |
| 223 | 223 | }); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,17 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const cp = require('child_process'); | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const assert = require('assert'); | ||
| 5 | + | ||
| 6 | + const p = cp.spawn('echo'); | ||
| 7 | + | ||
| 8 | + p.on('close', common.mustCall(function(code, signal) { | ||
| 9 | + assert.strictEqual(code, 0); | ||
| 10 | + assert.strictEqual(signal, null); | ||
| 11 | + })); | ||
| 12 | + | ||
| 13 | + p.stdout.read(); | ||
| 14 | + | ||
| 15 | + setTimeout(function() { | ||
| 16 | + p.kill(); | ||
| 17 | + }, 100); | ||
0 commit comments