Skip to content

Commit 571bb14

Browse files
goffriecarllerche
authored andcommitted
Be more lenient with streams in the pending_send queue. (#261)
The `is_peer_reset()` check doesn't quite cover all the cases where we call `clear_queue`, such as when we call `recv_err`. Instead of trying to make the check more precise, let's gracefully handle spurious entries in the queue.
1 parent cf62b78 commit 571bb14

File tree

2 files changed

+17
-23
lines changed

2 files changed

+17
-23
lines changed

src/proto/streams/prioritize.rs

+17-16
Original file line numberDiff line numberDiff line change
@@ -647,15 +647,6 @@ impl Prioritize {
647647
// To be safe, we just always ask the stream.
648648
let is_pending_reset = stream.is_pending_reset_expiration();
649649

650-
// If the stream receives a RESET from the peer, it may have
651-
// had data buffered to be sent, but all the frames are cleared
652-
// in clear_queue(). Instead of doing O(N) traversal through queue
653-
// to remove, lets just ignore peer_reset streams here.
654-
if stream.state.is_peer_reset() {
655-
counts.transition_after(stream, is_pending_reset);
656-
continue;
657-
}
658-
659650
trace!(" --> stream={:?}; is_pending_reset={:?};",
660651
stream.id, is_pending_reset);
661652

@@ -757,13 +748,23 @@ impl Prioritize {
757748
)
758749
),
759750
None => {
760-
let reason = stream.state.get_scheduled_reset()
761-
.expect("must be scheduled to reset");
762-
763-
stream.state.set_reset(reason);
764-
765-
let frame = frame::Reset::new(stream.id, reason);
766-
Frame::Reset(frame)
751+
if let Some(reason) = stream.state.get_scheduled_reset() {
752+
stream.state.set_reset(reason);
753+
754+
let frame = frame::Reset::new(stream.id, reason);
755+
Frame::Reset(frame)
756+
} else {
757+
// If the stream receives a RESET from the peer, it may have
758+
// had data buffered to be sent, but all the frames are cleared
759+
// in clear_queue(). Instead of doing O(N) traversal through queue
760+
// to remove, lets just ignore the stream here.
761+
trace!("removing dangling stream from pending_send");
762+
// Since this should only happen as a consequence of `clear_queue`,
763+
// we must be in a closed state of some kind.
764+
debug_assert!(stream.state.is_closed());
765+
counts.transition_after(stream, is_pending_reset);
766+
continue;
767+
}
767768
}
768769
};
769770

src/proto/streams/state.rs

-7
Original file line numberDiff line numberDiff line change
@@ -325,13 +325,6 @@ impl State {
325325
}
326326
}
327327

328-
pub fn is_peer_reset(&self) -> bool {
329-
match self.inner {
330-
Closed(Cause::Proto(_)) => true,
331-
_ => false,
332-
}
333-
}
334-
335328
/// Returns true if the stream is already reset.
336329
pub fn is_reset(&self) -> bool {
337330
match self.inner {

0 commit comments

Comments
 (0)