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 27, 2020
1 parent f557279 commit f2c746d
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
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
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 f2c746d

Please sign in to comment.