Skip to content

stream: fix _final and 'prefinish' timing#32780

Closed
ronag wants to merge 3 commits intonodejs:masterfrom
nxtedition:stream-finish
Closed

stream: fix _final and 'prefinish' timing#32780
ronag wants to merge 3 commits intonodejs:masterfrom
nxtedition:stream-finish

Conversation

@ronag
Copy link
Copy Markdown
Member

@ronag ronag commented Apr 11, 2020

This PR fixes a few different things:

The timing of 'prefinish' depends on whether or not
_final is defined. In one case the event is emitted
synchronously with end() and otherwise asynchronously.

_final is currently unecessarily called asynchronously
which forces implementors to use 'prefinish' as a hack
to emulate synchronous behaviour. Furthermore, this hack
is subtly broken due to the above issue.

The stream should not finish if errored or destroyed
synchronously during the prefinish stage.

Refs: #31401
Refs: #32763 (comment)

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

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. semver-major PRs that contain breaking changes and should be released in the next major version. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants