Skip to content

Commit

Permalink
stream: lazy read ReadStream
Browse files Browse the repository at this point in the history
Using stream._construct would cause the Readable
to incorrectly greedily start reading.

Fixes: #36251

PR-URL: #36823
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
mmomtchev authored and danielleadams committed Jan 12, 2021
1 parent 0c11a17 commit cb0b53e
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/internal/streams/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ function Readable(options) {
Stream.call(this, options);

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

Expand Down
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
Expand Up @@ -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 cb0b53e

Please sign in to comment.