implement h2::server::Stream::send_reset(Reason) and Body::is_empty() #22
Conversation
src/proto/streams/state.rs
Outdated
| Idle => Err(ProtocolError.into()), | ||
| Closed(..) => Ok(()), | ||
| _ => { | ||
| trace!("send_reset: HalfClosedRemote => Closed"); |
olix0r
changed the title
src/proto/streams/streams.rs
Outdated
|
|
||
| let stream = me.store.resolve(self.key); | ||
|
|
||
| stream.state.is_recv_closed() && me.actions.recv.is_empty() |
There was a problem hiding this comment.
Is this correct? It seems to be checking that there are no buffered frames on the entire connection vs. stream?
src/proto/streams/buffer.rs
Outdated
| @@ -36,6 +36,14 @@ impl<B> Buffer<B> { | |||
| slab: Slab::new(), | |||
| } | |||
| } | |||
|
|
|||
| pub fn len(&self) -> usize { | |||
There was a problem hiding this comment.
Looks like these fns aren't used anymore?
src/proto/streams/recv.rs
Outdated
| @@ -74,6 +74,14 @@ impl<B> Recv<B> where B: Buf { | |||
| } | |||
| } | |||
|
|
|||
| pub fn is_empty(&self) -> bool { | |||
There was a problem hiding this comment.
These aren't used anymore either?
src/proto/streams/send.rs
Outdated
| stream: &mut store::Ptr<B>) | ||
| -> Result<(), ConnectionError> | ||
| { | ||
| stream.state.send_reset(reason)?; |
There was a problem hiding this comment.
It looks like the reset will still be sent if the stream state is already closed (potentially reset already). Is this expected?
src/proto/streams/streams.rs
Outdated
|
|
||
| let stream = me.store.resolve(self.key); | ||
|
|
||
| stream.state.is_recv_closed() && stream.pending_recv.is_empty() |
There was a problem hiding this comment.
stream.pending_recv could only contain a trailers frame, and is_recv_empty() would return false, yet there are no more data frames?
There was a problem hiding this comment.
is it worth just tracking a bool for this?
There was a problem hiding this comment.
Actually, i think this is okay if it's only meant to determine whether eos may be set...
src/proto/streams/send.rs
Outdated
| stream: &mut store::Ptr<B>) | ||
| -> Result<(), ConnectionError> | ||
| { | ||
| if !stream.state.is_closed() { |
There was a problem hiding this comment.
but it should perhaps be an error to send a reset on a closed stream...
src/lib.rs
Outdated
| impl<B: IntoBuf> Body<B> { | ||
| pub fn is_empty(&self) -> bool { | ||
| // If the recv side is closed and the receive queue is empty, the body is empty. | ||
| self.inner.is_recv_eos() |
There was a problem hiding this comment.
As per the comment thread below, this will return false if the body is consumed and the next pending frame is a trailer.
There was a problem hiding this comment.
i think that's correct fsvo is_empty. I intend to expose "is there a body at all" and not "is there remaining body to be consumed". i should probably rename this
|
Merging. #yolo |
No description provided.