Skip to content

Commit

Permalink
stream: don't wait for close on legacy streams
Browse files Browse the repository at this point in the history
Try to detect non standard streams and don't wait for
'close' on these. In particular if we detected
that destroyed is true before we expect it to be then
fallback to compat behavior.

Fixes: #33050

PR-URL: #33058
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
  • Loading branch information
ronag authored and BethGriggs committed Apr 28, 2020
1 parent fad188f commit 184e80a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
12 changes: 11 additions & 1 deletion lib/internal/streams/end-of-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function eos(stream, opts, callback) {
// TODO (ronag): Improve soft detection to include core modules and
// common ecosystem modules that do properly emit 'close' but fail
// this generic check.
const willEmitClose = (
let willEmitClose = (
state &&
state.autoDestroy &&
state.emitClose &&
Expand All @@ -85,6 +85,11 @@ function eos(stream, opts, callback) {
(wState && wState.finished);
const onfinish = () => {
writableFinished = true;
// Stream should not be destroyed here. If it is that
// means that user space is doing something differently and
// we cannot trust willEmitClose.
if (stream.destroyed) willEmitClose = false;

if (willEmitClose && (!stream.readable || readable)) return;
if (!readable || readableEnded) callback.call(stream);
};
Expand All @@ -93,6 +98,11 @@ function eos(stream, opts, callback) {
(rState && rState.endEmitted);
const onend = () => {
readableEnded = true;
// Stream should not be destroyed here. If it is that
// means that user space is doing something differently and
// we cannot trust willEmitClose.
if (stream.destroyed) willEmitClose = false;

if (willEmitClose && (!stream.writable || writable)) return;
if (!writable || writableFinished) callback.call(stream);
};
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-stream-finished.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,3 +384,15 @@ testClosed((opts) => new Writable({ write() {}, ...opts }));

d.resume();
}

{
// Test for compat for e.g. fd-slicer which implements
// non standard destroy behavior which might not emit
// 'close'.
const r = new Readable();
finished(r, common.mustCall());
r.resume();
r.push('asd');
r.destroyed = true;
r.push(null);
}

0 comments on commit 184e80a

Please sign in to comment.