Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
stdio: lazy read ReadStream
Browse files Browse the repository at this point in the history
All stdio ReadStream's use manual start to avoid
consuming data for example when a process
execs/spawns.

Using stream._construct would cause the Readable
to incorrectly greedily start reading.

Refs: #36251
mmomtchev authored and ronag committed Jan 6, 2021
1 parent db79783 commit e8517c2
Showing 3 changed files with 59 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/internal/streams/readable.js
Original file line number Diff line number Diff line change
@@ -166,6 +166,8 @@ function ReadableState(options, stream, isDuplex) {
// If true, a maybeReadMore has been scheduled.
this.readingMore = false;

this.didRead = false;

this.decoder = null;
this.encoding = null;
if (options && options.encoding) {
@@ -203,7 +205,9 @@ function Readable(options) {
Stream.call(this, options);

destroyImpl.construct(this, () => {
maybeReadMore(this, this._readableState);
if (this._readableState.didRead) {
maybeReadMore(this, this._readableState);
}
});
}

@@ -401,6 +405,8 @@ Readable.prototype.read = function(n) {
const state = this._readableState;
const nOrig = n;

this.didRead = true;

// If we're asking for more than the current hwm, then raise the hwm.
if (n > state.highWaterMark)
state.highWaterMark = computeNewHighWaterMark(n);
42 changes: 42 additions & 0 deletions test/parallel/test-stdin-from-file-spawn.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';
const common = require('../common');
const process = require('process');

let defaultShell;
if (process.platform === 'linux' || process.platform === 'darwin') {
defaultShell = '/bin/sh';
} else if (process.platform === 'win32') {
defaultShell = 'cmd.exe';
} else {
common.skip('This is test exists only on Linux/Win32/OSX');
}

const { execSync } = require('child_process');
const fs = require('fs');
const path = require('path');
const tmpdir = require('../common/tmpdir');

const tmpDir = tmpdir.path;
tmpdir.refresh();
const tmpCmdFile = path.join(tmpDir, 'test-stdin-from-file-spawn-cmd');
const tmpJsFile = path.join(tmpDir, 'test-stdin-from-file-spawn.js');
fs.writeFileSync(tmpCmdFile, 'echo hello');
fs.writeFileSync(tmpJsFile, `
'use strict';
const { spawn } = require('child_process');
// Reference the object to invoke the getter
process.stdin;
setTimeout(() => {
let ok = false;
const child = spawn(process.env.SHELL || '${defaultShell}',
[], { stdio: ['inherit', 'pipe'] });
child.stdout.on('data', () => {
ok = true;
});
child.on('close', () => {
process.exit(ok ? 0 : -1);
});
}, 100);
`);

execSync(`${process.argv[0]} ${tmpJsFile} < ${tmpCmdFile}`);
10 changes: 10 additions & 0 deletions test/parallel/test-stream-construct.js
Original file line number Diff line number Diff line change
@@ -268,3 +268,13 @@ testDestroy((opts) => new Writable({
assert.strictEqual(constructed, true);
}));
}

{
// Construct should not cause stream to read.
new Readable({
construct: common.mustCall((callback) => {
callback();
}),
read: common.mustNotCall()
});
}

0 comments on commit e8517c2

Please sign in to comment.