Skip to content

Commit

Permalink
Assert that stream state is released
Browse files Browse the repository at this point in the history
This patch adds a debug level assertion checking that state associated
with streams is successfully released. This is done by ensuring that no
stream state remains when the connection inner state is dropped. This
does not happen until all handles to the connection drops, which should
result in all streams being dropped.

This also pulls in a fix for one bug that is exposed with this change.
The fix is first proposed as part of #261.
  • Loading branch information
carllerche committed May 6, 2018
1 parent 3e1de2f commit 44c9005
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/proto/streams/prioritize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,21 +613,22 @@ impl Prioritize {
trace!("pop_frame; stream={:?}; stream.state={:?}",
stream.id, stream.state);

// It's possible that this stream, besides having data to send,
// is also queued to send a reset, and thus is already in the queue
// to wait for "some time" after a reset.
//
// To be safe, we just always ask the stream.
let is_pending_reset = stream.is_pending_reset_expiration();

// If the stream receives a RESET from the peer, it may have
// had data buffered to be sent, but all the frames are cleared
// in clear_queue(). Instead of doing O(N) traversal through queue
// to remove, lets just ignore peer_reset streams here.
if stream.state.is_peer_reset() {
counts.transition_after(stream, is_pending_reset);
continue;
}

// It's possible that this stream, besides having data to send,
// is also queued to send a reset, and thus is already in the queue
// to wait for "some time" after a reset.
//
// To be safe, we just always ask the stream.
let is_pending_reset = stream.is_pending_reset_expiration();

trace!(" --> stream={:?}; is_pending_reset={:?};",
stream.id, is_pending_reset);

Expand Down
4 changes: 4 additions & 0 deletions src/proto/streams/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ impl Store {
}
}

pub fn is_empty(&self) -> bool {
self.slab.is_empty()
}

pub fn find_mut(&mut self, id: &StreamId) -> Option<Ptr> {
let key = match self.ids.get(id) {
Some(key) => *key,
Expand Down
10 changes: 10 additions & 0 deletions src/proto/streams/streams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,3 +1099,13 @@ impl Actions {
}
}
}

impl Drop for Inner {
fn drop(&mut self) {
use std::thread;

if !thread::panicking() {
debug_assert!(self.store.is_empty());
}
}
}

0 comments on commit 44c9005

Please sign in to comment.