Commit cb0b53e
stream: lazy read ReadStream
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>1 parent 0c11a17 commit cb0b53e
3 files changed
Lines changed: 55 additions & 1 deletion
File tree
- lib/internal/streams
- test/parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -203,7 +203,9 @@ function Readable(options) { | |||
| 203 | 203 | Stream.call(this, options); | |
| 204 | 204 | ||
| 205 | 205 | destroyImpl.construct(this, () => { | |
| 206 | - maybeReadMore(this, this._readableState); | ||
| 206 | + if (this._readableState.needReadable) { | ||
| 207 | + maybeReadMore(this, this._readableState); | ||
| 208 | + } | ||
| 207 | 209 | }); | |
| 208 | 210 | } | |
| 209 | 211 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,42 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + const process = require('process'); | ||
| 4 | + | ||
| 5 | + let defaultShell; | ||
| 6 | + if (process.platform === 'linux' || process.platform === 'darwin') { | ||
| 7 | + defaultShell = '/bin/sh'; | ||
| 8 | + } else if (process.platform === 'win32') { | ||
| 9 | + defaultShell = 'cmd.exe'; | ||
| 10 | + } else { | ||
| 11 | + common.skip('This is test exists only on Linux/Win32/OSX'); | ||
| 12 | + } | ||
| 13 | + | ||
| 14 | + const { execSync } = require('child_process'); | ||
| 15 | + const fs = require('fs'); | ||
| 16 | + const path = require('path'); | ||
| 17 | + const tmpdir = require('../common/tmpdir'); | ||
| 18 | + | ||
| 19 | + const tmpDir = tmpdir.path; | ||
| 20 | + tmpdir.refresh(); | ||
| 21 | + const tmpCmdFile = path.join(tmpDir, 'test-stdin-from-file-spawn-cmd'); | ||
| 22 | + const tmpJsFile = path.join(tmpDir, 'test-stdin-from-file-spawn.js'); | ||
| 23 | + fs.writeFileSync(tmpCmdFile, 'echo hello'); | ||
| 24 | + fs.writeFileSync(tmpJsFile, ` | ||
| 25 | + 'use strict'; | ||
| 26 | + const { spawn } = require('child_process'); | ||
| 27 | + // Reference the object to invoke the getter | ||
| 28 | + process.stdin; | ||
| 29 | + setTimeout(() => { | ||
| 30 | + let ok = false; | ||
| 31 | + const child = spawn(process.env.SHELL || '${defaultShell}', | ||
| 32 | + [], { stdio: ['inherit', 'pipe'] }); | ||
| 33 | + child.stdout.on('data', () => { | ||
| 34 | + ok = true; | ||
| 35 | + }); | ||
| 36 | + child.on('close', () => { | ||
| 37 | + process.exit(ok ? 0 : -1); | ||
| 38 | + }); | ||
| 39 | + }, 100); | ||
| 40 | + `); | ||
| 41 | + | ||
| 42 | + execSync(`${process.argv[0]} ${tmpJsFile} < ${tmpCmdFile}`); | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -268,3 +268,13 @@ testDestroy((opts) => new Writable({ | |||
| 268 | 268 | assert.strictEqual(constructed, true); | |
| 269 | 269 | })); | |
| 270 | 270 | } | |
| 271 | + | ||
| 272 | + { | ||
| 273 | + // Construct should not cause stream to read. | ||
| 274 | + new Readable({ | ||
| 275 | + construct: common.mustCall((callback) => { | ||
| 276 | + callback(); | ||
| 277 | + }), | ||
| 278 | + read: common.mustNotCall() | ||
| 279 | + }); | ||
| 280 | + } | ||
0 commit comments