Commit 83ec33b9 authored by Anna Henningsen's avatar Anna Henningsen Committed by Rich Trott
Browse files

stream: fix end-of-stream for HTTP/2

HTTP/2 streams call `.end()` on themselves from their
`.destroy()` method, which might be queued (e.g. due to network
congestion) and not processed before the stream itself is destroyed.

In that case, the `_writableState.ended` property could be set before
the stream emits its `'close'` event, and never actually emits the
`'finished'` event, confusing the end-of-stream implementation so
that it wouldn’t call its callback.

This can be fixed by watching for the end events themselves using the
existing `'finish'` and `'end'` listeners rather than relying on the
`.ended` properties of the `_...State` objects.

These properties still need to be checked to know whether stream
closure was premature – My understanding is that ideally, streams
should not emit `'close'` before `'end'` and/or `'finished'`, so this
might be another bug, but changing this would require modifying tests
and almost certainly be a breaking change.

Fixes: https://github.com/nodejs/node/issues/24456

PR-URL: https://github.com/nodejs/node/pull/24926


Reviewed-By: default avatarRich Trott <rtrott@gmail.com>
Reviewed-By: default avatarLuigi Pinca <luigipinca@gmail.com>
Reviewed-By: default avatarMatteo Collina <matteo.collina@gmail.com>
Reviewed-By: default avatarFranziska Hinkelmann <franziska.hinkelmann@gmail.com>
parent 2f75eed1
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment