Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid reclaiming frames for dead streams. #262

Merged
merged 2 commits into from Apr 24, 2018

Conversation

@goffrie
Copy link
Contributor

@goffrie goffrie commented Apr 20, 2018

In clear_queue we drop all the queued frames for a stream, but this doesn't
take into account a buffered frame inside of the FramedWrite. This can lead
to a panic when reclaim_frame tries to recover a frame onto a stream that has
already been destroyed, or in general cause wrong behaviour.

Instead, let's keep track of what frame is currently in-flight; then, when we
clear_queue a stream with an in-flight data frame, mark the frame to be
dropped instead of reclaimed.

In `clear_queue` we drop all the queued frames for a stream, but this doesn't
take into account a buffered frame inside of the `FramedWrite`. This can lead
to a panic when `reclaim_frame` tries to recover a frame onto a stream that has
already been destroyed, or in general cause wrong behaviour.

Instead, let's keep track of what frame is currently in-flight; then, when we
`clear_queue` a stream with an in-flight data frame, mark the frame to be
dropped instead of reclaimed.
@carllerche
Copy link
Collaborator

@carllerche carllerche commented Apr 24, 2018

Quick update, this change looks good to me. I've been trying to write a test that catches this. It has not been trivial, so I am going to keep working on that.

Loading

@carllerche carllerche merged commit 558e6b6 into hyperium:master Apr 24, 2018
1 check passed
Loading
@goffrie goffrie deleted the inflight-data branch Mar 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants