From 23b724ad0679dd0fedd4f9a7be5dcab04079a242 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 25 Apr 2020 16:46:47 +0200 Subject: [PATCH 1/2] stream: don't wait for close on legacy streams 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: https://github.com/nodejs/node/issues/33050 --- lib/internal/streams/end-of-stream.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index f1f1b3e5a77877..7d5689ddadb7fc 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -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 && @@ -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); }; @@ -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); }; From aef90e36c252e92cffffaf0ac61002cd3feff5f1 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 25 Apr 2020 16:52:45 +0200 Subject: [PATCH 2/2] fixup: test --- test/parallel/test-stream-finished.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index 17ab976c6136f4..d4336e84db35d6 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -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); +}