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

Projects
None yet
3 participants
@goffrie
Copy link
Contributor

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.

Avoid reclaiming frames for dead streams.
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.

@goffrie goffrie referenced this pull request Apr 20, 2018

Closed

Fuzzing with honggfuzz-rs #263

@carllerche

This comment has been minimized.

Copy link
Owner

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.

@carllerche carllerche merged commit 558e6b6 into carllerche:master Apr 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.