Fix send flow control bug
The send stream state is transitioned before data is buffered. As such, the stream state could be closed while there is still data to be sent.
This commit is contained in:
@@ -36,6 +36,8 @@ impl Settings {
|
|||||||
B: Buf,
|
B: Buf,
|
||||||
C: Buf,
|
C: Buf,
|
||||||
{
|
{
|
||||||
|
trace!("send_pending_ack; pending={:?}", self.pending);
|
||||||
|
|
||||||
if let Some(ref settings) = self.pending {
|
if let Some(ref settings) = self.pending {
|
||||||
let frame = frame::Settings::ack();
|
let frame = frame::Settings::ack();
|
||||||
|
|
||||||
@@ -44,7 +46,7 @@ impl Settings {
|
|||||||
return Ok(Async::NotReady);
|
return Ok(Async::NotReady);
|
||||||
}
|
}
|
||||||
|
|
||||||
trace!("ACK sent");
|
trace!("ACK sent; applying settings");
|
||||||
|
|
||||||
dst.apply_remote_settings(settings);
|
dst.apply_remote_settings(settings);
|
||||||
streams.apply_remote_settings(settings)?;
|
streams.apply_remote_settings(settings)?;
|
||||||
|
|||||||
@@ -159,10 +159,8 @@ impl<B> Prioritize<B>
|
|||||||
stream: &mut store::Ptr<B>)
|
stream: &mut store::Ptr<B>)
|
||||||
-> Result<(), ConnectionError>
|
-> Result<(), ConnectionError>
|
||||||
{
|
{
|
||||||
// Ignore window updates when the stream is not active.
|
trace!("recv_stream_window_update; stream={:?}; state={:?}; inc={}; flow={:?}",
|
||||||
if !stream.state.could_send_data() {
|
stream.id, stream.state, inc, stream.send_flow);
|
||||||
return Ok(());
|
|
||||||
}
|
|
||||||
|
|
||||||
// Update the stream level flow control.
|
// Update the stream level flow control.
|
||||||
stream.send_flow.inc_window(inc)?;
|
stream.send_flow.inc_window(inc)?;
|
||||||
@@ -222,8 +220,9 @@ impl<B> Prioritize<B>
|
|||||||
}
|
}
|
||||||
|
|
||||||
// If the stream has requested capacity, then it must be in the
|
// If the stream has requested capacity, then it must be in the
|
||||||
// streaming state.
|
// streaming state (more data could be sent) or there is buffered data
|
||||||
debug_assert!(stream.state.is_send_streaming());
|
// waiting to be sent.
|
||||||
|
debug_assert!(stream.state.is_send_streaming() || stream.buffered_send_data > 0);
|
||||||
|
|
||||||
// The amount of currently available capacity on the connection
|
// The amount of currently available capacity on the connection
|
||||||
let conn_available = self.flow.available();
|
let conn_available = self.flow.available();
|
||||||
|
|||||||
@@ -250,17 +250,21 @@ impl<B> Send<B> where B: Buf {
|
|||||||
if val < old_val {
|
if val < old_val {
|
||||||
let dec = old_val - val;
|
let dec = old_val - val;
|
||||||
|
|
||||||
|
trace!("decrementing all windows; dec={}", dec);
|
||||||
|
|
||||||
store.for_each(|mut stream| {
|
store.for_each(|mut stream| {
|
||||||
let stream = &mut *stream;
|
let stream = &mut *stream;
|
||||||
|
|
||||||
if stream.state.is_send_streaming() {
|
stream.send_flow.dec_window(dec);
|
||||||
stream.send_flow.dec_window(dec);
|
trace!("decremented stream window; id={:?}; decr={}; flow={:?}",
|
||||||
|
stream.id, dec, stream.send_flow);
|
||||||
|
|
||||||
// TODO: Handle reclaiming connection level window
|
// TODO: Probably try to assign capacity?
|
||||||
// capacity.
|
|
||||||
|
|
||||||
// TODO: Should this notify the producer?
|
// TODO: Handle reclaiming connection level window
|
||||||
}
|
// capacity.
|
||||||
|
|
||||||
|
// TODO: Should this notify the producer?
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
})?;
|
})?;
|
||||||
|
|||||||
@@ -213,6 +213,7 @@ impl State {
|
|||||||
match self.inner {
|
match self.inner {
|
||||||
Closed(..) => {}
|
Closed(..) => {}
|
||||||
_ => {
|
_ => {
|
||||||
|
trace!("recv_err; err={:?}", err);
|
||||||
self.inner = Closed(match *err {
|
self.inner = Closed(match *err {
|
||||||
ConnectionError::Proto(reason) => Some(Cause::Proto(reason)),
|
ConnectionError::Proto(reason) => Some(Cause::Proto(reason)),
|
||||||
ConnectionError::Io(..) => Some(Cause::Io),
|
ConnectionError::Io(..) => Some(Cause::Io),
|
||||||
@@ -264,16 +265,6 @@ impl State {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns true if the stream is in a state such that it could send data in
|
|
||||||
/// the future.
|
|
||||||
pub fn could_send_data(&self) -> bool {
|
|
||||||
match self.inner {
|
|
||||||
Open { .. } => true,
|
|
||||||
HalfClosedRemote(_) => true,
|
|
||||||
_ => false,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn is_send_streaming(&self) -> bool {
|
pub fn is_send_streaming(&self) -> bool {
|
||||||
match self.inner {
|
match self.inner {
|
||||||
Open { local: Peer::Streaming, .. } => true,
|
Open { local: Peer::Streaming, .. } => true,
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ pub(super) struct Stream<B> {
|
|||||||
pub requested_send_capacity: WindowSize,
|
pub requested_send_capacity: WindowSize,
|
||||||
|
|
||||||
/// Amount of data buffered at the prioritization layer.
|
/// Amount of data buffered at the prioritization layer.
|
||||||
|
/// TODO: Technically this could be greater than the window size...
|
||||||
pub buffered_send_data: WindowSize,
|
pub buffered_send_data: WindowSize,
|
||||||
|
|
||||||
/// Task tracking additional send capacity (i.e. window updates).
|
/// Task tracking additional send capacity (i.e. window updates).
|
||||||
|
|||||||
Reference in New Issue
Block a user