Skip to content

stream: pipeline should only destroy un-finished streams.

Rodrigo Muino Tomonari requested to merge github/fork/nxtedition/stream-pipeline-destroy2 into master

This PR logically reverts https://github.com/nodejs/node/pull/31940 which has caused lots of unnecessary breakage in the ecosystem.

This PR also aligns better with the actual documented behavior:

stream.pipeline() will call stream.destroy(err) on all streams except:

  • Readable streams which have emitted 'end' or 'close'.
  • Writable streams which have emitted 'finish' or 'close'.

The behavior introduced in https://github.com/nodejs/node/pull/31940 was much more aggressive in terms of destroying streams. This was good for avoiding potential resources leaks however breaks some common assumputions in legacy streams.

Furthermore, it makes the code simpler and removes some hacks.

Fixes: https://github.com/nodejs/node/issues/32954 Fixes: https://github.com/nodejs/node/issues/32955

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports