Skip to content

Commit

Permalink
stream: do not call _read() after destroy()
Browse files Browse the repository at this point in the history
PR-URL: nodejs#29491
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
  • Loading branch information
ronag authored and Trott committed Sep 22, 2019
1 parent 8709a40 commit ec390b6
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 7 deletions.
7 changes: 4 additions & 3 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,10 @@ Readable.prototype.read = function(n) {
debug('length less than watermark', doRead);
}

// However, if we've ended, then there's no point, and if we're already
// reading, then it's unnecessary.
if (state.ended || state.reading) {
// However, if we've ended, then there's no point, if we're already
// reading, then it's unnecessary, and if we're destroyed, then it's
// not allowed.
if (state.ended || state.reading || state.destroyed) {
doRead = false;
debug('reading or ended', doRead);
} else if (doRead) {
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ ReadStream.prototype._read = function(n) {
});
}

if (this.destroyed)
return;

if (!pool || pool.length - pool.used < kMinPoolSpace) {
// Discard the old pool.
allocNewPool(this.readableHighWaterMark);
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-stream-readable-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,12 @@ const assert = require('assert');
read.push('hi');
read.on('data', common.mustNotCall());
}

{
const read = new Readable({
read: common.mustNotCall(function() {})
});
read.destroy();
assert.strictEqual(read.destroyed, true);
read.read();
}
2 changes: 1 addition & 1 deletion test/parallel/test-wrap-js-stream-exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ process.once('uncaughtException', common.mustCall((err) => {
}));

const socket = new JSStreamWrap(new Duplex({
read: common.mustCall(),
read: common.mustNotCall(),
write: common.mustCall((buffer, data, cb) => {
throw new Error('exception!');
})
Expand Down

0 comments on commit ec390b6

Please sign in to comment.