From 77681674e2b2c1e3da0bc74b127d531ea0d56fd4 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 2 Aug 2017 13:43:14 -0700 Subject: [PATCH 01/19] Start prepping for Sync --- src/proto/streams/mod.rs | 243 +------------------------------- src/proto/streams/stream.rs | 0 src/proto/streams/streams.rs | 266 +++++++++++++++++++++++++++++++++++ 3 files changed, 270 insertions(+), 239 deletions(-) create mode 100644 src/proto/streams/stream.rs create mode 100644 src/proto/streams/streams.rs diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index 931adbccf..27bf15784 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -3,6 +3,10 @@ mod recv; mod send; mod state; mod store; +mod stream; +mod streams; + +pub use self::streams::Streams; use self::flow_control::FlowControl; use self::recv::Recv; @@ -15,30 +19,6 @@ use proto::*; use error::Reason::*; use error::User::*; -// TODO: All the VecDeques should become linked lists using the State -// values. -#[derive(Debug)] -pub struct Streams

{ - /// State related to managing the set of streams. - inner: Inner

, - - /// Streams - streams: Store, -} - -/// Fields needed to manage state related to managing the set of streams. This -/// is mostly split out to make ownership happy. -/// -/// TODO: better name -#[derive(Debug)] -struct Inner

{ - /// Manages state transitions initiated by receiving frames - recv: Recv

, - - /// Manages state transitions initiated by sending frames - send: Send

, -} - #[derive(Debug)] pub struct Config { /// Maximum number of remote initiated streams @@ -53,218 +33,3 @@ pub struct Config { /// Initial window size of locally initiated streams pub init_local_window_sz: WindowSize, } - -impl Streams

{ - pub fn new(config: Config) -> Self { - Streams { - inner: Inner { - recv: Recv::new(&config), - send: Send::new(&config), - }, - streams: Store::new(), - } - } - - pub fn recv_headers(&mut self, frame: frame::Headers) - -> Result, ConnectionError> - { - let id = frame.stream_id(); - - let state = match self.streams.entry(id) { - Entry::Occupied(e) => e.into_mut(), - Entry::Vacant(e) => { - // Trailers cannot open a stream. Trailers are header frames - // that do not contain pseudo headers. Requests MUST contain a - // method and responses MUST contain a status. If they do not,t - // hey are considered to be malformed. - if frame.is_trailers() { - return Err(ProtocolError.into()); - } - - match try!(self.inner.recv.open(id)) { - Some(state) => e.insert(state), - None => return Ok(None), - } - } - }; - - if frame.is_trailers() { - if !frame.is_end_stream() { - // TODO: What error should this return? - unimplemented!(); - } - - try!(self.inner.recv.recv_eos(state)); - } else { - try!(self.inner.recv.recv_headers(state, frame.is_end_stream())); - } - - if state.is_closed() { - self.inner.dec_num_streams(id); - } - - Ok(Some(frame)) - } - - pub fn recv_data(&mut self, frame: &frame::Data) - -> Result<(), ConnectionError> - { - let id = frame.stream_id(); - - let state = match self.streams.get_mut(&id) { - Some(state) => state, - None => return Err(ProtocolError.into()), - }; - - // Ensure there's enough capacity on the connection before acting on the - // stream. - try!(self.inner.recv.recv_data(frame, state)); - - if state.is_closed() { - self.inner.dec_num_streams(id); - } - - Ok(()) - } - - pub fn recv_reset(&mut self, _frame: &frame::Reset) - -> Result<(), ConnectionError> - { - unimplemented!(); - } - - pub fn recv_window_update(&mut self, frame: frame::WindowUpdate) - -> Result<(), ConnectionError> { - let id = frame.stream_id(); - - if id.is_zero() { - try!(self.inner.send.recv_connection_window_update(frame)); - } else { - // The remote may send window updates for streams that the local now - // considers closed. It's ok... - if let Some(state) = self.streams.get_mut(&id) { - try!(self.inner.send.recv_stream_window_update(frame, state)); - } - } - - Ok(()) - } - - pub fn recv_push_promise(&mut self, _frame: frame::PushPromise) - -> Result<(), ConnectionError> - { - unimplemented!(); - } - - pub fn send_headers(&mut self, frame: &frame::Headers) - -> Result<(), ConnectionError> - { - let id = frame.stream_id(); - - trace!("send_headers; id={:?}", id); - - let state = match self.streams.entry(id) { - Entry::Occupied(e) => e.into_mut(), - Entry::Vacant(e) => { - // Trailers cannot open a stream. Trailers are header frames - // that do not contain pseudo headers. Requests MUST contain a - // method and responses MUST contain a status. If they do not,t - // hey are considered to be malformed. - if frame.is_trailers() { - // TODO: Should this be a different error? - return Err(UnexpectedFrameType.into()); - } - - let state = try!(self.inner.send.open(id)); - e.insert(state) - } - }; - - if frame.is_trailers() { - try!(self.inner.send.send_eos(state)); - } else { - try!(self.inner.send.send_headers(state, frame.is_end_stream())); - } - - if state.is_closed() { - self.inner.dec_num_streams(id); - } - - Ok(()) - } - - pub fn send_data(&mut self, frame: &frame::Data) - -> Result<(), ConnectionError> - { - let id = frame.stream_id(); - - let state = match self.streams.get_mut(&id) { - Some(state) => state, - None => return Err(UnexpectedFrameType.into()), - }; - - // Ensure there's enough capacity on the connection before acting on the - // stream. - try!(self.inner.send.send_data(frame, state)); - - if state.is_closed() { - self.inner.dec_num_streams(id); - } - - Ok(()) - } - - pub fn poll_window_update(&mut self) - -> Poll - { - self.inner.send.poll_window_update(&mut self.streams) - } - - pub fn expand_window(&mut self, id: StreamId, sz: WindowSize) - -> Result<(), ConnectionError> - { - if id.is_zero() { - try!(self.inner.recv.expand_connection_window(sz)); - } else { - if let Some(state) = self.streams.get_mut(&id) { - try!(self.inner.recv.expand_stream_window(id, sz, state)); - } - } - - Ok(()) - } - - pub fn send_pending_refusal(&mut self, dst: &mut Codec) - -> Poll<(), ConnectionError> - where T: AsyncWrite, - B: Buf, - { - self.inner.recv.send_pending_refusal(dst) - } - - pub fn send_pending_window_updates(&mut self, dst: &mut Codec) - -> Poll<(), ConnectionError> - where T: AsyncWrite, - B: Buf, - { - try_ready!(self.inner.recv.send_connection_window_update(dst)); - try_ready!(self.inner.recv.send_stream_window_update(&mut self.streams, dst)); - - Ok(().into()) - } -} - -impl Inner

{ - fn dec_num_streams(&mut self, id: StreamId) { - if self.is_local_init(id) { - self.send.dec_num_streams(); - } else { - self.recv.dec_num_streams(); - } - } - - fn is_local_init(&self, id: StreamId) -> bool { - assert!(!id.is_zero()); - P::is_server() == id.is_server_initiated() - } -} diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs new file mode 100644 index 000000000..e69de29bb diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs new file mode 100644 index 000000000..11da1776b --- /dev/null +++ b/src/proto/streams/streams.rs @@ -0,0 +1,266 @@ +use proto::*; +use super::*; + +use std::sync::{Arc, Mutex}; + +// TODO: All the VecDeques should become linked lists using the State +// values. +#[derive(Debug)] +pub struct Streams

{ + inner: Arc>>, +} + +/// Fields needed to manage state related to managing the set of streams. This +/// is mostly split out to make ownership happy. +/// +/// TODO: better name +#[derive(Debug)] +struct Inner

{ + actions: Actions

, + store: Store, +} + +#[derive(Debug)] +struct Actions

{ + /// Manages state transitions initiated by receiving frames + recv: Recv

, + + /// Manages state transitions initiated by sending frames + send: Send

, +} + +impl Streams

{ + pub fn new(config: Config) -> Self { + Streams { + inner: Arc::new(Mutex::new(Inner { + actions: Actions { + recv: Recv::new(&config), + send: Send::new(&config), + }, + store: Store::new(), + })), + } + } + + pub fn recv_headers(&mut self, frame: frame::Headers) + -> Result, ConnectionError> + { + let id = frame.stream_id(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let state = match me.store.entry(id) { + Entry::Occupied(e) => e.into_mut(), + Entry::Vacant(e) => { + // Trailers cannot open a stream. Trailers are header frames + // that do not contain pseudo headers. Requests MUST contain a + // method and responses MUST contain a status. If they do not,t + // hey are considered to be malformed. + if frame.is_trailers() { + return Err(ProtocolError.into()); + } + + match try!(me.actions.recv.open(id)) { + Some(state) => e.insert(state), + None => return Ok(None), + } + } + }; + + if frame.is_trailers() { + if !frame.is_end_stream() { + // TODO: What error should this return? + unimplemented!(); + } + + try!(me.actions.recv.recv_eos(state)); + } else { + try!(me.actions.recv.recv_headers(state, frame.is_end_stream())); + } + + if state.is_closed() { + me.actions.dec_num_streams(id); + } + + Ok(Some(frame)) + } + + pub fn recv_data(&mut self, frame: &frame::Data) + -> Result<(), ConnectionError> + { + let id = frame.stream_id(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let state = match me.store.get_mut(&id) { + Some(state) => state, + None => return Err(ProtocolError.into()), + }; + + // Ensure there's enough capacity on the connection before acting on the + // stream. + try!(me.actions.recv.recv_data(frame, state)); + + if state.is_closed() { + me.actions.dec_num_streams(id); + } + + Ok(()) + } + + pub fn recv_reset(&mut self, _frame: &frame::Reset) + -> Result<(), ConnectionError> + { + unimplemented!(); + } + + pub fn recv_window_update(&mut self, frame: frame::WindowUpdate) + -> Result<(), ConnectionError> { + let id = frame.stream_id(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + if id.is_zero() { + try!(me.actions.send.recv_connection_window_update(frame)); + } else { + // The remote may send window updates for streams that the local now + // considers closed. It's ok... + if let Some(state) = me.store.get_mut(&id) { + try!(me.actions.send.recv_stream_window_update(frame, state)); + } + } + + Ok(()) + } + + pub fn recv_push_promise(&mut self, _frame: frame::PushPromise) + -> Result<(), ConnectionError> + { + unimplemented!(); + } + + pub fn send_headers(&mut self, frame: &frame::Headers) + -> Result<(), ConnectionError> + { + let id = frame.stream_id(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + trace!("send_headers; id={:?}", id); + + let state = match me.store.entry(id) { + Entry::Occupied(e) => e.into_mut(), + Entry::Vacant(e) => { + // Trailers cannot open a stream. Trailers are header frames + // that do not contain pseudo headers. Requests MUST contain a + // method and responses MUST contain a status. If they do not,t + // hey are considered to be malformed. + if frame.is_trailers() { + // TODO: Should this be a different error? + return Err(UnexpectedFrameType.into()); + } + + let state = try!(me.actions.send.open(id)); + e.insert(state) + } + }; + + if frame.is_trailers() { + try!(me.actions.send.send_eos(state)); + } else { + try!(me.actions.send.send_headers(state, frame.is_end_stream())); + } + + if state.is_closed() { + me.actions.dec_num_streams(id); + } + + Ok(()) + } + + pub fn send_data(&mut self, frame: &frame::Data) + -> Result<(), ConnectionError> + { + let id = frame.stream_id(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let state = match me.store.get_mut(&id) { + Some(state) => state, + None => return Err(UnexpectedFrameType.into()), + }; + + // Ensure there's enough capacity on the connection before acting on the + // stream. + try!(me.actions.send.send_data(frame, state)); + + if state.is_closed() { + me.actions.dec_num_streams(id); + } + + Ok(()) + } + + pub fn poll_window_update(&mut self) + -> Poll + { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + me.actions.send.poll_window_update(&mut me.store) + } + + pub fn expand_window(&mut self, id: StreamId, sz: WindowSize) + -> Result<(), ConnectionError> + { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + if id.is_zero() { + try!(me.actions.recv.expand_connection_window(sz)); + } else { + if let Some(state) = me.store.get_mut(&id) { + try!(me.actions.recv.expand_stream_window(id, sz, state)); + } + } + + Ok(()) + } + + pub fn send_pending_refusal(&mut self, dst: &mut Codec) + -> Poll<(), ConnectionError> + where T: AsyncWrite, + B: Buf, + { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + me.actions.recv.send_pending_refusal(dst) + } + + pub fn send_pending_window_updates(&mut self, dst: &mut Codec) + -> Poll<(), ConnectionError> + where T: AsyncWrite, + B: Buf, + { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + try_ready!(me.actions.recv.send_connection_window_update(dst)); + try_ready!(me.actions.recv.send_stream_window_update(&mut me.store, dst)); + + Ok(().into()) + } +} + +impl Actions

{ + fn dec_num_streams(&mut self, id: StreamId) { + if self.is_local_init(id) { + self.send.dec_num_streams(); + } else { + self.recv.dec_num_streams(); + } + } + + fn is_local_init(&self, id: StreamId) -> bool { + assert!(!id.is_zero()); + P::is_server() == id.is_server_initiated() + } +} From 9f9bf85168e7060a152abdee5c1166b370603cd5 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 2 Aug 2017 14:48:10 -0700 Subject: [PATCH 02/19] More restructuring --- src/client.rs | 92 ++++++++++++++++++++++++--------------- src/lib.rs | 23 +--------- src/proto/connection.rs | 45 +++++++++---------- src/proto/mod.rs | 26 +++++++++-- src/proto/streams/mod.rs | 2 +- src/proto/streams/recv.rs | 2 +- src/proto/streams/send.rs | 2 +- 7 files changed, 104 insertions(+), 88 deletions(-) diff --git a/src/client.rs b/src/client.rs index a50408781..4167be23b 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,4 +1,5 @@ -use {frame, proto, Peer, ConnectionError, StreamId}; +use {frame, ConnectionError, StreamId}; +use proto::{self, Connection}; use http; use futures::{Future, Poll, Sink, AsyncSink}; @@ -10,55 +11,63 @@ use std::fmt; /// In progress H2 connection binding pub struct Handshake { // TODO: unbox - inner: Box, Error = ConnectionError>>, + inner: Box, Error = ConnectionError>>, } -/// Marker type indicating a client peer #[derive(Debug)] -pub struct Client; +struct Peer; -pub type Connection = super::Connection; +/// Marker type indicating a client peer +pub struct Client { + connection: Connection, +} -pub fn handshake(io: T) -> Handshake +impl Client where T: AsyncRead + AsyncWrite + 'static, { - handshake2(io) + pub fn handshake(io: T) -> Handshake { + Client::handshake2(io) + } } -/// Bind an H2 client connection. -/// -/// Returns a future which resolves to the connection value once the H2 -/// handshake has been completed. -pub fn handshake2(io: T) -> Handshake +impl Client + // TODO: Get rid of 'static where T: AsyncRead + AsyncWrite + 'static, B: IntoBuf + 'static, { - use tokio_io::io; - - debug!("binding client connection"); - - let handshake = io::write_all(io, b"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n") - .map_err(ConnectionError::from) - .and_then(|(io, _)| { - debug!("client connection bound"); - - let mut framed_write = proto::framed_write(io); - let settings = frame::Settings::default(); - - // Send initial settings frame - match framed_write.start_send(settings.into()) { - Ok(AsyncSink::Ready) => { - Ok(proto::from_framed_write(framed_write)) + /// Bind an H2 client connection. + /// + /// Returns a future which resolves to the connection value once the H2 + /// handshake has been completed. + pub fn handshake2(io: T) -> Handshake { + use tokio_io::io; + + debug!("binding client connection"); + + let handshake = io::write_all(io, b"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n") + .map_err(ConnectionError::from) + .and_then(|(io, _)| { + debug!("client connection bound"); + + let mut framed_write = proto::framed_write(io); + let settings = frame::Settings::default(); + + // Send initial settings frame + match framed_write.start_send(settings.into()) { + Ok(AsyncSink::Ready) => { + let conn = proto::from_framed_write(framed_write); + Ok(Client { connection: conn }) + } + Ok(_) => unreachable!(), + Err(e) => Err(ConnectionError::from(e)), } - Ok(_) => unreachable!(), - Err(e) => Err(ConnectionError::from(e)), - } - }); + }); - Handshake { inner: Box::new(handshake) } + Handshake { inner: Box::new(handshake) } + } } -impl Peer for Client { +impl proto::Peer for Peer { type Send = http::request::Head; type Poll = http::response::Head; @@ -98,7 +107,7 @@ impl Peer for Client { } impl Future for Handshake { - type Item = Connection; + type Item = Client; type Error = ConnectionError; fn poll(&mut self) -> Poll { @@ -109,8 +118,21 @@ impl Future for Handshake { impl fmt::Debug for Handshake where T: fmt::Debug, B: fmt::Debug + IntoBuf, + B::Buf: fmt::Debug + IntoBuf, { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { write!(fmt, "client::Handshake") } } + +impl fmt::Debug for Client + where T: fmt::Debug, + B: fmt::Debug + IntoBuf, + B::Buf: fmt::Debug + IntoBuf, +{ + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.debug_struct("Client") + .field("connection", &self.connection) + .finish() + } +} diff --git a/src/lib.rs b/src/lib.rs index b4fdf4deb..1eee29ba8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,11 +30,10 @@ pub mod error; mod hpack; mod proto; mod frame; -pub mod server; +// pub mod server; pub use error::{ConnectionError, Reason}; pub use frame::StreamId; -pub use proto::Connection; use bytes::Bytes; @@ -68,23 +67,3 @@ pub enum Frame { error: Reason, }, } - -/// Either a Client or a Server -pub trait Peer { - /// Message type sent into the transport - type Send; - - /// Message type polled from the transport - type Poll; - - fn is_server() -> bool; - - #[doc(hidden)] - fn convert_send_message( - id: StreamId, - headers: Self::Send, - end_of_stream: bool) -> frame::Headers; - - #[doc(hidden)] - fn convert_poll_message(headers: frame::Headers) -> Self::Poll; -} diff --git a/src/proto/connection.rs b/src/proto/connection.rs index b9f70b2fd..234432b79 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -1,8 +1,6 @@ -use {ConnectionError, Frame, Peer}; +use {ConnectionError, Frame}; use HeaderMap; use frame::{self, StreamId}; -use client::Client; -use server::Server; use proto::*; @@ -26,34 +24,29 @@ pub struct Connection { _phantom: PhantomData

, } -pub fn new(codec: Codec) - -> Connection +impl Connection where T: AsyncRead + AsyncWrite, P: Peer, B: IntoBuf, { - // TODO: Actually configure - let streams = Streams::new(streams::Config { - max_remote_initiated: None, - init_remote_window_sz: DEFAULT_INITIAL_WINDOW_SIZE, - max_local_initiated: None, - init_local_window_sz: DEFAULT_INITIAL_WINDOW_SIZE, - }); - - Connection { - codec: codec, - ping_pong: PingPong::new(), - settings: Settings::new(), - streams: streams, - _phantom: PhantomData, + pub fn new(codec: Codec) -> Connection { + // TODO: Actually configure + let streams = Streams::new(streams::Config { + max_remote_initiated: None, + init_remote_window_sz: DEFAULT_INITIAL_WINDOW_SIZE, + max_local_initiated: None, + init_local_window_sz: DEFAULT_INITIAL_WINDOW_SIZE, + }); + + Connection { + codec: codec, + ping_pong: PingPong::new(), + settings: Settings::new(), + streams: streams, + _phantom: PhantomData, + } } -} -impl Connection - where T: AsyncRead + AsyncWrite, - P: Peer, - B: IntoBuf, -{ /// Polls for the next update to a remote flow control window. pub fn poll_window_update(&mut self) -> Poll { self.streams.poll_window_update() @@ -250,6 +243,7 @@ impl Connection } } +/* impl Connection where T: AsyncRead + AsyncWrite, B: IntoBuf, @@ -296,6 +290,7 @@ impl Connection }) } } +*/ impl Stream for Connection where T: AsyncRead + AsyncWrite, diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 30f2c6cfc..2df1eb558 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -13,15 +13,35 @@ use self::ping_pong::PingPong; use self::settings::Settings; use self::streams::Streams; -use {StreamId, Peer}; +use StreamId; use error::Reason; -use frame::Frame; +use frame::{self, Frame}; use futures::*; use bytes::{Buf, IntoBuf}; use tokio_io::{AsyncRead, AsyncWrite}; use tokio_io::codec::length_delimited; +/// Either a Client or a Server +pub trait Peer { + /// Message type sent into the transport + type Send; + + /// Message type polled from the transport + type Poll; + + fn is_server() -> bool; + + #[doc(hidden)] + fn convert_send_message( + id: StreamId, + headers: Self::Send, + end_of_stream: bool) -> frame::Headers; + + #[doc(hidden)] + fn convert_poll_message(headers: frame::Headers) -> Self::Poll; +} + pub type PingPayload = [u8; 8]; pub type WindowSize = u32; @@ -69,7 +89,7 @@ pub fn from_framed_write(framed_write: FramedWrite) let codec = FramedRead::new(framed); - connection::new(codec) + Connection::new(codec) } impl WindowUpdate { diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index 27bf15784..3cbc39b60 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -14,7 +14,7 @@ use self::send::Send; use self::state::State; use self::store::{Store, Entry}; -use {frame, Peer, StreamId, ConnectionError}; +use {frame, StreamId, ConnectionError}; use proto::*; use error::Reason::*; use error::User::*; diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 8dd60745a..ce12f0247 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -1,4 +1,4 @@ -use {frame, Peer, ConnectionError}; +use {frame, ConnectionError}; use proto::*; use super::*; diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 875fa0652..b36dc32ce 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -1,4 +1,4 @@ -use {frame, Peer, ConnectionError}; +use {frame, ConnectionError}; use proto::*; use super::*; From e810b309995415872639146e60dba81822a2c5a8 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 3 Aug 2017 10:00:50 -0700 Subject: [PATCH 03/19] Track http crate changes --- Cargo.toml | 2 +- src/client.rs | 24 ++++++++++++++---------- src/frame/headers.rs | 36 +++++++++++++++++------------------- src/hpack/decoder.rs | 16 ++++++++-------- src/proto/connection.rs | 14 +++++++------- src/proto/mod.rs | 4 ++-- 6 files changed, 49 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2d1199c5d..f21d93904 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ futures = "0.1" tokio-io = "0.1" tokio-timer = "0.1" bytes = "0.4" -http = { git = "https://github.com/carllerche/http" } +http = { git = "https://github.com/carllerche/http", branch = "uri-try-from-parts" } byteorder = "1.0" log = "0.3.8" fnv = "1.0.5" diff --git a/src/client.rs b/src/client.rs index 4167be23b..3b84cbbed 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,7 +1,8 @@ use {frame, ConnectionError, StreamId}; use proto::{self, Connection}; +use error::Reason::*; -use http; +use http::{self, Request, Response}; use futures::{Future, Poll, Sink, AsyncSink}; use tokio_io::{AsyncRead, AsyncWrite}; use bytes::{Bytes, IntoBuf}; @@ -65,11 +66,15 @@ impl Client Handshake { inner: Box::new(handshake) } } + + pub fn request(&mut self) { + unimplemented!(); + } } impl proto::Peer for Peer { - type Send = http::request::Head; - type Poll = http::response::Head; + type Send = Request<()>; + type Poll = Response<()>; fn is_server() -> bool { false @@ -77,15 +82,12 @@ impl proto::Peer for Peer { fn convert_send_message( id: StreamId, - headers: Self::Send, + request: Self::Send, end_of_stream: bool) -> frame::Headers { - use http::request::Head; - - // Extract the components of the HTTP request - let Head { method, uri, headers, .. } = headers; + use http::request::Parts; - // TODO: Ensure that the version is set to H2 + let (Parts { method, uri, headers, .. }, _) = request.into_parts(); // Build the set pseudo header set. All requests will include `method` // and `path`. @@ -101,8 +103,10 @@ impl proto::Peer for Peer { frame } - fn convert_poll_message(headers: frame::Headers) -> Self::Poll { + fn convert_poll_message(headers: frame::Headers) -> Result { headers.into_response() + // TODO: Is this always a protocol error? + .map_err(|_| ProtocolError.into()) } } diff --git a/src/frame/headers.rs b/src/frame/headers.rs index dbbf7996a..6c33a206d 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -3,7 +3,8 @@ use hpack; use frame::{self, Frame, Head, Kind, Error}; use HeaderMap; -use http::{request, response, version, uri, Method, StatusCode, Uri}; +use http::{self, request, response, version, uri, Method, StatusCode, Uri}; +use http::{Request, Response}; use http::header::{self, HeaderName, HeaderValue}; use bytes::{BytesMut, Bytes}; @@ -199,31 +200,28 @@ impl Headers { self.flags.set_end_stream() } - pub fn into_response(self) -> response::Head { - let mut response = response::Head::default(); + pub fn into_response(self) -> http::Result> { + let mut b = Response::builder(); if let Some(status) = self.pseudo.status { - response.status = status; - } else { - unimplemented!(); + b.status(status); } - response.headers = self.fields; - response + let mut response = try!(b.body(())); + *response.headers_mut() = self.fields; + + Ok(response) } - pub fn into_request(self) -> request::Head { - let mut request = request::Head::default(); + pub fn into_request(self) -> http::Result> { + let mut b = Request::builder(); // TODO: should we distinguish between HTTP_2 and HTTP_2C? // carllerche/http#42 - request.version = version::HTTP_2; + b.version(version::HTTP_2); if let Some(method) = self.pseudo.method { - request.method = method; - } else { - // TODO: invalid request - unimplemented!(); + b.method(method); } // Convert the URI @@ -244,12 +242,12 @@ impl Headers { parts.origin_form = Some(uri::OriginForm::try_from_shared(path.into_inner()).unwrap()); } - request.uri = parts.into(); + b.uri(parts); - // Set the header fields - request.headers = self.fields; + let mut request = try!(b.body(())); + *request.headers_mut() = self.fields; - request + Ok(request) } pub fn into_fields(self) -> HeaderMap { diff --git a/src/hpack/decoder.rs b/src/hpack/decoder.rs index d6cde3b13..70e3212b0 100644 --- a/src/hpack/decoder.rs +++ b/src/hpack/decoder.rs @@ -495,29 +495,29 @@ impl From for DecoderError { } } -impl From for DecoderError { - fn from(_: header::InvalidValueError) -> DecoderError { +impl From for DecoderError { + fn from(_: header::InvalidHeaderValue) -> DecoderError { // TODO: Better error? DecoderError::InvalidUtf8 } } -impl From for DecoderError { - fn from(_: method::FromBytesError) -> DecoderError { +impl From for DecoderError { + fn from(_: header::InvalidHeaderName) -> DecoderError { // TODO: Better error DecoderError::InvalidUtf8 } } -impl From for DecoderError { - fn from(_: header::FromBytesError) -> DecoderError { +impl From for DecoderError { + fn from(_: method::InvalidMethod) -> DecoderError { // TODO: Better error DecoderError::InvalidUtf8 } } -impl From for DecoderError { - fn from(_: status::FromStrError) -> DecoderError { +impl From for DecoderError { + fn from(_: status::InvalidStatusCode) -> DecoderError { // TODO: Better error DecoderError::InvalidUtf8 } diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 234432b79..2ee224006 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -174,7 +174,7 @@ impl Connection // Update stream state while ensuring that the headers frame // can be received. if let Some(frame) = try!(self.streams.recv_headers(frame)) { - let frame = Self::convert_poll_message(frame); + let frame = Self::convert_poll_message(frame)?; return Ok(Some(frame).into()); } } @@ -227,18 +227,18 @@ impl Connection } } - fn convert_poll_message(frame: frame::Headers) -> Frame { + fn convert_poll_message(frame: frame::Headers) -> Result, ConnectionError> { if frame.is_trailers() { - Frame::Trailers { + Ok(Frame::Trailers { id: frame.stream_id(), headers: frame.into_fields() - } + }) } else { - Frame::Headers { + Ok(Frame::Headers { id: frame.stream_id(), end_of_stream: frame.is_end_stream(), - headers: P::convert_poll_message(frame), - } + headers: P::convert_poll_message(frame)?, + }) } } } diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 2df1eb558..a032e91d1 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -13,7 +13,7 @@ use self::ping_pong::PingPong; use self::settings::Settings; use self::streams::Streams; -use StreamId; +use {StreamId, ConnectionError}; use error::Reason; use frame::{self, Frame}; @@ -39,7 +39,7 @@ pub trait Peer { end_of_stream: bool) -> frame::Headers; #[doc(hidden)] - fn convert_poll_message(headers: frame::Headers) -> Self::Poll; + fn convert_poll_message(headers: frame::Headers) -> Result; } pub type PingPayload = [u8; 8]; From 7a804601c531966c82ea862d0fbb2ebd73d9eecf Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 3 Aug 2017 11:40:50 -0700 Subject: [PATCH 04/19] A lot of structural work --- src/client.rs | 112 ++++++++++++++++++++++++----------- src/frame/stream_id.rs | 4 ++ src/lib.rs | 4 +- src/proto/connection.rs | 53 ++++++++--------- src/proto/framed_read.rs | 2 +- src/proto/mod.rs | 4 +- src/proto/streams/mod.rs | 4 +- src/proto/streams/send.rs | 25 +++++--- src/proto/streams/store.rs | 5 ++ src/proto/streams/streams.rs | 60 +++++++++++++++---- tests/client_request.rs | 5 ++ 11 files changed, 193 insertions(+), 85 deletions(-) diff --git a/src/client.rs b/src/client.rs index 3b84cbbed..b20003411 100644 --- a/src/client.rs +++ b/src/client.rs @@ -16,13 +16,19 @@ pub struct Handshake { } #[derive(Debug)] -struct Peer; +pub(crate) struct Peer; /// Marker type indicating a client peer pub struct Client { connection: Connection, } +/// Client half of an active HTTP/2.0 stream. +pub struct Stream { + inner: proto::Stream, + _p: ::std::marker::PhantomData, +} + impl Client where T: AsyncRead + AsyncWrite + 'static, { @@ -67,11 +73,82 @@ impl Client Handshake { inner: Box::new(handshake) } } - pub fn request(&mut self) { + /// Returns `Ready` when the connection can initialize a new HTTP 2.0 + /// stream. + pub fn poll_ready(&mut self) -> Poll<(), ConnectionError> { + unimplemented!(); + } + + /// Send a request on a new HTTP 2.0 stream + pub fn request(&mut self, request: Request<()>, end_of_stream: bool) + -> Result, ConnectionError> + { + self.connection.send_request(request, end_of_stream) + .map(|stream| Stream { + inner: stream, + _p: ::std::marker::PhantomData, + }) + } +} + +impl fmt::Debug for Client + where T: fmt::Debug, + B: fmt::Debug + IntoBuf, + B::Buf: fmt::Debug + IntoBuf, +{ + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.debug_struct("Client") + .field("connection", &self.connection) + .finish() + } +} + +// ===== impl Handshake ===== + +impl Future for Handshake { + type Item = Client; + type Error = ConnectionError; + + fn poll(&mut self) -> Poll { + self.inner.poll() + } +} + +impl fmt::Debug for Handshake + where T: fmt::Debug, + B: fmt::Debug + IntoBuf, + B::Buf: fmt::Debug + IntoBuf, +{ + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + write!(fmt, "client::Handshake") + } +} + +// ===== impl Stream ===== + +impl Stream { + /// Receive the HTTP/2.0 response, if it is ready. + pub fn poll_response(&mut self) -> Poll<(), ConnectionError> { + unimplemented!(); + } + + /// Send data + pub fn send_data(&mut self, data: B, end_of_stream: bool) + -> Result<(), ConnectionError> + { + unimplemented!(); + } + + /// Send trailers + pub fn send_trailers(&mut self, trailers: ()) + -> Result<(), ConnectionError> + { unimplemented!(); } } +// ===== impl Peer ===== + impl proto::Peer for Peer { type Send = Request<()>; type Poll = Response<()>; @@ -109,34 +186,3 @@ impl proto::Peer for Peer { .map_err(|_| ProtocolError.into()) } } - -impl Future for Handshake { - type Item = Client; - type Error = ConnectionError; - - fn poll(&mut self) -> Poll { - self.inner.poll() - } -} - -impl fmt::Debug for Handshake - where T: fmt::Debug, - B: fmt::Debug + IntoBuf, - B::Buf: fmt::Debug + IntoBuf, -{ - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - write!(fmt, "client::Handshake") - } -} - -impl fmt::Debug for Client - where T: fmt::Debug, - B: fmt::Debug + IntoBuf, - B::Buf: fmt::Debug + IntoBuf, -{ - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.debug_struct("Client") - .field("connection", &self.connection) - .finish() - } -} diff --git a/src/frame/stream_id.rs b/src/frame/stream_id.rs index 6150bef76..417a1e065 100644 --- a/src/frame/stream_id.rs +++ b/src/frame/stream_id.rs @@ -39,6 +39,10 @@ impl StreamId { pub fn is_zero(&self) -> bool { self.0 == 0 } + + pub fn increment(&mut self) { + self.0 += 2; + } } impl From for StreamId { diff --git a/src/lib.rs b/src/lib.rs index 1eee29ba8..693a6db2f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,5 @@ -// #![allow(warnings)] -#![deny(missing_debug_implementations)] +#![allow(warnings)] +// #![deny(missing_debug_implementations)] #[macro_use] extern crate futures; diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 2ee224006..24fe62f38 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -1,10 +1,10 @@ -use {ConnectionError, Frame}; +use {client, ConnectionError, Frame}; use HeaderMap; use frame::{self, StreamId}; use proto::*; -use http::{request, response}; +use http::{Request, Response}; use bytes::{Bytes, IntoBuf}; use tokio_io::{AsyncRead, AsyncWrite}; @@ -80,6 +80,7 @@ impl Connection unimplemented!(); } + /// Returns `Ready` when the connection is ready to receive a frame. pub fn poll_ready(&mut self) -> Poll<(), ConnectionError> { try_ready!(self.poll_send_ready()); @@ -89,6 +90,7 @@ impl Connection Ok(().into()) } + /* pub fn send_data(self, id: StreamId, data: B, @@ -112,10 +114,7 @@ impl Connection headers, }) } - - pub fn start_ping(&mut self, _body: PingPayload) -> StartSend { - unimplemented!(); - } + */ // ===== Private ===== @@ -227,6 +226,13 @@ impl Connection } } + fn poll_complete(&mut self) -> Poll<(), ConnectionError> { + try_ready!(self.poll_send_ready()); + try_ready!(self.codec.poll_complete()); + + Ok(().into()) + } + fn convert_poll_message(frame: frame::Headers) -> Result, ConnectionError> { if frame.is_trailers() { Ok(Frame::Trailers { @@ -243,6 +249,18 @@ impl Connection } } +impl Connection + where T: AsyncRead + AsyncWrite, + B: IntoBuf, +{ + /// Initialize a new HTTP/2.0 stream and send the message. + pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) + -> Result, ConnectionError> + { + self.streams.send_request(request, end_of_stream) + } +} + /* impl Connection where T: AsyncRead + AsyncWrite, @@ -292,20 +310,7 @@ impl Connection } */ -impl Stream for Connection - where T: AsyncRead + AsyncWrite, - P: Peer, - B: IntoBuf, -{ - type Item = Frame; - type Error = ConnectionError; - - fn poll(&mut self) -> Poll, ConnectionError> { - // TODO: intercept errors and flag the connection - self.recv_frame() - } -} - +/* impl Sink for Connection where T: AsyncRead + AsyncWrite, P: Peer, @@ -379,11 +384,5 @@ impl Sink for Connection // Return success Ok(AsyncSink::Ready) } - - fn poll_complete(&mut self) -> Poll<(), ConnectionError> { - try_ready!(self.poll_send_ready()); - try_ready!(self.codec.poll_complete()); - - Ok(().into()) - } } +*/ diff --git a/src/proto/framed_read.rs b/src/proto/framed_read.rs index ae2d059f4..3b3033319 100644 --- a/src/proto/framed_read.rs +++ b/src/proto/framed_read.rs @@ -117,7 +117,7 @@ impl ApplySettings for FramedRead { } */ -impl Stream for FramedRead +impl futures::Stream for FramedRead where T: AsyncRead, { type Item = Frame; diff --git a/src/proto/mod.rs b/src/proto/mod.rs index a032e91d1..71294022e 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -6,18 +6,18 @@ mod settings; mod streams; pub use self::connection::Connection; +pub use self::streams::{Streams, Stream}; use self::framed_read::FramedRead; use self::framed_write::FramedWrite; use self::ping_pong::PingPong; use self::settings::Settings; -use self::streams::Streams; use {StreamId, ConnectionError}; use error::Reason; use frame::{self, Frame}; -use futures::*; +use futures::{self, task, Poll, Async, AsyncSink, Sink, Stream as Stream2}; use bytes::{Buf, IntoBuf}; use tokio_io::{AsyncRead, AsyncWrite}; use tokio_io::codec::length_delimited; diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index 3cbc39b60..7165bd73d 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -6,7 +6,7 @@ mod store; mod stream; mod streams; -pub use self::streams::Streams; +pub use self::streams::{Streams, Stream}; use self::flow_control::FlowControl; use self::recv::Recv; @@ -19,6 +19,8 @@ use proto::*; use error::Reason::*; use error::User::*; +use http::{Request, Response}; + #[derive(Debug)] pub struct Config { /// Maximum number of remote initiated streams diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index b36dc32ce..a233b837e 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -17,6 +17,9 @@ pub struct Send

{ /// Current number of locally initiated streams num_streams: usize, + /// Stream identifier to use for next initialized stream. + next_stream_id: StreamId, + /// Initial window size of locally initiated streams init_window_sz: WindowSize, @@ -37,9 +40,16 @@ pub struct Send

{ impl Send

{ pub fn new(config: &Config) -> Self { + let next_stream_id = if P::is_server() { + 2 + } else { + 1 + }; + Send { max_streams: config.max_local_initiated, num_streams: 0, + next_stream_id: next_stream_id.into(), init_window_sz: config.init_local_window_sz, flow_control: FlowControl::new(config.init_local_window_sz), pending_window_updates: VecDeque::new(), @@ -51,8 +61,8 @@ impl Send

{ /// Update state reflecting a new, locally opened stream /// /// Returns the stream state if successful. `None` if refused - pub fn open(&mut self, id: StreamId) -> Result { - try!(self.ensure_can_open(id)); + pub fn open(&mut self) -> Result<(StreamId, State), ConnectionError> { + try!(self.ensure_can_open()); if let Some(max) = self.max_streams { if max <= self.num_streams { @@ -60,10 +70,13 @@ impl Send

{ } } + let ret = (self.next_stream_id, State::default()); + // Increment the number of locally initiated streams self.num_streams += 1; + self.next_stream_id.increment(); - Ok(State::default()) + Ok(ret) } pub fn send_headers(&mut self, state: &mut State, eos: bool) @@ -191,15 +204,13 @@ impl Send

{ } /// Returns true if the local actor can initiate a stream with the given ID. - fn ensure_can_open(&self, id: StreamId) -> Result<(), ConnectionError> { + fn ensure_can_open(&self) -> Result<(), ConnectionError> { if P::is_server() { // Servers cannot open streams. PushPromise must first be reserved. return Err(UnexpectedFrameType.into()); } - if !id.is_client_initiated() { - return Err(InvalidStreamId.into()); - } + // TODO: Handle StreamId overflow Ok(()) } diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index 39625299c..3e545309a 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -42,6 +42,11 @@ impl Store { } } + pub fn insert(&mut self, id: StreamId, val: State) { + let handle = self.slab.insert(val); + assert!(self.ids.insert(id, handle).is_none()); + } + pub fn entry(&mut self, id: StreamId) -> Entry { use self::hash_map::Entry::*; diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 11da1776b..2022a3c67 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -1,3 +1,4 @@ +use client; use proto::*; use super::*; @@ -10,6 +11,12 @@ pub struct Streams

{ inner: Arc>>, } +#[derive(Debug)] +pub struct Stream

{ + inner: Arc>>, + id: StreamId, +} + /// Fields needed to manage state related to managing the set of streams. This /// is mostly split out to make ownership happy. /// @@ -139,28 +146,22 @@ impl Streams

{ unimplemented!(); } - pub fn send_headers(&mut self, frame: &frame::Headers) + pub fn send_headers(&mut self, headers: frame::Headers) -> Result<(), ConnectionError> { + unimplemented!(); + /* let id = frame.stream_id(); let mut me = self.inner.lock().unwrap(); let me = &mut *me; - trace!("send_headers; id={:?}", id); + // let (id, state) = me.actions.send.open()); + let state = match me.store.entry(id) { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { - // Trailers cannot open a stream. Trailers are header frames - // that do not contain pseudo headers. Requests MUST contain a - // method and responses MUST contain a status. If they do not,t - // hey are considered to be malformed. - if frame.is_trailers() { - // TODO: Should this be a different error? - return Err(UnexpectedFrameType.into()); - } - - let state = try!(me.actions.send.open(id)); + let (id, state) = try!(me.actions.send.open()); e.insert(state) } }; @@ -176,6 +177,7 @@ impl Streams

{ } Ok(()) + */ } pub fn send_data(&mut self, frame: &frame::Data) @@ -250,6 +252,40 @@ impl Streams

{ } } +impl Streams { + pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) + -> Result, ConnectionError> + { + let id = { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + // Initialize a new stream. This fails if the connection is at capacity. + let (id, mut state) = me.actions.send.open()?; + + // Convert the message + let headers = client::Peer::convert_send_message( + id, request, end_of_stream); + + me.actions.send.send_headers(&mut state, end_of_stream)?; + + // Given that the stream has been initialized, it should not be in the + // closed state. + debug_assert!(!state.is_closed()); + + // Store the state + me.store.insert(id, state); + + id + }; + + Ok(Stream { + inner: self.inner.clone(), + id: id, + }) + } +} + impl Actions

{ fn dec_num_streams(&mut self, id: StreamId) { if self.is_local_init(id) { diff --git a/tests/client_request.rs b/tests/client_request.rs index 49250b2cb..6e897ebba 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -117,6 +117,11 @@ fn recv_invalid_server_stream_id() { assert_proto_err!(err, ProtocolError); } +#[test] +#[ignore] +fn sending_request_on_closed_soket() { +} + const SETTINGS: &'static [u8] = &[0, 0, 0, 4, 0, 0, 0, 0, 0]; const SETTINGS_ACK: &'static [u8] = &[0, 0, 0, 4, 1, 0, 0, 0, 0]; From dd8412d66096ac78b704e594752e546ef733d74b Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 3 Aug 2017 15:50:13 -0700 Subject: [PATCH 05/19] Much work --- src/client.rs | 4 +- src/lib.rs | 2 + src/proto/connection.rs | 4 +- src/proto/mod.rs | 2 +- src/proto/streams/buffer.rs | 60 ++++++++++++++++++++++++++ src/proto/streams/mod.rs | 5 ++- src/proto/streams/recv.rs | 46 ++++++++++---------- src/proto/streams/send.rs | 46 ++++++++++++-------- src/proto/streams/store.rs | 38 ++++++++--------- src/proto/streams/stream.rs | 27 ++++++++++++ src/proto/streams/streams.rs | 81 ++++++++++++++++++++---------------- 11 files changed, 211 insertions(+), 104 deletions(-) create mode 100644 src/proto/streams/buffer.rs diff --git a/src/client.rs b/src/client.rs index b20003411..94b043abf 100644 --- a/src/client.rs +++ b/src/client.rs @@ -25,8 +25,7 @@ pub struct Client { /// Client half of an active HTTP/2.0 stream. pub struct Stream { - inner: proto::Stream, - _p: ::std::marker::PhantomData, + inner: proto::StreamRef, } impl Client @@ -86,7 +85,6 @@ impl Client self.connection.send_request(request, end_of_stream) .map(|stream| Stream { inner: stream, - _p: ::std::marker::PhantomData, }) } } diff --git a/src/lib.rs b/src/lib.rs index 693a6db2f..08ac6264d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,8 @@ extern crate fnv; extern crate byteorder; +extern crate slab; + #[macro_use] extern crate log; diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 24fe62f38..3e3d7249d 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -19,7 +19,7 @@ pub struct Connection { // TODO: Remove ping_pong: PingPong, settings: Settings, - streams: Streams

, + streams: Streams, _phantom: PhantomData

, } @@ -255,7 +255,7 @@ impl Connection { /// Initialize a new HTTP/2.0 stream and send the message. pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) - -> Result, ConnectionError> + -> Result, ConnectionError> { self.streams.send_request(request, end_of_stream) } diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 71294022e..aaca6dbc4 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -6,7 +6,7 @@ mod settings; mod streams; pub use self::connection::Connection; -pub use self::streams::{Streams, Stream}; +pub use self::streams::{Streams, StreamRef}; use self::framed_read::FramedRead; use self::framed_write::FramedWrite; diff --git a/src/proto/streams/buffer.rs b/src/proto/streams/buffer.rs new file mode 100644 index 000000000..a8613c62f --- /dev/null +++ b/src/proto/streams/buffer.rs @@ -0,0 +1,60 @@ +use frame::{self, Frame}; + +use slab::Slab; + +use std::marker::PhantomData; + +/// Buffers frames for multiple streams. +#[derive(Debug)] +pub struct Buffer { + slab: Slab>, +} + +/// A sequence of frames in a `Buffer` +#[derive(Debug)] +pub struct Deque { + indices: Option, + _p: PhantomData, +} + +/// Tracks the head & tail for a sequence of frames in a `Buffer`. +#[derive(Debug, Default)] +struct Indices { + head: usize, + tail: usize, +} + +#[derive(Debug)] +struct Slot { + frame: Frame, + next: usize, +} + +impl Buffer { + pub fn new() -> Self { + Buffer { + slab: Slab::new(), + } + } +} + +impl Deque { + pub fn new() -> Self { + Deque { + indices: None, + _p: PhantomData, + } + } + + pub fn is_empty(&self) -> bool { + self.indices.is_none() + } + + pub fn push_back(&mut self, buf: &mut Buffer, val: Frame) { + unimplemented!(); + } + + pub fn pop_front(&mut self, buf: &mut Buffer) -> Option> { + unimplemented!(); + } +} diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index 7165bd73d..a36602fb7 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -1,3 +1,4 @@ +mod buffer; mod flow_control; mod recv; mod send; @@ -6,13 +7,15 @@ mod store; mod stream; mod streams; -pub use self::streams::{Streams, Stream}; +pub use self::streams::{Streams, StreamRef}; +use self::buffer::Buffer; use self::flow_control::FlowControl; use self::recv::Recv; use self::send::Send; use self::state::State; use self::store::{Store, Entry}; +use self::stream::Stream; use {frame, StreamId, ConnectionError}; use proto::*; diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index ce12f0247..c222afe9d 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -8,7 +8,7 @@ use std::collections::VecDeque; use std::marker::PhantomData; #[derive(Debug)] -pub struct Recv

{ +pub(super) struct Recv { /// Maximum number of remote initiated streams max_streams: Option, @@ -26,10 +26,13 @@ pub struct Recv

{ /// Refused StreamId, this represents a frame that must be sent out. refused: Option, - _p: PhantomData

, + _p: PhantomData<(P, B)>, } -impl Recv

{ +impl Recv + where P: Peer, + B: Buf, +{ pub fn new(config: &Config) -> Self { Recv { max_streams: config.max_remote_initiated, @@ -45,7 +48,7 @@ impl Recv

{ /// Update state reflecting a new, remotely opened stream /// /// Returns the stream state if successful. `None` if refused - pub fn open(&mut self, id: StreamId) -> Result, ConnectionError> { + pub fn open(&mut self, id: StreamId) -> Result>, ConnectionError> { assert!(self.refused.is_none()); try!(self.ensure_can_open(id)); @@ -60,25 +63,25 @@ impl Recv

{ // Increment the number of remote initiated streams self.num_streams += 1; - Ok(Some(State::default())) + Ok(Some(Stream::new())) } /// Transition the stream state based on receiving headers - pub fn recv_headers(&mut self, state: &mut State, eos: bool) + pub fn recv_headers(&mut self, stream: &mut Stream, eos: bool) -> Result<(), ConnectionError> { - state.recv_open(self.init_window_sz, eos) + stream.state.recv_open(self.init_window_sz, eos) } - pub fn recv_eos(&mut self, state: &mut State) + pub fn recv_eos(&mut self, stream: &mut Stream) -> Result<(), ConnectionError> { - state.recv_close() + stream.state.recv_close() } pub fn recv_data(&mut self, frame: &frame::Data, - state: &mut State) + stream: &mut Stream) -> Result<(), ConnectionError> { let sz = frame.payload().len(); @@ -89,7 +92,7 @@ impl Recv

{ let sz = sz as WindowSize; - match state.recv_flow_control() { + match stream.recv_flow_control() { Some(flow) => { // Ensure there's enough capacity on the connection before // acting on the stream. @@ -106,7 +109,7 @@ impl Recv

{ } if frame.is_end_stream() { - try!(state.recv_close()); + try!(stream.state.recv_close()); } Ok(()) @@ -133,10 +136,9 @@ impl Recv

{ } /// Send any pending refusals. - pub fn send_pending_refusal(&mut self, dst: &mut Codec) + pub fn send_pending_refusal(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, - B: Buf, { if let Some(stream_id) = self.refused.take() { let frame = frame::Reset::new(stream_id, RefusedStream); @@ -168,11 +170,11 @@ impl Recv

{ pub fn expand_stream_window(&mut self, id: StreamId, sz: WindowSize, - state: &mut State) + stream: &mut Stream) -> Result<(), ConnectionError> { // TODO: handle overflow - if let Some(flow) = state.recv_flow_control() { + if let Some(flow) = stream.recv_flow_control() { flow.expand_window(sz); self.pending_window_updates.push_back(id); } @@ -181,10 +183,9 @@ impl Recv

{ } /// Send connection level window update - pub fn send_connection_window_update(&mut self, dst: &mut Codec) + pub fn send_connection_window_update(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, - B: Buf, { if let Some(incr) = self.flow_control.peek_window_update() { let frame = frame::WindowUpdate::new(StreamId::zero(), incr); @@ -200,16 +201,15 @@ impl Recv

{ } /// Send stream level window update - pub fn send_stream_window_update(&mut self, - streams: &mut Store, - dst: &mut Codec) + pub fn send_stream_window_update(&mut self, + streams: &mut Store, + dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, - B: Buf, { while let Some(id) = self.pending_window_updates.pop_front() { let flow = streams.get_mut(&id) - .and_then(|state| state.recv_flow_control()); + .and_then(|stream| stream.recv_flow_control()); if let Some(flow) = flow { diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index a233b837e..bbc363c73 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::marker::PhantomData; #[derive(Debug)] -pub struct Send

{ +pub(super) struct Send { /// Maximum number of locally initiated streams max_streams: Option, @@ -30,6 +30,9 @@ pub struct Send

{ // XXX It would be cool if this didn't exist. pending_window_updates: VecDeque, + /// Holds frames that are waiting to be written to the socket + buffer: Buffer, + /// When `poll_window_update` is not ready, then the calling task is saved to /// be notified later. Access to poll_window_update must not be shared across tasks, /// as we only track a single task (and *not* i.e. a task per stream id). @@ -38,7 +41,10 @@ pub struct Send

{ _p: PhantomData

, } -impl Send

{ +impl Send + where P: Peer, + B: Buf, +{ pub fn new(config: &Config) -> Self { let next_stream_id = if P::is_server() { 2 @@ -53,6 +59,7 @@ impl Send

{ init_window_sz: config.init_local_window_sz, flow_control: FlowControl::new(config.init_local_window_sz), pending_window_updates: VecDeque::new(), + buffer: Buffer::new(), blocked: None, _p: PhantomData, } @@ -61,7 +68,7 @@ impl Send

{ /// Update state reflecting a new, locally opened stream /// /// Returns the stream state if successful. `None` if refused - pub fn open(&mut self) -> Result<(StreamId, State), ConnectionError> { + pub fn open(&mut self) -> Result<(StreamId, Stream), ConnectionError> { try!(self.ensure_can_open()); if let Some(max) = self.max_streams { @@ -70,7 +77,7 @@ impl Send

{ } } - let ret = (self.next_stream_id, State::default()); + let ret = (self.next_stream_id, Stream::new()); // Increment the number of locally initiated streams self.num_streams += 1; @@ -79,21 +86,24 @@ impl Send

{ Ok(ret) } - pub fn send_headers(&mut self, state: &mut State, eos: bool) + pub fn send_headers(&mut self, stream: &mut Stream, frame: frame::Headers) -> Result<(), ConnectionError> { - state.send_open(self.init_window_sz, eos) + // Update the state + stream.state.send_open(self.init_window_sz, frame.is_end_stream())?; + // stream.send_buf.headers = Some(frame); + Ok(()) } - pub fn send_eos(&mut self, state: &mut State) + pub fn send_eos(&mut self, stream: &mut Stream) -> Result<(), ConnectionError> { - state.send_close() + stream.state.send_close() } - pub fn send_data(&mut self, - frame: &frame::Data, - state: &mut State) + pub fn send_data(&mut self, + frame: &frame::Data, + stream: &mut Stream) -> Result<(), ConnectionError> { let sz = frame.payload().remaining(); @@ -107,7 +117,7 @@ impl Send

{ // Make borrow checker happy loop { - match state.send_flow_control() { + match stream.send_flow_control() { Some(flow) => { try!(self.flow_control.ensure_window(sz, FlowControlViolation)); @@ -123,7 +133,7 @@ impl Send

{ None => {} } - if state.is_closed() { + if stream.state.is_closed() { return Err(InactiveStreamId.into()) } else { return Err(UnexpectedFrameType.into()) @@ -131,14 +141,14 @@ impl Send

{ } if frame.is_end_stream() { - try!(state.send_close()); + try!(stream.state.send_close()); } Ok(()) } /// Get pending window updates - pub fn poll_window_update(&mut self, streams: &mut Store) + pub fn poll_window_update(&mut self, streams: &mut Store) -> Poll { // This biases connection window updates, which probably makes sense. @@ -152,7 +162,7 @@ impl Send

{ let update = self.pending_window_updates.pop_front() .and_then(|id| { streams.get_mut(&id) - .and_then(|state| state.send_flow_control()) + .and_then(|stream| stream.send_flow_control()) .and_then(|flow| flow.apply_window_update()) .map(|incr| WindowUpdate::new(id, incr)) }); @@ -184,10 +194,10 @@ impl Send

{ pub fn recv_stream_window_update(&mut self, frame: frame::WindowUpdate, - state: &mut State) + stream: &mut Stream) -> Result<(), ConnectionError> { - if let Some(flow) = state.send_flow_control() { + if let Some(flow) = stream.send_flow_control() { // TODO: Handle invalid increment flow.expand_window(frame.size_increment()); } diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index 3e545309a..4c2dbf7ff 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -1,32 +1,32 @@ -extern crate slab; - use super::*; +use slab; + use std::collections::{HashMap, hash_map}; /// Storage for streams #[derive(Debug)] -pub struct Store { - slab: slab::Slab, +pub(super) struct Store { + slab: slab::Slab>, ids: HashMap, } -pub enum Entry<'a> { - Occupied(OccupiedEntry<'a>), - Vacant(VacantEntry<'a>), +pub(super) enum Entry<'a, B: 'a> { + Occupied(OccupiedEntry<'a, B>), + Vacant(VacantEntry<'a, B>), } -pub struct OccupiedEntry<'a> { +pub(super) struct OccupiedEntry<'a, B: 'a> { ids: hash_map::OccupiedEntry<'a, StreamId, usize>, - slab: &'a mut slab::Slab, + slab: &'a mut slab::Slab>, } -pub struct VacantEntry<'a> { +pub(super) struct VacantEntry<'a, B: 'a> { ids: hash_map::VacantEntry<'a, StreamId, usize>, - slab: &'a mut slab::Slab, + slab: &'a mut slab::Slab>, } -impl Store { +impl Store { pub fn new() -> Self { Store { slab: slab::Slab::new(), @@ -34,7 +34,7 @@ impl Store { } } - pub fn get_mut(&mut self, id: &StreamId) -> Option<&mut State> { + pub fn get_mut(&mut self, id: &StreamId) -> Option<&mut Stream> { if let Some(handle) = self.ids.get(id) { Some(&mut self.slab[*handle]) } else { @@ -42,12 +42,12 @@ impl Store { } } - pub fn insert(&mut self, id: StreamId, val: State) { + pub fn insert(&mut self, id: StreamId, val: Stream) { let handle = self.slab.insert(val); assert!(self.ids.insert(id, handle).is_none()); } - pub fn entry(&mut self, id: StreamId) -> Entry { + pub fn entry(&mut self, id: StreamId) -> Entry { use self::hash_map::Entry::*; match self.ids.entry(id) { @@ -67,14 +67,14 @@ impl Store { } } -impl<'a> OccupiedEntry<'a> { - pub fn into_mut(self) -> &'a mut State { +impl<'a, B> OccupiedEntry<'a, B> { + pub fn into_mut(self) -> &'a mut Stream { &mut self.slab[*self.ids.get()] } } -impl<'a> VacantEntry<'a> { - pub fn insert(self, value: State) -> &'a mut State { +impl<'a, B> VacantEntry<'a, B> { + pub fn insert(self, value: Stream) -> &'a mut Stream { // Insert the value in the slab let handle = self.slab.insert(value); diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs index e69de29bb..2b3a67be1 100644 --- a/src/proto/streams/stream.rs +++ b/src/proto/streams/stream.rs @@ -0,0 +1,27 @@ +use super::*; + +#[derive(Debug)] +pub(super) struct Stream { + /// Current state of the stream + pub state: State, + + /// Frames pending for this stream being sent to the socket + pub pending_send: buffer::Deque, +} + +impl Stream { + pub fn new() -> Stream { + Stream { + state: State::default(), + pending_send: buffer::Deque::new(), + } + } + + pub fn send_flow_control(&mut self) -> Option<&mut FlowControl> { + self.state.send_flow_control() + } + + pub fn recv_flow_control(&mut self) -> Option<&mut FlowControl> { + self.state.recv_flow_control() + } +} diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 2022a3c67..d385956dc 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -7,13 +7,14 @@ use std::sync::{Arc, Mutex}; // TODO: All the VecDeques should become linked lists using the State // values. #[derive(Debug)] -pub struct Streams

{ - inner: Arc>>, +pub struct Streams { + inner: Arc>>, } +/// Reference to the stream state #[derive(Debug)] -pub struct Stream

{ - inner: Arc>>, +pub struct StreamRef { + inner: Arc>>, id: StreamId, } @@ -22,21 +23,24 @@ pub struct Stream

{ /// /// TODO: better name #[derive(Debug)] -struct Inner

{ - actions: Actions

, - store: Store, +struct Inner { + actions: Actions, + store: Store, } #[derive(Debug)] -struct Actions

{ +struct Actions { /// Manages state transitions initiated by receiving frames - recv: Recv

, + recv: Recv, /// Manages state transitions initiated by sending frames - send: Send

, + send: Send, } -impl Streams

{ +impl Streams + where P: Peer, + B: Buf, +{ pub fn new(config: Config) -> Self { Streams { inner: Arc::new(Mutex::new(Inner { @@ -56,7 +60,7 @@ impl Streams

{ let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let state = match me.store.entry(id) { + let stream = match me.store.entry(id) { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { // Trailers cannot open a stream. Trailers are header frames @@ -68,7 +72,7 @@ impl Streams

{ } match try!(me.actions.recv.open(id)) { - Some(state) => e.insert(state), + Some(stream) => e.insert(stream), None => return Ok(None), } } @@ -80,12 +84,12 @@ impl Streams

{ unimplemented!(); } - try!(me.actions.recv.recv_eos(state)); + try!(me.actions.recv.recv_eos(stream)); } else { - try!(me.actions.recv.recv_headers(state, frame.is_end_stream())); + try!(me.actions.recv.recv_headers(stream, frame.is_end_stream())); } - if state.is_closed() { + if stream.state.is_closed() { me.actions.dec_num_streams(id); } @@ -99,16 +103,16 @@ impl Streams

{ let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let state = match me.store.get_mut(&id) { - Some(state) => state, + let stream = match me.store.get_mut(&id) { + Some(stream) => stream, None => return Err(ProtocolError.into()), }; // Ensure there's enough capacity on the connection before acting on the // stream. - try!(me.actions.recv.recv_data(frame, state)); + try!(me.actions.recv.recv_data(frame, stream)); - if state.is_closed() { + if stream.state.is_closed() { me.actions.dec_num_streams(id); } @@ -180,23 +184,23 @@ impl Streams

{ */ } - pub fn send_data(&mut self, frame: &frame::Data) + pub fn send_data(&mut self, frame: &frame::Data) -> Result<(), ConnectionError> { let id = frame.stream_id(); let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let state = match me.store.get_mut(&id) { - Some(state) => state, + let stream = match me.store.get_mut(&id) { + Some(stream) => stream, None => return Err(UnexpectedFrameType.into()), }; // Ensure there's enough capacity on the connection before acting on the // stream. - try!(me.actions.send.send_data(frame, state)); + try!(me.actions.send.send_data(frame, stream)); - if state.is_closed() { + if stream.state.is_closed() { me.actions.dec_num_streams(id); } @@ -228,20 +232,18 @@ impl Streams

{ Ok(()) } - pub fn send_pending_refusal(&mut self, dst: &mut Codec) + pub fn send_pending_refusal(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, - B: Buf, { let mut me = self.inner.lock().unwrap(); let me = &mut *me; me.actions.recv.send_pending_refusal(dst) } - pub fn send_pending_window_updates(&mut self, dst: &mut Codec) + pub fn send_pending_window_updates(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, - B: Buf, { let mut me = self.inner.lock().unwrap(); let me = &mut *me; @@ -252,41 +254,46 @@ impl Streams

{ } } -impl Streams { +impl Streams + where B: Buf, +{ pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) - -> Result, ConnectionError> + -> Result, ConnectionError> { let id = { let mut me = self.inner.lock().unwrap(); let me = &mut *me; // Initialize a new stream. This fails if the connection is at capacity. - let (id, mut state) = me.actions.send.open()?; + let (id, mut stream) = me.actions.send.open()?; // Convert the message let headers = client::Peer::convert_send_message( id, request, end_of_stream); - me.actions.send.send_headers(&mut state, end_of_stream)?; + me.actions.send.send_headers(&mut stream, headers)?; // Given that the stream has been initialized, it should not be in the // closed state. - debug_assert!(!state.is_closed()); + debug_assert!(!stream.state.is_closed()); // Store the state - me.store.insert(id, state); + me.store.insert(id, stream); id }; - Ok(Stream { + Ok(StreamRef { inner: self.inner.clone(), id: id, }) } } -impl Actions

{ +impl Actions + where P: Peer, + B: Buf, +{ fn dec_num_streams(&mut self, id: StreamId) { if self.is_local_init(id) { self.send.dec_num_streams(); From 74b3852a58434b6de270ce0416db5ff0f6010249 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 3 Aug 2017 22:44:19 -0700 Subject: [PATCH 06/19] Start working on prioritization --- src/frame/headers.rs | 4 +- src/proto/streams/mod.rs | 2 + src/proto/streams/prioritize.rs | 61 +++++++++++++++++++++++++++++ src/proto/streams/recv.rs | 2 +- src/proto/streams/send.rs | 16 +++++--- src/proto/streams/store.rs | 69 ++++++++++++++++++++++++++++++--- src/proto/streams/stream.rs | 8 ++++ src/proto/streams/streams.rs | 17 ++++---- 8 files changed, 156 insertions(+), 23 deletions(-) create mode 100644 src/proto/streams/prioritize.rs diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 6c33a206d..81c0dee92 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -296,8 +296,8 @@ impl Headers { } } -impl From for Frame { - fn from(src: Headers) -> Frame { +impl From for Frame { + fn from(src: Headers) -> Self { Frame::Headers(src) } } diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index a36602fb7..2b5429e81 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -1,5 +1,6 @@ mod buffer; mod flow_control; +mod prioritize; mod recv; mod send; mod state; @@ -11,6 +12,7 @@ pub use self::streams::{Streams, StreamRef}; use self::buffer::Buffer; use self::flow_control::FlowControl; +use self::prioritize::Prioritize; use self::recv::Recv; use self::send::Send; use self::state::State; diff --git a/src/proto/streams/prioritize.rs b/src/proto/streams/prioritize.rs new file mode 100644 index 000000000..0e0dbfdbf --- /dev/null +++ b/src/proto/streams/prioritize.rs @@ -0,0 +1,61 @@ +use super::*; + +#[derive(Debug)] +pub(super) struct Prioritize { + pending_send: Option, + + /// Holds frames that are waiting to be written to the socket + buffer: Buffer, +} + +#[derive(Debug, Clone, Copy)] +struct Indices { + head: store::Key, + tail: store::Key, +} + +impl Prioritize { + pub fn new() -> Prioritize { + Prioritize { + pending_send: None, + buffer: Buffer::new(), + } + } + + pub fn queue_frame(&mut self, + frame: Frame, + stream: &mut store::Ptr) + { + // queue the frame in the buffer + stream.pending_send.push_back(&mut self.buffer, frame); + + if stream.is_pending_send { + debug_assert!(self.pending_send.is_some()); + + // Already queued to have frame processed. + return; + } + + // The next pointer shouldn't be set + debug_assert!(stream.next_pending_send.is_none()); + + // Queue the stream + match self.pending_send { + Some(ref mut idxs) => { + // Update the current tail node to point to `stream` + stream.resolve(idxs.tail).next_pending_send = Some(stream.key()); + + // Update the tail pointer + idxs.tail = stream.key(); + } + None => { + self.pending_send = Some(Indices { + head: stream.key(), + tail: stream.key(), + }); + } + } + + stream.is_pending_send = true; + } +} diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index c222afe9d..99a7a397c 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -208,7 +208,7 @@ impl Recv where T: AsyncWrite, { while let Some(id) = self.pending_window_updates.pop_front() { - let flow = streams.get_mut(&id) + let flow = streams.find_mut(&id) .and_then(|stream| stream.recv_flow_control()); diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index bbc363c73..6ed4fdd1d 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -30,8 +30,7 @@ pub(super) struct Send { // XXX It would be cool if this didn't exist. pending_window_updates: VecDeque, - /// Holds frames that are waiting to be written to the socket - buffer: Buffer, + prioritize: Prioritize, /// When `poll_window_update` is not ready, then the calling task is saved to /// be notified later. Access to poll_window_update must not be shared across tasks, @@ -58,8 +57,8 @@ impl Send next_stream_id: next_stream_id.into(), init_window_sz: config.init_local_window_sz, flow_control: FlowControl::new(config.init_local_window_sz), + prioritize: Prioritize::new(), pending_window_updates: VecDeque::new(), - buffer: Buffer::new(), blocked: None, _p: PhantomData, } @@ -86,12 +85,17 @@ impl Send Ok(ret) } - pub fn send_headers(&mut self, stream: &mut Stream, frame: frame::Headers) + pub fn send_headers(&mut self, + frame: frame::Headers, + stream: &mut store::Ptr) -> Result<(), ConnectionError> { // Update the state stream.state.send_open(self.init_window_sz, frame.is_end_stream())?; - // stream.send_buf.headers = Some(frame); + + // Queue the frame for sending + self.prioritize.queue_frame(frame.into(), stream); + Ok(()) } @@ -161,7 +165,7 @@ impl Send // TODO this should probably account for stream priority? let update = self.pending_window_updates.pop_front() .and_then(|id| { - streams.get_mut(&id) + streams.find_mut(&id) .and_then(|stream| stream.send_flow_control()) .and_then(|flow| flow.apply_window_update()) .map(|incr| WindowUpdate::new(id, incr)) diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index 4c2dbf7ff..b19c755c0 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -2,6 +2,7 @@ use super::*; use slab; +use std::ops; use std::collections::{HashMap, hash_map}; /// Storage for streams @@ -11,6 +12,16 @@ pub(super) struct Store { ids: HashMap, } +/// "Pointer" to an entry in the store +pub(super) struct Ptr<'a, B: 'a> { + key: Key, + store: &'a mut Store, +} + +/// References an entry in the store. +#[derive(Debug, Clone, Copy)] +pub(super) struct Key(usize); + pub(super) enum Entry<'a, B: 'a> { Occupied(OccupiedEntry<'a, B>), Vacant(VacantEntry<'a, B>), @@ -26,6 +37,8 @@ pub(super) struct VacantEntry<'a, B: 'a> { slab: &'a mut slab::Slab>, } +// ===== impl Store ===== + impl Store { pub fn new() -> Self { Store { @@ -34,7 +47,7 @@ impl Store { } } - pub fn get_mut(&mut self, id: &StreamId) -> Option<&mut Stream> { + pub fn find_mut(&mut self, id: &StreamId) -> Option<&mut Stream> { if let Some(handle) = self.ids.get(id) { Some(&mut self.slab[*handle]) } else { @@ -42,12 +55,17 @@ impl Store { } } - pub fn insert(&mut self, id: StreamId, val: Stream) { - let handle = self.slab.insert(val); - assert!(self.ids.insert(id, handle).is_none()); + pub fn insert(&mut self, id: StreamId, val: Stream) -> Ptr { + let key = self.slab.insert(val); + assert!(self.ids.insert(id, key).is_none()); + + Ptr { + key: Key(key), + store: self, + } } - pub fn entry(&mut self, id: StreamId) -> Entry { + pub fn find_entry(&mut self, id: StreamId) -> Entry { use self::hash_map::Entry::*; match self.ids.entry(id) { @@ -67,12 +85,53 @@ impl Store { } } +// ===== impl Ptr ===== + +impl<'a, B: 'a> Ptr<'a, B> { + pub fn key(&self) -> Key { + self.key + } + + pub fn resolve(&mut self, key: Key) -> Ptr { + Ptr { + key: key, + store: self.store, + } + } +} + +impl<'a, B: 'a> ops::Deref for Ptr<'a, B> { + type Target = Stream; + + fn deref(&self) -> &Stream { + &self.store.slab[self.key.0] + } +} + +impl<'a, B: 'a> ops::DerefMut for Ptr<'a, B> { + fn deref_mut(&mut self) -> &mut Stream { + &mut self.store.slab[self.key.0] + } +} + +// ===== impl OccupiedEntry ===== + impl<'a, B> OccupiedEntry<'a, B> { + pub fn get(&self) -> &Stream { + &self.slab[*self.ids.get()] + } + + pub fn get_mut(&mut self) -> &mut Stream { + &mut self.slab[*self.ids.get()] + } + pub fn into_mut(self) -> &'a mut Stream { &mut self.slab[*self.ids.get()] } } +// ===== impl VacantEntry ===== +// impl<'a, B> VacantEntry<'a, B> { pub fn insert(self, value: Stream) -> &'a mut Stream { // Insert the value in the slab diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs index 2b3a67be1..0689d3cdb 100644 --- a/src/proto/streams/stream.rs +++ b/src/proto/streams/stream.rs @@ -7,6 +7,12 @@ pub(super) struct Stream { /// Frames pending for this stream being sent to the socket pub pending_send: buffer::Deque, + + /// Next stream pending send + pub next_pending_send: Option, + + /// True if the stream is currently pending send + pub is_pending_send: bool, } impl Stream { @@ -14,6 +20,8 @@ impl Stream { Stream { state: State::default(), pending_send: buffer::Deque::new(), + next_pending_send: None, + is_pending_send: false, } } diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index d385956dc..b5b3ded80 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -60,7 +60,7 @@ impl Streams let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let stream = match me.store.entry(id) { + let stream = match me.store.find_entry(id) { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { // Trailers cannot open a stream. Trailers are header frames @@ -103,7 +103,7 @@ impl Streams let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let stream = match me.store.get_mut(&id) { + let stream = match me.store.find_mut(&id) { Some(stream) => stream, None => return Err(ProtocolError.into()), }; @@ -136,7 +136,7 @@ impl Streams } else { // The remote may send window updates for streams that the local now // considers closed. It's ok... - if let Some(state) = me.store.get_mut(&id) { + if let Some(state) = me.store.find_mut(&id) { try!(me.actions.send.recv_stream_window_update(frame, state)); } } @@ -191,7 +191,7 @@ impl Streams let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let stream = match me.store.get_mut(&id) { + let stream = match me.store.find_mut(&id) { Some(stream) => stream, None => return Err(UnexpectedFrameType.into()), }; @@ -224,7 +224,7 @@ impl Streams if id.is_zero() { try!(me.actions.recv.expand_connection_window(sz)); } else { - if let Some(state) = me.store.get_mut(&id) { + if let Some(state) = me.store.find_mut(&id) { try!(me.actions.recv.expand_stream_window(id, sz, state)); } } @@ -271,15 +271,14 @@ impl Streams let headers = client::Peer::convert_send_message( id, request, end_of_stream); - me.actions.send.send_headers(&mut stream, headers)?; + let mut stream = me.store.insert(id, stream); + + me.actions.send.send_headers(headers, &mut stream)?; // Given that the stream has been initialized, it should not be in the // closed state. debug_assert!(!stream.state.is_closed()); - // Store the state - me.store.insert(id, stream); - id }; From fc0a7eb898c42836e2caa057e09898e0c4365ece Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 4 Aug 2017 12:12:22 -0700 Subject: [PATCH 07/19] More work --- src/client.rs | 4 +- src/proto/connection.rs | 149 +++++++++++++++++++------------- src/proto/streams/mod.rs | 1 + src/proto/streams/prioritize.rs | 34 +++++++- src/proto/streams/recv.rs | 48 +++++++++- src/proto/streams/send.rs | 9 ++ src/proto/streams/store.rs | 19 +++- src/proto/streams/stream.rs | 14 +++ src/proto/streams/streams.rs | 48 +++++++--- 9 files changed, 242 insertions(+), 84 deletions(-) diff --git a/src/client.rs b/src/client.rs index 94b043abf..dae10406a 100644 --- a/src/client.rs +++ b/src/client.rs @@ -126,8 +126,8 @@ impl fmt::Debug for Handshake impl Stream { /// Receive the HTTP/2.0 response, if it is ready. - pub fn poll_response(&mut self) -> Poll<(), ConnectionError> { - unimplemented!(); + pub fn poll_response(&mut self) -> Poll, ConnectionError> { + self.inner.poll_response() } /// Send data diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 3e3d7249d..b363c18c4 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -90,64 +90,8 @@ impl Connection Ok(().into()) } - /* - pub fn send_data(self, - id: StreamId, - data: B, - end_of_stream: bool) - -> sink::Send - { - self.send(Frame::Data { - id, - data, - end_of_stream, - }) - } - - pub fn send_trailers(self, - id: StreamId, - headers: HeaderMap) - -> sink::Send - { - self.send(Frame::Trailers { - id, - headers, - }) - } - */ - - // ===== Private ===== - - /// Returns `Ready` when the `Connection` is ready to receive a frame from - /// the socket. - fn poll_recv_ready(&mut self) -> Poll<(), ConnectionError> { - // Pong, settings ack, and stream refusals are high priority frames to - // send. If the write buffer is full, we stop reading any further frames - // until these high priority writes can be committed to the buffer. - - try_ready!(self.ping_pong.send_pending_pong(&mut self.codec)); - try_ready!(self.settings.send_pending_ack(&mut self.codec)); - try_ready!(self.streams.send_pending_refusal(&mut self.codec)); - - Ok(().into()) - } - - /// Returns `Ready` when the `Connection` is ready to accept a frame from - /// the user - /// - /// This function is currently used by poll_complete, but at some point it - /// will probably not be required. - fn poll_send_ready(&mut self) -> Poll<(), ConnectionError> { - try_ready!(self.poll_recv_ready()); - - // Ensure all window updates have been sent. - try_ready!(self.streams.send_pending_window_updates(&mut self.codec)); - - Ok(().into()) - } - - /// Try to receive the next frame - fn recv_frame(&mut self) -> Poll>, ConnectionError> { + /// Advances the internal state of the connection. + pub fn poll(&mut self) -> Poll, ConnectionError> { use frame::Frame::*; loop { @@ -159,9 +103,7 @@ impl Connection let frame = match try!(self.codec.poll()) { Async::Ready(frame) => frame, Async::NotReady => { - // Receiving new frames may depend on ensuring that the write buffer - // is clear (e.g. if window updates need to be sent), so `poll_complete` - // is called here. + // Flush any pending writes let _ = try!(self.poll_complete()); return Ok(Async::NotReady); } @@ -170,14 +112,23 @@ impl Connection match frame { Some(Headers(frame)) => { trace!("recv HEADERS; frame={:?}", frame); + + if let Some(frame) = try!(self.streams.recv_headers(frame)) { + unimplemented!(); + } + + /* // Update stream state while ensuring that the headers frame // can be received. if let Some(frame) = try!(self.streams.recv_headers(frame)) { let frame = Self::convert_poll_message(frame)?; return Ok(Some(frame).into()); } + */ } Some(Data(frame)) => { + unimplemented!(); + /* trace!("recv DATA; frame={:?}", frame); try!(self.streams.recv_data(&frame)); @@ -188,8 +139,11 @@ impl Connection }; return Ok(Some(frame).into()); + */ } Some(Reset(frame)) => { + unimplemented!(); + /* trace!("recv RST_STREAM; frame={:?}", frame); try!(self.streams.recv_reset(&frame)); @@ -199,10 +153,14 @@ impl Connection }; return Ok(Some(frame).into()); + */ } Some(PushPromise(frame)) => { + unimplemented!(); + /* trace!("recv PUSH_PROMISE; frame={:?}", frame); try!(self.streams.recv_push_promise(frame)); + */ } Some(Settings(frame)) => { trace!("recv SETTINGS; frame={:?}", frame); @@ -211,23 +169,92 @@ impl Connection // TODO: ACK must be sent THEN settings applied. } Some(Ping(frame)) => { + unimplemented!(); + /* trace!("recv PING; frame={:?}", frame); self.ping_pong.recv_ping(frame); + */ } Some(WindowUpdate(frame)) => { + unimplemented!(); + /* trace!("recv WINDOW_UPDATE; frame={:?}", frame); try!(self.streams.recv_window_update(frame)); + */ } None => { + unimplemented!(); + /* trace!("codec closed"); return Ok(Async::Ready(None)); + */ } } } + + // TODO: Flush the write buffer + unimplemented!(); + } + + /* + pub fn send_data(self, + id: StreamId, + data: B, + end_of_stream: bool) + -> sink::Send + { + self.send(Frame::Data { + id, + data, + end_of_stream, + }) + } + + pub fn send_trailers(self, + id: StreamId, + headers: HeaderMap) + -> sink::Send + { + self.send(Frame::Trailers { + id, + headers, + }) + } + */ + + // ===== Private ===== + + /// Returns `Ready` when the `Connection` is ready to receive a frame from + /// the socket. + fn poll_recv_ready(&mut self) -> Poll<(), ConnectionError> { + // Pong, settings ack, and stream refusals are high priority frames to + // send. If the write buffer is full, we stop reading any further frames + // until these high priority writes can be committed to the buffer. + + try_ready!(self.ping_pong.send_pending_pong(&mut self.codec)); + try_ready!(self.settings.send_pending_ack(&mut self.codec)); + try_ready!(self.streams.send_pending_refusal(&mut self.codec)); + + Ok(().into()) + } + + /// Returns `Ready` when the `Connection` is ready to accept a frame from + /// the user + /// + /// This function is currently used by poll_complete, but at some point it + /// will probably not be required. + fn poll_send_ready(&mut self) -> Poll<(), ConnectionError> { + // TODO: Is this function needed? + try_ready!(self.poll_recv_ready()); + + Ok(().into()) } fn poll_complete(&mut self) -> Poll<(), ConnectionError> { try_ready!(self.poll_send_ready()); + + // Ensure all window updates have been sent. + try_ready!(self.streams.poll_complete(&mut self.codec)); try_ready!(self.codec.poll_complete()); Ok(().into()) diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index 2b5429e81..c03ea99ff 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -25,6 +25,7 @@ use error::Reason::*; use error::User::*; use http::{Request, Response}; +use bytes::Bytes; #[derive(Debug)] pub struct Config { diff --git a/src/proto/streams/prioritize.rs b/src/proto/streams/prioritize.rs index 0e0dbfdbf..4acf64499 100644 --- a/src/proto/streams/prioritize.rs +++ b/src/proto/streams/prioritize.rs @@ -14,7 +14,9 @@ struct Indices { tail: store::Key, } -impl Prioritize { +impl Prioritize + where B: Buf, +{ pub fn new() -> Prioritize { Prioritize { pending_send: None, @@ -58,4 +60,34 @@ impl Prioritize { stream.is_pending_send = true; } + + pub fn poll_complete(&mut self, + store: &mut Store, + dst: &mut Codec) + -> Poll<(), ConnectionError> + where T: AsyncWrite, + { + loop { + // Ensure codec is ready + try_ready!(dst.poll_ready()); + + match self.pop_frame(store) { + Some(frame) => { + // TODO: data frames should be handled specially... + let res = dst.start_send(frame)?; + + // We already verified that `dst` is ready to accept the + // write + assert!(res.is_ready()); + } + None => break, + } + } + + Ok(().into()) + } + + fn pop_frame(&mut self, store: &mut Store) -> Option> { + unimplemented!(); + } } diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 99a7a397c..0de3d4e0c 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -1,4 +1,4 @@ -use {frame, ConnectionError}; +use {client, frame, ConnectionError}; use proto::*; use super::*; @@ -23,6 +23,9 @@ pub(super) struct Recv { pending_window_updates: VecDeque, + /// Holds frames that are waiting to be read + buffer: Buffer, + /// Refused StreamId, this represents a frame that must be sent out. refused: Option, @@ -40,6 +43,7 @@ impl Recv init_window_sz: config.init_remote_window_sz, flow_control: FlowControl::new(config.init_remote_window_sz), pending_window_updates: VecDeque::new(), + buffer: Buffer::new(), refused: None, _p: PhantomData, } @@ -67,10 +71,24 @@ impl Recv } /// Transition the stream state based on receiving headers - pub fn recv_headers(&mut self, stream: &mut Stream, eos: bool) - -> Result<(), ConnectionError> + pub fn recv_headers(&mut self, + frame: frame::Headers, + stream: &mut store::Ptr) + -> Result, ConnectionError> { - stream.state.recv_open(self.init_window_sz, eos) + stream.state.recv_open(self.init_window_sz, frame.is_end_stream())?; + + // Only servers can receive a headers frame that initiates the stream. + // This is verified in `Streams` before calling this function. + if P::is_server() { + Ok(Some(frame)) + } else { + // Push the frame onto the recv buffer + stream.pending_recv.push_back(&mut self.buffer, frame.into()); + stream.notify_recv(); + + Ok(None) + } } pub fn recv_eos(&mut self, stream: &mut Stream) @@ -233,3 +251,25 @@ impl Recv unimplemented!(); } } + +impl Recv + where B: Buf, +{ + pub fn poll_response(&mut self, stream: &mut store::Ptr) + -> Poll, ConnectionError> { + // If the buffer is not empty, then the first frame must be a HEADERS + // frame or the user violated the contract. + match stream.pending_recv.pop_front(&mut self.buffer) { + Some(Frame::Headers(v)) => { + // TODO: This error should probably be caught on receipt of the + // frame vs. now. + Ok(client::Peer::convert_poll_message(v)?.into()) + } + Some(frame) => unimplemented!(), + None => { + stream.recv_task = Some(task::current()); + Ok(Async::NotReady) + } + } + } +} diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 6ed4fdd1d..a6d80316d 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -151,6 +151,15 @@ impl Send Ok(()) } + pub fn poll_complete(&mut self, + store: &mut Store, + dst: &mut Codec) + -> Poll<(), ConnectionError> + where T: AsyncWrite, + { + self.prioritize.poll_complete(store, dst) + } + /// Get pending window updates pub fn poll_window_update(&mut self, streams: &mut Store) -> Poll diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index b19c755c0..d59eb5bc7 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -47,6 +47,13 @@ impl Store { } } + pub fn resolve(&mut self, key: Key) -> Ptr { + Ptr { + key: key, + store: self, + } + } + pub fn find_mut(&mut self, id: &StreamId) -> Option<&mut Stream> { if let Some(handle) = self.ids.get(id) { Some(&mut self.slab[*handle]) @@ -117,6 +124,10 @@ impl<'a, B: 'a> ops::DerefMut for Ptr<'a, B> { // ===== impl OccupiedEntry ===== impl<'a, B> OccupiedEntry<'a, B> { + pub fn key(&self) -> Key { + Key(*self.ids.get()) + } + pub fn get(&self) -> &Stream { &self.slab[*self.ids.get()] } @@ -133,13 +144,13 @@ impl<'a, B> OccupiedEntry<'a, B> { // ===== impl VacantEntry ===== // impl<'a, B> VacantEntry<'a, B> { - pub fn insert(self, value: Stream) -> &'a mut Stream { + pub fn insert(self, value: Stream) -> Key { // Insert the value in the slab - let handle = self.slab.insert(value); + let key = self.slab.insert(value); // Insert the handle in the ID map - self.ids.insert(handle); + self.ids.insert(key); - &mut self.slab[handle] + Key(key) } } diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs index 0689d3cdb..588edcb71 100644 --- a/src/proto/streams/stream.rs +++ b/src/proto/streams/stream.rs @@ -5,6 +5,12 @@ pub(super) struct Stream { /// Current state of the stream pub state: State, + /// Frames pending for this stream to read + pub pending_recv: buffer::Deque, + + /// Task tracking receiving frames + pub recv_task: Option, + /// Frames pending for this stream being sent to the socket pub pending_send: buffer::Deque, @@ -19,6 +25,8 @@ impl Stream { pub fn new() -> Stream { Stream { state: State::default(), + pending_recv: buffer::Deque::new(), + recv_task: None, pending_send: buffer::Deque::new(), next_pending_send: None, is_pending_send: false, @@ -32,4 +40,10 @@ impl Stream { pub fn recv_flow_control(&mut self) -> Option<&mut FlowControl> { self.state.recv_flow_control() } + + pub fn notify_recv(&mut self) { + if let Some(ref mut task) = self.recv_task { + task.notify(); + } + } } diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index b5b3ded80..231ea017e 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -15,7 +15,7 @@ pub struct Streams { #[derive(Debug)] pub struct StreamRef { inner: Arc>>, - id: StreamId, + key: store::Key, } /// Fields needed to manage state related to managing the set of streams. This @@ -53,6 +53,7 @@ impl Streams } } + /// Process inbound headers pub fn recv_headers(&mut self, frame: frame::Headers) -> Result, ConnectionError> { @@ -60,8 +61,8 @@ impl Streams let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let stream = match me.store.find_entry(id) { - Entry::Occupied(e) => e.into_mut(), + let key = match me.store.find_entry(id) { + Entry::Occupied(e) => e.key(), Entry::Vacant(e) => { // Trailers cannot open a stream. Trailers are header frames // that do not contain pseudo headers. Requests MUST contain a @@ -78,22 +79,28 @@ impl Streams } }; - if frame.is_trailers() { + let mut stream = me.store.resolve(key); + + let ret = if frame.is_trailers() { + unimplemented!(); + /* if !frame.is_end_stream() { // TODO: What error should this return? unimplemented!(); } try!(me.actions.recv.recv_eos(stream)); + */ } else { - try!(me.actions.recv.recv_headers(stream, frame.is_end_stream())); - } + try!(me.actions.recv.recv_headers(frame, &mut stream)) + }; + // TODO: move this into a fn if stream.state.is_closed() { me.actions.dec_num_streams(id); } - Ok(Some(frame)) + Ok(ret) } pub fn recv_data(&mut self, frame: &frame::Data) @@ -241,16 +248,20 @@ impl Streams me.actions.recv.send_pending_refusal(dst) } - pub fn send_pending_window_updates(&mut self, dst: &mut Codec) + pub fn poll_complete(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> where T: AsyncWrite, { let mut me = self.inner.lock().unwrap(); let me = &mut *me; + + // TODO: sending window updates should be part of Prioritize + /* try_ready!(me.actions.recv.send_connection_window_update(dst)); try_ready!(me.actions.recv.send_stream_window_update(&mut me.store, dst)); + */ - Ok(().into()) + me.actions.send.poll_complete(&mut me.store, dst) } } @@ -260,7 +271,7 @@ impl Streams pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) -> Result, ConnectionError> { - let id = { + let key = { let mut me = self.inner.lock().unwrap(); let me = &mut *me; @@ -279,16 +290,29 @@ impl Streams // closed state. debug_assert!(!stream.state.is_closed()); - id + stream.key() }; Ok(StreamRef { inner: self.inner.clone(), - id: id, + key: key, }) } } +impl StreamRef + where B: Buf, +{ + pub fn poll_response(&mut self) -> Poll, ConnectionError> { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let mut stream = me.store.resolve(self.key); + + me.actions.recv.poll_response(&mut stream) + } +} + impl Actions where P: Peer, B: Buf, From 1c55ad75ea7cbcdad8673c69a1c5ec807b95bbed Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 4 Aug 2017 17:25:39 -0700 Subject: [PATCH 08/19] More code --- src/client.rs | 23 ++++++++++ src/proto/connection.rs | 8 ++-- src/proto/streams/buffer.rs | 41 +++++++++++++++--- src/proto/streams/prioritize.rs | 76 ++++++++++++++++++++++++--------- src/proto/streams/store.rs | 2 +- src/proto/streams/streams.rs | 5 +++ tests/stream_states.rs | 16 +++++-- tests/support/mod.rs | 8 ++-- 8 files changed, 139 insertions(+), 40 deletions(-) diff --git a/src/client.rs b/src/client.rs index dae10406a..c980b59d9 100644 --- a/src/client.rs +++ b/src/client.rs @@ -24,6 +24,7 @@ pub struct Client { } /// Client half of an active HTTP/2.0 stream. +#[derive(Debug)] pub struct Stream { inner: proto::StreamRef, } @@ -89,6 +90,19 @@ impl Client } } +impl Future for Client + // TODO: Get rid of 'static + where T: AsyncRead + AsyncWrite + 'static, + B: IntoBuf + 'static, +{ + type Item = (); + type Error = ConnectionError; + + fn poll(&mut self) -> Poll<(), ConnectionError> { + self.connection.poll() + } +} + impl fmt::Debug for Client where T: fmt::Debug, B: fmt::Debug + IntoBuf, @@ -145,6 +159,15 @@ impl Stream { } } +impl Future for Stream { + type Item = Response<()>; + type Error = ConnectionError; + + fn poll(&mut self) -> Poll { + self.poll_response() + } +} + // ===== impl Peer ===== impl proto::Peer for Peer { diff --git a/src/proto/connection.rs b/src/proto/connection.rs index b363c18c4..e8b3cd61f 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -91,7 +91,7 @@ impl Connection } /// Advances the internal state of the connection. - pub fn poll(&mut self) -> Poll, ConnectionError> { + pub fn poll(&mut self) -> Poll<(), ConnectionError> { use frame::Frame::*; loop { @@ -183,11 +183,9 @@ impl Connection */ } None => { - unimplemented!(); - /* + // TODO: Is this correct? trace!("codec closed"); - return Ok(Async::Ready(None)); - */ + return Ok(Async::Ready(())); } } } diff --git a/src/proto/streams/buffer.rs b/src/proto/streams/buffer.rs index a8613c62f..8aa1c991f 100644 --- a/src/proto/streams/buffer.rs +++ b/src/proto/streams/buffer.rs @@ -18,7 +18,7 @@ pub struct Deque { } /// Tracks the head & tail for a sequence of frames in a `Buffer`. -#[derive(Debug, Default)] +#[derive(Debug, Default, Copy, Clone)] struct Indices { head: usize, tail: usize, @@ -27,7 +27,7 @@ struct Indices { #[derive(Debug)] struct Slot { frame: Frame, - next: usize, + next: Option, } impl Buffer { @@ -50,11 +50,42 @@ impl Deque { self.indices.is_none() } - pub fn push_back(&mut self, buf: &mut Buffer, val: Frame) { - unimplemented!(); + pub fn push_back(&mut self, buf: &mut Buffer, frame: Frame) { + let key = buf.slab.insert(Slot { + frame, + next: None, + }); + + match self.indices { + Some(ref mut idxs) => { + buf.slab[idxs.tail].next = Some(key); + idxs.tail = key; + } + None => { + self.indices = Some(Indices { + head: key, + tail: key, + }); + } + } } pub fn pop_front(&mut self, buf: &mut Buffer) -> Option> { - unimplemented!(); + match self.indices { + Some(mut idxs) => { + let mut slot = buf.slab.remove(idxs.head); + + if idxs.head == idxs.tail { + assert!(slot.next.is_none()); + self.indices = None; + } else { + idxs.head = slot.next.take().unwrap(); + self.indices = Some(idxs); + } + + return Some(slot.frame); + } + None => None, + } } } diff --git a/src/proto/streams/prioritize.rs b/src/proto/streams/prioritize.rs index 4acf64499..d88548d1e 100644 --- a/src/proto/streams/prioritize.rs +++ b/src/proto/streams/prioritize.rs @@ -38,27 +38,8 @@ impl Prioritize return; } - // The next pointer shouldn't be set - debug_assert!(stream.next_pending_send.is_none()); - // Queue the stream - match self.pending_send { - Some(ref mut idxs) => { - // Update the current tail node to point to `stream` - stream.resolve(idxs.tail).next_pending_send = Some(stream.key()); - - // Update the tail pointer - idxs.tail = stream.key(); - } - None => { - self.pending_send = Some(Indices { - head: stream.key(), - tail: stream.key(), - }); - } - } - - stream.is_pending_send = true; + self.push_sender(stream); } pub fn poll_complete(&mut self, @@ -88,6 +69,59 @@ impl Prioritize } fn pop_frame(&mut self, store: &mut Store) -> Option> { - unimplemented!(); + match self.pop_sender(store) { + Some(mut stream) => { + let frame = stream.pending_send.pop_front(&mut self.buffer).unwrap(); + + if !stream.pending_send.is_empty() { + self.push_sender(&mut stream); + } + + Some(frame) + } + None => None, + } + } + + fn push_sender(&mut self, stream: &mut store::Ptr) { + // The next pointer shouldn't be set + debug_assert!(stream.next_pending_send.is_none()); + + // Queue the stream + match self.pending_send { + Some(ref mut idxs) => { + // Update the current tail node to point to `stream` + stream.resolve(idxs.tail).next_pending_send = Some(stream.key()); + + // Update the tail pointer + idxs.tail = stream.key(); + } + None => { + self.pending_send = Some(Indices { + head: stream.key(), + tail: stream.key(), + }); + } + } + + stream.is_pending_send = true; + } + + fn pop_sender<'a>(&mut self, store: &'a mut Store) -> Option> { + if let Some(mut idxs) = self.pending_send { + let mut stream = store.resolve(idxs.head); + + if idxs.head == idxs.tail { + assert!(stream.next_pending_send.is_none()); + self.pending_send = None; + } else { + idxs.head = stream.next_pending_send.take().unwrap(); + self.pending_send = Some(idxs); + } + + return Some(stream); + } + + None } } diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index d59eb5bc7..e4af18174 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -19,7 +19,7 @@ pub(super) struct Ptr<'a, B: 'a> { } /// References an entry in the store. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) struct Key(usize); pub(super) enum Entry<'a, B: 'a> { diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 231ea017e..8134b804d 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -271,6 +271,11 @@ impl Streams pub fn send_request(&mut self, request: Request<()>, end_of_stream: bool) -> Result, ConnectionError> { + // TODO: There is a hazard with assigning a stream ID before the + // prioritize layer. If prioritization reorders new streams, this + // implicitly closes the earlier stream IDs. + // + // See: carllerche/h2#11 let key = { let mut me = self.inner.lock().unwrap(); let me = &mut *me; diff --git a/tests/stream_states.rs b/tests/stream_states.rs index 2b1594f45..aeaf2af0e 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -23,16 +23,21 @@ fn send_recv_headers_only() { .read(&[0, 0, 1, 1, 5, 0, 0, 0, 1, 0x89]) .build(); - let h2 = client::handshake(mock) + let mut h2 = Client::handshake(mock) .wait().unwrap(); // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); + let request = Request::builder() + .uri("https://http2.akamai.com/") + .body(()).unwrap(); info!("sending request"); - let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); + let stream = h2.request(request, true).unwrap(); + + let resp = stream.select2(h2).wait().ok().unwrap(); + println!("GOT: {:?}", resp); + /* // Get the response info!("getting response"); @@ -48,8 +53,10 @@ fn send_recv_headers_only() { // No more frames info!("ensure no more responses"); assert!(Stream::wait(h2).next().is_none());; + */ } +/* #[test] fn send_recv_data() { let _ = env_logger::init(); @@ -257,3 +264,4 @@ fn send_data_without_headers() { #[ignore] fn exceed_max_streams() { } +*/ diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 3074d5272..0d2ebf6b2 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -18,12 +18,12 @@ pub use self::http::{ response, method, status, + Request, + Response, }; -pub use self::h2::{ - client, - server, -}; +pub use self::h2::client::{self, Client}; +// pub use self::h2::server; pub use self::bytes::{ Buf, From 650b40fc90c9db26942db5d458e9007d4fcf90d7 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 4 Aug 2017 17:29:37 -0700 Subject: [PATCH 09/19] Zomg something works --- tests/stream_states.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/stream_states.rs b/tests/stream_states.rs index aeaf2af0e..c87529713 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -34,8 +34,13 @@ fn send_recv_headers_only() { info!("sending request"); let stream = h2.request(request, true).unwrap(); - let resp = stream.select2(h2).wait().ok().unwrap(); - println!("GOT: {:?}", resp); + // Wait + h2.wait().ok().unwrap(); + + // Try to get response + println!("GOT: {:?}", stream.wait().ok().unwrap()); + + // let resp = stream.select2(h2).wait().ok().unwrap(); /* // Get the response From 90df6e387901e85f7f7d0cbadcdadc72d0db1afe Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 4 Aug 2017 20:54:49 -0700 Subject: [PATCH 10/19] Try to clean up test --- tests/stream_states.rs | 7 +++---- tests/support/mod.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/tests/stream_states.rs b/tests/stream_states.rs index c87529713..1a4afe071 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -32,13 +32,12 @@ fn send_recv_headers_only() { .body(()).unwrap(); info!("sending request"); - let stream = h2.request(request, true).unwrap(); + let mut stream = h2.request(request, true).unwrap(); - // Wait - h2.wait().ok().unwrap(); + let resp = h2.run(poll_fn(|| stream.poll_response())).unwrap(); // Try to get response - println!("GOT: {:?}", stream.wait().ok().unwrap()); + println!("GOT: {:?}", resp); // let resp = stream.select2(h2).wait().ok().unwrap(); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 0d2ebf6b2..47869d8ff 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -3,6 +3,7 @@ pub extern crate bytes; pub extern crate h2; pub extern crate http; +pub extern crate tokio_io; pub extern crate futures; pub extern crate mock_io; pub extern crate env_logger; @@ -12,6 +13,7 @@ pub use self::futures::{ Sink, Stream, }; +pub use self::futures::future::poll_fn; pub use self::http::{ request, @@ -30,8 +32,11 @@ pub use self::bytes::{ BufMut, Bytes, BytesMut, + IntoBuf, }; +use tokio_io::{AsyncRead, AsyncWrite}; + pub trait MockH2 { fn handshake(&mut self) -> &mut Self; } @@ -46,6 +51,33 @@ impl MockH2 for mock_io::Builder { } } +pub trait ClientExt { + fn run(&mut self, f: F) -> Result; +} + +impl ClientExt for Client + where T: AsyncRead + AsyncWrite + 'static, + B: IntoBuf + 'static, +{ + fn run(&mut self, f: F) -> Result { + use futures::future::{self, Future}; + use futures::future::Either::*; + + let res = future::poll_fn(|| self.poll()) + .select2(f).wait(); + + match res { + Ok(A((_, b))) => { + // Connection is done... + b.wait() + } + Ok(B((v, _))) => return Ok(v), + Err(A((e, _))) => panic!("err: {:?}", e), + Err(B((e, _))) => return Err(e), + } + } +} + pub mod frames { //! Some useful frames From d918215397a39ff2424503a300e60bc986b97d42 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 4 Aug 2017 22:19:42 -0700 Subject: [PATCH 11/19] Fix test --- tests/stream_states.rs | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/tests/stream_states.rs b/tests/stream_states.rs index 1a4afe071..eef431837 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -35,29 +35,9 @@ fn send_recv_headers_only() { let mut stream = h2.request(request, true).unwrap(); let resp = h2.run(poll_fn(|| stream.poll_response())).unwrap(); + assert_eq!(resp.status(), status::NO_CONTENT); - // Try to get response - println!("GOT: {:?}", resp); - - // let resp = stream.select2(h2).wait().ok().unwrap(); - - /* - // Get the response - - info!("getting response"); - let (resp, h2) = h2.into_future().wait().unwrap(); - - match resp.unwrap() { - Frame::Headers { headers, .. } => { - assert_eq!(headers.status, status::NO_CONTENT); - } - _ => panic!("unexpected frame"), - } - - // No more frames - info!("ensure no more responses"); - assert!(Stream::wait(h2).next().is_none());; - */ + h2.wait().unwrap(); } /* From 71acfe3961a5dacfeb25929812d4ed4a3f2d2142 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Mon, 7 Aug 2017 12:17:52 -0700 Subject: [PATCH 12/19] Start hooking up data receiving --- src/client.rs | 33 +++++++++--------- src/frame/mod.rs | 9 +++++ src/proto/connection.rs | 13 +------ src/proto/mod.rs | 2 +- src/proto/streams/buffer.rs | 49 ++++++++++++++++++++++++++ src/proto/streams/mod.rs | 2 +- src/proto/streams/recv.rs | 39 ++++++++++++++++++++- src/proto/streams/streams.rs | 67 +++++++++++++++++++++++++++++++++++- 8 files changed, 181 insertions(+), 33 deletions(-) diff --git a/src/client.rs b/src/client.rs index c980b59d9..7ae70c023 100644 --- a/src/client.rs +++ b/src/client.rs @@ -23,12 +23,21 @@ pub struct Client { connection: Connection, } -/// Client half of an active HTTP/2.0 stream. #[derive(Debug)] pub struct Stream { inner: proto::StreamRef, } +#[derive(Debug)] +pub struct Body { + inner: proto::StreamRef, +} + +#[derive(Debug)] +pub struct Chunk { + inner: proto::Chunk, +} + impl Client where T: AsyncRead + AsyncWrite + 'static, { @@ -90,19 +99,6 @@ impl Client } } -impl Future for Client - // TODO: Get rid of 'static - where T: AsyncRead + AsyncWrite + 'static, - B: IntoBuf + 'static, -{ - type Item = (); - type Error = ConnectionError; - - fn poll(&mut self) -> Poll<(), ConnectionError> { - self.connection.poll() - } -} - impl fmt::Debug for Client where T: fmt::Debug, B: fmt::Debug + IntoBuf, @@ -140,8 +136,11 @@ impl fmt::Debug for Handshake impl Stream { /// Receive the HTTP/2.0 response, if it is ready. - pub fn poll_response(&mut self) -> Poll, ConnectionError> { - self.inner.poll_response() + pub fn poll_response(&mut self) -> Poll>, ConnectionError> { + let (parts, _) = try_ready!(self.inner.poll_response()).into_parts(); + let body = Body { inner: self.inner.clone() }; + + Ok(Response::from_parts(parts, body).into()) } /// Send data @@ -160,7 +159,7 @@ impl Stream { } impl Future for Stream { - type Item = Response<()>; + type Item = Response>; type Error = ConnectionError; fn poll(&mut self) -> Poll { diff --git a/src/frame/mod.rs b/src/frame/mod.rs index 2480a71ba..b7a8022e2 100644 --- a/src/frame/mod.rs +++ b/src/frame/mod.rs @@ -67,6 +67,15 @@ pub enum Frame { } impl Frame { + /// Returns true if the frame is a DATA frame. + pub fn is_data(&self) -> bool { + use self::Frame::*; + + match *self { + Data(..) => true, + _ => false, + } + } } impl fmt::Debug for Frame { diff --git a/src/proto/connection.rs b/src/proto/connection.rs index e8b3cd61f..a28ce80b1 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -127,19 +127,8 @@ impl Connection */ } Some(Data(frame)) => { - unimplemented!(); - /* trace!("recv DATA; frame={:?}", frame); - try!(self.streams.recv_data(&frame)); - - let frame = Frame::Data { - id: frame.stream_id(), - end_of_stream: frame.is_end_stream(), - data: frame.into_payload(), - }; - - return Ok(Some(frame).into()); - */ + try!(self.streams.recv_data(frame)); } Some(Reset(frame)) => { unimplemented!(); diff --git a/src/proto/mod.rs b/src/proto/mod.rs index aaca6dbc4..7c842d95f 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -6,7 +6,7 @@ mod settings; mod streams; pub use self::connection::Connection; -pub use self::streams::{Streams, StreamRef}; +pub use self::streams::{Streams, StreamRef, Chunk}; use self::framed_read::FramedRead; use self::framed_write::FramedWrite; diff --git a/src/proto/streams/buffer.rs b/src/proto/streams/buffer.rs index 8aa1c991f..077d10d39 100644 --- a/src/proto/streams/buffer.rs +++ b/src/proto/streams/buffer.rs @@ -88,4 +88,53 @@ impl Deque { None => None, } } + + pub fn take_while(&mut self, buf: &mut Buffer, mut f: F) -> Self + where F: FnMut(&Frame) -> bool + { + match self.indices { + Some(mut idxs) => { + if !f(&buf.slab[idxs.head].frame) { + return Deque::new(); + } + + let head = idxs.head; + let mut tail = idxs.head; + + loop { + let next = match buf.slab[tail].next { + Some(next) => next, + None => { + self.indices = None; + return Deque { + indices: Some(idxs), + _p: PhantomData, + }; + } + }; + + if !f(&buf.slab[next].frame) { + // Split the linked list + buf.slab[tail].next = None; + + self.indices = Some(Indices { + head: next, + tail: idxs.tail, + }); + + return Deque { + indices: Some(Indices { + head: head, + tail: tail, + }), + _p: PhantomData, + } + } + + tail = next; + } + } + None => Deque::new(), + } + } } diff --git a/src/proto/streams/mod.rs b/src/proto/streams/mod.rs index c03ea99ff..1dfe92b2f 100644 --- a/src/proto/streams/mod.rs +++ b/src/proto/streams/mod.rs @@ -8,7 +8,7 @@ mod store; mod stream; mod streams; -pub use self::streams::{Streams, StreamRef}; +pub use self::streams::{Streams, StreamRef, Chunk}; use self::buffer::Buffer; use self::flow_control::FlowControl; diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 0de3d4e0c..8a6d4d1f5 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -32,6 +32,12 @@ pub(super) struct Recv { _p: PhantomData<(P, B)>, } +#[derive(Debug)] +pub(super) struct Chunk { + /// Data frames pending receival + pub pending_recv: buffer::Deque, +} + impl Recv where P: Peer, B: Buf, @@ -98,7 +104,7 @@ impl Recv } pub fn recv_data(&mut self, - frame: &frame::Data, + frame: frame::Data, stream: &mut Stream) -> Result<(), ConnectionError> { @@ -130,6 +136,10 @@ impl Recv try!(stream.state.recv_close()); } + // Push the frame onto the recv buffer + stream.pending_recv.push_back(&mut self.buffer, frame.into()); + stream.notify_recv(); + Ok(()) } @@ -218,6 +228,33 @@ impl Recv Ok(().into()) } + + pub fn poll_chunk(&mut self, stream: &mut Stream) + -> Poll, ConnectionError> + { + let frames = stream.pending_recv + .take_while(&mut self.buffer, |frame| frame.is_data()); + + if frames.is_empty() { + stream.recv_task = Some(task::current()); + Ok(Async::NotReady) + } else { + Ok(Some(Chunk { + pending_recv: frames, + }).into()) + } + } + + pub fn pop_bytes(&mut self, chunk: &mut Chunk) -> Option { + match chunk.pending_recv.pop_front(&mut self.buffer) { + Some(Frame::Data(frame)) => { + Some(frame.into_payload()) + } + None => None, + _ => panic!("unexpected frame type"), + } + } + /// Send stream level window update pub fn send_stream_window_update(&mut self, streams: &mut Store, diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 8134b804d..23a44be08 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -18,6 +18,15 @@ pub struct StreamRef { key: store::Key, } +#[derive(Debug)] +pub struct Chunk + where P: Peer, + B: Buf, +{ + inner: Arc>>, + recv: recv::Chunk, +} + /// Fields needed to manage state related to managing the set of streams. This /// is mostly split out to make ownership happy. /// @@ -103,7 +112,7 @@ impl Streams Ok(ret) } - pub fn recv_data(&mut self, frame: &frame::Data) + pub fn recv_data(&mut self, frame: frame::Data) -> Result<(), ConnectionError> { let id = frame.stream_id(); @@ -305,6 +314,34 @@ impl Streams } } +// ===== impl StreamRef ===== + +impl StreamRef + where P: Peer, + B: Buf, +{ + pub fn poll_data(&mut self) -> Poll>, ConnectionError> { + let recv = { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let mut stream = me.store.resolve(self.key); + + try_ready!(me.actions.recv.poll_chunk(&mut stream)) + }; + + // Convert to a chunk + let chunk = recv.map(|recv| { + Chunk { + inner: self.inner.clone(), + recv: recv, + } + }); + + Ok(chunk.into()) + } +} + impl StreamRef where B: Buf, { @@ -318,6 +355,34 @@ impl StreamRef } } + + +impl Clone for StreamRef { + fn clone(&self) -> Self { + StreamRef { + inner: self.inner.clone(), + key: self.key.clone(), + } + } +} + +// ===== impl Chunk ===== + +impl Drop for Chunk + where P: Peer, + B: Buf, +{ + fn drop(&mut self) { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + while let Some(_) = me.actions.recv.pop_bytes(&mut self.recv) { + } + } +} + +// ===== impl Actions ===== + impl Actions where P: Peer, B: Buf, From 6053ee059d5af78809ba8005f9e6adeb351dd3dc Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Mon, 7 Aug 2017 12:48:50 -0700 Subject: [PATCH 13/19] Get receiving data working --- src/client.rs | 37 +++++++++++++++++++++- src/proto/streams/recv.rs | 8 +++-- src/proto/streams/state.rs | 7 +++++ src/proto/streams/streams.rs | 13 ++++++++ tests/stream_states.rs | 60 +++++++++++++++--------------------- 5 files changed, 86 insertions(+), 39 deletions(-) diff --git a/src/client.rs b/src/client.rs index 7ae70c023..ccec35679 100644 --- a/src/client.rs +++ b/src/client.rs @@ -3,7 +3,7 @@ use proto::{self, Connection}; use error::Reason::*; use http::{self, Request, Response}; -use futures::{Future, Poll, Sink, AsyncSink}; +use futures::{self, Future, Poll, Sink, AsyncSink}; use tokio_io::{AsyncRead, AsyncWrite}; use bytes::{Bytes, IntoBuf}; @@ -99,6 +99,19 @@ impl Client } } +impl Future for Client + // TODO: Get rid of 'static + where T: AsyncRead + AsyncWrite + 'static, + B: IntoBuf + 'static, +{ + type Item = (); + type Error = ConnectionError; + + fn poll(&mut self) -> Poll<(), ConnectionError> { + self.connection.poll() + } +} + impl fmt::Debug for Client where T: fmt::Debug, B: fmt::Debug + IntoBuf, @@ -167,6 +180,28 @@ impl Future for Stream { } } +// ===== impl Body ===== + +impl futures::Stream for Body { + type Item = Chunk; + type Error = ConnectionError; + + fn poll(&mut self) -> Poll, Self::Error> { + let chunk = try_ready!(self.inner.poll_data()) + .map(|inner| Chunk { inner }); + + Ok(chunk.into()) + } +} + +// ===== impl Chunk ===== + +impl Chunk { + pub fn pop_bytes(&mut self) -> Option { + self.inner.pop_bytes() + } +} + // ===== impl Peer ===== impl proto::Peer for Peer { diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 8a6d4d1f5..f7ceba910 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -236,8 +236,12 @@ impl Recv .take_while(&mut self.buffer, |frame| frame.is_data()); if frames.is_empty() { - stream.recv_task = Some(task::current()); - Ok(Async::NotReady) + if stream.state.is_recv_closed() { + Ok(None.into()) + } else { + stream.recv_task = Some(task::current()); + Ok(Async::NotReady) + } } else { Ok(Some(Chunk { pending_recv: frames, diff --git a/src/proto/streams/state.rs b/src/proto/streams/state.rs index 369ac8c96..e6b681cc5 100644 --- a/src/proto/streams/state.rs +++ b/src/proto/streams/state.rs @@ -203,6 +203,13 @@ impl State { } } + pub fn is_recv_closed(&self) -> bool { + match self.inner { + Closed(..) | HalfClosedRemote(..) => true, + _ => false, + } + } + pub fn recv_flow_control(&mut self) -> Option<&mut FlowControl> { match self.inner { Open { ref mut remote, .. } | diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 23a44be08..9f59a2ddc 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -368,6 +368,19 @@ impl Clone for StreamRef { // ===== impl Chunk ===== +impl Chunk + where P: Peer, + B: Buf, +{ + // TODO: Come up w/ a better API + pub fn pop_bytes(&mut self) -> Option { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + me.actions.recv.pop_bytes(&mut self.recv) + } +} + impl Drop for Chunk where P: Peer, B: Buf, diff --git a/tests/stream_states.rs b/tests/stream_states.rs index eef431837..e3c480ef6 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -5,8 +5,6 @@ extern crate log; pub mod support; use support::*; -use h2::Frame; - #[test] fn send_recv_headers_only() { let _ = env_logger::init(); @@ -103,9 +101,10 @@ fn send_recv_data() { assert!(Stream::wait(h2).next().is_none());; } +*/ #[test] -fn send_headers_recv_data() { +fn send_headers_recv_data_single_frame() { let _ = env_logger::init(); let mock = mock_io::Builder::new() @@ -123,51 +122,40 @@ fn send_headers_recv_data() { ]) .build(); - let h2 = client::handshake(mock) + let mut h2 = Client::handshake(mock) .wait().unwrap(); // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); + let request = Request::builder() + .uri("https://http2.akamai.com/") + .body(()).unwrap(); - // Get the response headers - let (resp, h2) = h2.into_future().wait().unwrap(); + info!("sending request"); + let mut stream = h2.request(request, true).unwrap(); - match resp.unwrap() { - Frame::Headers { headers, .. } => { - assert_eq!(headers.status, status::OK); - } - _ => panic!("unexpected frame"), - } + let resp = h2.run(poll_fn(|| stream.poll_response())).unwrap(); + assert_eq!(resp.status(), status::OK); - // Get the response body - let (data, h2) = h2.into_future().wait().unwrap(); + // Take the body + let (_, body) = resp.into_parts(); - match data.unwrap() { - Frame::Data { id, data, end_of_stream, .. } => { - assert_eq!(id, 1.into()); - assert_eq!(data, &b"hello"[..]); - assert!(!end_of_stream); - } - _ => panic!("unexpected frame"), - } + // Wait for all the data frames to be received + let mut chunks = h2.run(body.collect()).unwrap(); - // Get the response body - let (data, h2) = h2.into_future().wait().unwrap(); + // Only one chunk since two frames are coalesced. + assert_eq!(1, chunks.len()); - match data.unwrap() { - Frame::Data { id, data, end_of_stream, .. } => { - assert_eq!(id, 1.into()); - assert_eq!(data, &b"world"[..]); - assert!(end_of_stream); - } - _ => panic!("unexpected frame"), - } + let data = chunks[0].pop_bytes().unwrap(); + assert_eq!(data, &b"hello"[..]); - assert!(Stream::wait(h2).next().is_none());; + let data = chunks[0].pop_bytes().unwrap(); + assert_eq!(data, &b"world"[..]); + + // The H2 connection is closed + h2.wait().unwrap(); } +/* #[test] fn send_headers_twice_with_same_stream_id() { let _ = env_logger::init(); From 71da8d121f39ea40674329d6e0e5999816ffb5d6 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Mon, 7 Aug 2017 21:01:15 -0700 Subject: [PATCH 14/19] Start hooking up sending data --- src/client.rs | 2 +- src/proto/streams/recv.rs | 2 +- src/proto/streams/send.rs | 10 ++++---- src/proto/streams/stream.rs | 6 ++++- src/proto/streams/streams.rs | 29 ++++++++++++++++++++--- tests/stream_states.rs | 45 +++++++++++++++++++++++++++++------- 6 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/client.rs b/src/client.rs index ccec35679..c9ab889b3 100644 --- a/src/client.rs +++ b/src/client.rs @@ -160,7 +160,7 @@ impl Stream { pub fn send_data(&mut self, data: B, end_of_stream: bool) -> Result<(), ConnectionError> { - unimplemented!(); + self.inner.send_data(data.into_buf(), end_of_stream) } /// Send trailers diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index f7ceba910..0c1743412 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -73,7 +73,7 @@ impl Recv // Increment the number of remote initiated streams self.num_streams += 1; - Ok(Some(Stream::new())) + Ok(Some(Stream::new(id))) } /// Transition the stream state based on receiving headers diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index a6d80316d..8650a31f4 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -67,7 +67,7 @@ impl Send /// Update state reflecting a new, locally opened stream /// /// Returns the stream state if successful. `None` if refused - pub fn open(&mut self) -> Result<(StreamId, Stream), ConnectionError> { + pub fn open(&mut self) -> Result, ConnectionError> { try!(self.ensure_can_open()); if let Some(max) = self.max_streams { @@ -76,7 +76,7 @@ impl Send } } - let ret = (self.next_stream_id, Stream::new()); + let ret = Stream::new(self.next_stream_id); // Increment the number of locally initiated streams self.num_streams += 1; @@ -106,8 +106,8 @@ impl Send } pub fn send_data(&mut self, - frame: &frame::Data, - stream: &mut Stream) + frame: frame::Data, + stream: &mut store::Ptr) -> Result<(), ConnectionError> { let sz = frame.payload().remaining(); @@ -148,6 +148,8 @@ impl Send try!(stream.state.send_close()); } + self.prioritize.queue_frame(frame.into(), stream); + Ok(()) } diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs index 588edcb71..d26266c4c 100644 --- a/src/proto/streams/stream.rs +++ b/src/proto/streams/stream.rs @@ -2,6 +2,9 @@ use super::*; #[derive(Debug)] pub(super) struct Stream { + /// The h2 stream identifier + pub id: StreamId, + /// Current state of the stream pub state: State, @@ -22,8 +25,9 @@ pub(super) struct Stream { } impl Stream { - pub fn new() -> Stream { + pub fn new(id: StreamId) -> Stream { Stream { + id, state: State::default(), pending_recv: buffer::Deque::new(), recv_task: None, diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 9f59a2ddc..f79e34197 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -200,6 +200,7 @@ impl Streams */ } + /* pub fn send_data(&mut self, frame: &frame::Data) -> Result<(), ConnectionError> { @@ -222,6 +223,7 @@ impl Streams Ok(()) } + */ pub fn poll_window_update(&mut self) -> Poll @@ -290,13 +292,13 @@ impl Streams let me = &mut *me; // Initialize a new stream. This fails if the connection is at capacity. - let (id, mut stream) = me.actions.send.open()?; + let mut stream = me.actions.send.open()?; // Convert the message let headers = client::Peer::convert_send_message( - id, request, end_of_stream); + stream.id, request, end_of_stream); - let mut stream = me.store.insert(id, stream); + let mut stream = me.store.insert(stream.id, stream); me.actions.send.send_headers(headers, &mut stream)?; @@ -320,6 +322,27 @@ impl StreamRef where P: Peer, B: Buf, { + pub fn send_data(&mut self, data: B, end_of_stream: bool) + -> Result<(), ConnectionError> + { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let mut stream = me.store.resolve(self.key); + + // Create the data frame + let frame = frame::Data::from_buf(stream.id, data, end_of_stream); + + // Send the data frame + me.actions.send.send_data(frame, &mut stream)?; + + if stream.state.is_closed() { + me.actions.dec_num_streams(stream.id); + } + + Ok(()) + } + pub fn poll_data(&mut self) -> Poll>, ConnectionError> { let recv = { let mut me = self.inner.lock().unwrap(); diff --git a/tests/stream_states.rs b/tests/stream_states.rs index e3c480ef6..224a2a342 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -38,7 +38,6 @@ fn send_recv_headers_only() { h2.wait().unwrap(); } -/* #[test] fn send_recv_data() { let _ = env_logger::init(); @@ -64,14 +63,42 @@ fn send_recv_data() { ]) .build(); - let h2 = client::handshake(mock).wait().expect("handshake"); + let mut h2 = Client::handshake2(mock) + .wait().unwrap(); - // Send the request - let mut request = request::Head::default(); - request.method = method::POST; - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let h2 = h2.send_request(1.into(), request, false).wait().expect("send request"); + let request = Request::builder() + .method(method::POST) + .uri("https://http2.akamai.com/") + .body(()).unwrap(); + + info!("sending request"); + let mut stream = h2.request(request, false).unwrap(); + + // Send the data + stream.send_data("hello", true).unwrap(); + + // Get the response + let resp = h2.run(poll_fn(|| stream.poll_response())).unwrap(); + assert_eq!(resp.status(), status::OK); + + // Take the body + let (_, body) = resp.into_parts(); + + // Wait for all the data frames to be received + let mut chunks = h2.run(body.collect()).unwrap(); + // Only one chunk since two frames are coalesced. + assert_eq!(1, chunks.len()); + + let data = chunks[0].pop_bytes().unwrap(); + assert_eq!(data, &b"world"[..]); + + assert!(chunks[0].pop_bytes().is_none()); + + // The H2 connection is closed + h2.wait().unwrap(); + + /* let b = "hello"; // Send the data @@ -100,8 +127,8 @@ fn send_recv_data() { } assert!(Stream::wait(h2).next().is_none());; + */ } -*/ #[test] fn send_headers_recv_data_single_frame() { @@ -151,6 +178,8 @@ fn send_headers_recv_data_single_frame() { let data = chunks[0].pop_bytes().unwrap(); assert_eq!(data, &b"world"[..]); + assert!(chunks[0].pop_bytes().is_none()); + // The H2 connection is closed h2.wait().unwrap(); } From 441a8416c6f2ca7fa7600b0e3ca82d35170005ea Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Mon, 7 Aug 2017 22:35:29 -0700 Subject: [PATCH 15/19] Handle the remote returning a protocol error --- src/proto/connection.rs | 13 +++-- src/proto/streams/recv.rs | 10 ++++ src/proto/streams/state.rs | 36 ++++++++++++- src/proto/streams/store.rs | 10 +++- src/proto/streams/streams.rs | 8 +++ tests/client_request.rs | 97 +++++++++--------------------------- tests/stream_states.rs | 75 ---------------------------- 7 files changed, 96 insertions(+), 153 deletions(-) diff --git a/src/proto/connection.rs b/src/proto/connection.rs index a28ce80b1..271b7183a 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -92,6 +92,16 @@ impl Connection /// Advances the internal state of the connection. pub fn poll(&mut self) -> Poll<(), ConnectionError> { + match self.poll2() { + Err(e) => { + self.streams.recv_err(&e); + Err(e) + } + ret => ret, + } + } + + fn poll2(&mut self) -> Poll<(), ConnectionError> { use frame::Frame::*; loop { @@ -178,9 +188,6 @@ impl Connection } } } - - // TODO: Flush the write buffer - unimplemented!(); } /* diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 0c1743412..8f319c004 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -143,6 +143,14 @@ impl Recv Ok(()) } + pub fn recv_err(&mut self, err: &ConnectionError, stream: &mut Stream) { + // Receive an error + stream.state.recv_err(err); + + // If a receiver is waiting, notify it + stream.notify_recv(); + } + pub fn dec_num_streams(&mut self) { self.num_streams -= 1; } @@ -308,6 +316,8 @@ impl Recv } Some(frame) => unimplemented!(), None => { + stream.state.ensure_recv_open()?; + stream.recv_task = Some(task::current()); Ok(Async::NotReady) } diff --git a/src/proto/streams/state.rs b/src/proto/streams/state.rs index e6b681cc5..b39bd14bd 100644 --- a/src/proto/streams/state.rs +++ b/src/proto/streams/state.rs @@ -66,7 +66,7 @@ enum Inner { HalfClosedLocal(Peer), // TODO: explicitly name this value HalfClosedRemote(Peer), // When reset, a reason is provided - Closed(Option), + Closed(Option), } #[derive(Debug, Copy, Clone)] @@ -76,6 +76,12 @@ enum Peer { Streaming(FlowControl), } +#[derive(Debug, Copy, Clone)] +enum Cause { + Proto(Reason), + Io, +} + impl State { /// Opens the send-half of a stream if it is not already open. pub fn send_open(&mut self, sz: WindowSize, eos: bool) -> Result<(), ConnectionError> { @@ -178,6 +184,19 @@ impl State { } } + pub fn recv_err(&mut self, err: &ConnectionError) { + match self.inner { + Closed(..) => {} + _ => { + self.inner = Closed(match *err { + ConnectionError::Proto(reason) => Some(Cause::Proto(reason)), + ConnectionError::Io(..) => Some(Cause::Io), + _ => panic!("cannot terminate stream with user error"), + }); + } + } + } + /// Indicates that the local side will not send more data to the local. pub fn send_close(&mut self) -> Result<(), ConnectionError> { match self.inner { @@ -225,6 +244,21 @@ impl State { _ => None, } } + + pub fn ensure_recv_open(&self) -> Result<(), ConnectionError> { + use std::io; + + // TODO: Is this correct? + match self.inner { + Closed(Some(Cause::Proto(reason))) => { + Err(ConnectionError::Proto(reason)) + } + Closed(Some(Cause::Io)) => { + Err(ConnectionError::Io(io::ErrorKind::BrokenPipe.into())) + } + _ => Ok(()), + } + } } impl Default for State { diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index e4af18174..6821ff811 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -90,6 +90,14 @@ impl Store { } } } + + pub fn for_each(&mut self, mut f: F) + where F: FnMut(&mut Stream) + { + for &id in self.ids.values() { + f(&mut self.slab[id]) + } + } } // ===== impl Ptr ===== @@ -142,7 +150,7 @@ impl<'a, B> OccupiedEntry<'a, B> { } // ===== impl VacantEntry ===== -// + impl<'a, B> VacantEntry<'a, B> { pub fn insert(self, value: Stream) -> Key { // Insert the value in the slab diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index f79e34197..edfd05855 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -141,6 +141,14 @@ impl Streams unimplemented!(); } + pub fn recv_err(&mut self, err: &ConnectionError) { + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let actions = &mut me.actions; + me.store.for_each(|stream| actions.recv.recv_err(err, stream)); + } + pub fn recv_window_update(&mut self, frame: frame::WindowUpdate) -> Result<(), ConnectionError> { let id = frame.stream_id(); diff --git a/tests/client_request.rs b/tests/client_request.rs index 6e897ebba..4db144e02 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -14,47 +14,47 @@ fn handshake() { .write(SETTINGS_ACK) .build(); - let h2 = client::handshake(mock) + let h2 = Client::handshake(mock) .wait().unwrap(); + trace!("hands have been shook"); // At this point, the connection should be closed - assert!(Stream::wait(h2).next().is_none()); + h2.wait().unwrap(); } - #[test] -fn send_request_with_zero_stream_id() { +fn recv_invalid_server_stream_id() { + let _ = ::env_logger::init(); + let mock = mock_io::Builder::new() .handshake() + // Write GET / + .write(&[ + 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, + 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, + ]) + .write(SETTINGS_ACK) + // Read response + .read(&[0, 0, 1, 1, 5, 0, 0, 0, 2, 137]) .build(); - let h2 = client::handshake(mock) + let mut h2 = Client::handshake(mock) .wait().unwrap(); // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); + let request = Request::builder() + .uri("https://http2.akamai.com/") + .body(()).unwrap(); - let err = h2.send_request(0.into(), request, true).wait().unwrap_err(); - assert_user_err!(err, InvalidStreamId); -} + info!("sending request"); + let mut stream = h2.request(request, true).unwrap(); -#[test] -fn send_request_with_server_stream_id() { - let mock = mock_io::Builder::new() - .handshake() - .build(); - - let h2 = client::handshake(mock) - .wait().unwrap(); + // The connection errors + assert_proto_err!(h2.wait().unwrap_err(), ProtocolError); - // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - - let err = h2.send_request(2.into(), request, true).wait().unwrap_err(); - assert_user_err!(err, InvalidStreamId); + // The stream errors + assert_proto_err!(stream.wait().unwrap_err(), ProtocolError); } #[test] @@ -67,55 +67,6 @@ fn request_without_scheme() { fn request_with_h1_version() { } -#[test] -fn send_invalid_client_stream_id() { - let _ = ::env_logger::init(); - - for &id in &[0, 2] { - let mock = mock_io::Builder::new() - .handshake() - .build(); - - let h2 = client::handshake(mock) - .wait().unwrap(); - - // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let err = h2.send_request(id.into(), request, true).wait().unwrap_err(); - - assert_user_err!(err, InvalidStreamId); - } -} - -#[test] -fn recv_invalid_server_stream_id() { - let _ = ::env_logger::init(); - - let mock = mock_io::Builder::new() - .handshake() - // Write GET / - .write(&[ - 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, - 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, - ]) - .write(SETTINGS_ACK) - // Read response - .read(&[0, 0, 1, 1, 5, 0, 0, 0, 2, 137]) - .build(); - - let h2 = client::handshake(mock) - .wait().unwrap(); - - // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); - - // Get the response - let (err, _) = h2.into_future().wait().unwrap_err(); - assert_proto_err!(err, ProtocolError); -} #[test] #[ignore] diff --git a/tests/stream_states.rs b/tests/stream_states.rs index 224a2a342..6ef557abc 100644 --- a/tests/stream_states.rs +++ b/tests/stream_states.rs @@ -97,37 +97,6 @@ fn send_recv_data() { // The H2 connection is closed h2.wait().unwrap(); - - /* - let b = "hello"; - - // Send the data - let h2 = h2.send_data(1.into(), b.into(), true).wait().expect("send data"); - - // Get the response headers - let (resp, h2) = h2.into_future().wait().expect("into future"); - - match resp.expect("response headers") { - Frame::Headers { headers, .. } => { - assert_eq!(headers.status, status::OK); - } - _ => panic!("unexpected frame"), - } - - // Get the response body - let (data, h2) = h2.into_future().wait().expect("into future"); - - match data.expect("response data") { - Frame::Data { id, data, end_of_stream, .. } => { - assert_eq!(id, 1.into()); - assert_eq!(data, &b"world"[..]); - assert!(end_of_stream); - } - _ => panic!("unexpected frame"), - } - - assert!(Stream::wait(h2).next().is_none());; - */ } #[test] @@ -185,35 +154,6 @@ fn send_headers_recv_data_single_frame() { } /* -#[test] -fn send_headers_twice_with_same_stream_id() { - let _ = env_logger::init(); - - let mock = mock_io::Builder::new() - .handshake() - // Write GET / - .write(&[ - 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, - 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, - ]) - .build(); - - let h2 = client::handshake(mock) - .wait().unwrap(); - - // Send the request - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); - - // Send another request with the same stream ID - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - let err = h2.send_request(1.into(), request, true).wait().unwrap_err(); - - assert_user_err!(err, UnexpectedFrameType); -} - #[test] fn send_data_after_headers_eos() { let _ = env_logger::init(); @@ -246,21 +186,6 @@ fn send_data_after_headers_eos() { assert_user_err!(err, UnexpectedFrameType); } -#[test] -fn send_data_without_headers() { - let mock = mock_io::Builder::new() - .handshake() - .build(); - - let h2 = client::handshake(mock) - .wait().unwrap(); - - let b = Bytes::from_static(b"hello world"); - let err = h2.send_data(1.into(), b, true).wait().unwrap_err(); - - assert_user_err!(err, UnexpectedFrameType); -} - #[test] #[ignore] fn exceed_max_streams() { From fa66323cec8a0a69923e243d4333d9e5a87df3a9 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 8 Aug 2017 09:47:29 -0700 Subject: [PATCH 16/19] Akamai request kind of works --- examples/akamai.rs | 51 ++++++++++++++++++++++++---------------- examples/client.rs | 4 ++++ examples/server.rs | 4 ++++ src/frame/headers.rs | 32 ++++++++++++++++++++++--- src/hpack/test/fuzz.rs | 5 ---- src/proto/connection.rs | 2 +- src/proto/framed_read.rs | 4 +++- tests/ping_pong.rs | 2 +- tests/prioritization.rs | 2 ++ tests/server_preface.rs | 4 +++- 10 files changed, 78 insertions(+), 32 deletions(-) diff --git a/examples/akamai.rs b/examples/akamai.rs index 863f24966..efe609798 100644 --- a/examples/akamai.rs +++ b/examples/akamai.rs @@ -8,23 +8,29 @@ extern crate openssl; extern crate io_dump; extern crate env_logger; -use h2::client; - -use http::request; +use h2::client::Client; +use http::{method, Request}; use futures::*; use tokio_core::reactor; use tokio_core::net::TcpStream; +use std::net::ToSocketAddrs; + pub fn main() { let _ = env_logger::init(); + // Sync DNS resolution. + let addr = "http2.akamai.com:443".to_socket_addrs() + .unwrap().next().unwrap(); + + println!("ADDR: {:?}", addr); + let mut core = reactor::Core::new().unwrap();; + let handle = core.handle(); - let tcp = TcpStream::connect( - &"23.39.23.98:443".parse().unwrap(), - &core.handle()); + let tcp = TcpStream::connect(&addr, &handle); let tcp = tcp.then(|res| { use openssl::ssl::{SslMethod, SslConnectorBuilder}; @@ -46,24 +52,29 @@ pub fn main() { // Dump output to stdout let tls = io_dump::Dump::to_stdout(tls); - client::handshake(tls) + println!("Starting client handshake"); + Client::handshake(tls) }) .then(|res| { - let conn = res.unwrap(); + let mut h2 = res.unwrap(); - let mut request = request::Head::default(); - request.uri = "https://http2.akamai.com/".parse().unwrap(); - // request.version = version::H2; + let request = Request::builder() + .method(method::GET) + .uri("https://http2.akamai.com/") + .body(()).unwrap(); - conn.send_request(1.into(), request, true) - }) - .then(|res| { - let conn = res.unwrap(); - // Get the next message - conn.for_each(|frame| { - println!("RX: {:?}", frame); - Ok(()) - }) + let stream = h2.request(request, true).unwrap(); + + let stream = stream.and_then(|response| { + let (_, body) = response.into_parts(); + + body.for_each(|chunk| { + println!("RX: {:?}", chunk); + Ok(()) + }) + }); + + h2.join(stream) }) }); diff --git a/examples/client.rs b/examples/client.rs index 7e79c5178..b371ab803 100644 --- a/examples/client.rs +++ b/examples/client.rs @@ -1,3 +1,4 @@ +/* extern crate h2; extern crate http; extern crate futures; @@ -59,3 +60,6 @@ pub fn main() { core.run(tcp).unwrap(); } +*/ + +pub fn main() {} diff --git a/examples/server.rs b/examples/server.rs index d82659580..7599aba08 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -1,3 +1,4 @@ +/* extern crate h2; extern crate http; extern crate futures; @@ -72,3 +73,6 @@ pub fn main() { core.run(server).unwrap(); } +*/ + +pub fn main() {} diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 81c0dee92..ab13fd65e 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -47,11 +47,11 @@ pub struct PushPromise { promised_id: StreamId, /// The associated flags - flags: HeadersFlag, + flags: PushPromiseFlag, } -impl PushPromise { -} +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub struct PushPromiseFlag(u8); #[derive(Debug)] pub struct Continuation { @@ -302,6 +302,32 @@ impl From for Frame { } } +// ===== impl PushPromise ===== + +impl PushPromise { + pub fn load(head: Head, payload: &[u8]) + -> Result + { + let flags = PushPromiseFlag(head.flag()); + + // TODO: Handle padding + + let promised_id = StreamId::parse(&payload[..4]); + + Ok(PushPromise { + stream_id: head.stream_id(), + promised_id: promised_id, + flags: flags, + }) + } +} + +impl From for Frame { + fn from(src: PushPromise) -> Self { + Frame::PushPromise(src) + } +} + // ===== impl Pseudo ===== impl Pseudo { diff --git a/src/hpack/test/fuzz.rs b/src/hpack/test/fuzz.rs index 80adbec7f..87515305f 100644 --- a/src/hpack/test/fuzz.rs +++ b/src/hpack/test/fuzz.rs @@ -251,7 +251,6 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { header::ACCEPT_CHARSET, header::ACCEPT_ENCODING, header::ACCEPT_LANGUAGE, - header::ACCEPT_PATCH, header::ACCEPT_RANGES, header::ACCESS_CONTROL_ALLOW_CREDENTIALS, header::ACCESS_CONTROL_ALLOW_HEADERS, @@ -272,7 +271,6 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { header::CONTENT_LANGUAGE, header::CONTENT_LENGTH, header::CONTENT_LOCATION, - header::CONTENT_MD5, header::CONTENT_RANGE, header::CONTENT_SECURITY_POLICY, header::CONTENT_SECURITY_POLICY_REPORT_ONLY, @@ -292,7 +290,6 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { header::IF_RANGE, header::IF_UNMODIFIED_SINCE, header::LAST_MODIFIED, - header::KEEP_ALIVE, header::LINK, header::LOCATION, header::MAX_FORWARDS, @@ -311,10 +308,8 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { header::SET_COOKIE, header::STRICT_TRANSPORT_SECURITY, header::TE, - header::TK, header::TRAILER, header::TRANSFER_ENCODING, - header::TSV, header::USER_AGENT, header::UPGRADE, header::UPGRADE_INSECURE_REQUESTS, diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 271b7183a..785171650 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -155,7 +155,7 @@ impl Connection */ } Some(PushPromise(frame)) => { - unimplemented!(); + // TODO: implement /* trace!("recv PUSH_PROMISE; frame={:?}", frame); try!(self.streams.recv_push_promise(frame)); diff --git a/src/proto/framed_read.rs b/src/proto/framed_read.rs index 3b3033319..412b1541f 100644 --- a/src/proto/framed_read.rs +++ b/src/proto/framed_read.rs @@ -92,7 +92,9 @@ impl FramedRead { let _todo = try!(frame::GoAway::load(&bytes[frame::HEADER_LEN..])); unimplemented!(); } - Kind::PushPromise | + Kind::PushPromise => { + frame::PushPromise::load(head, &bytes[frame::HEADER_LEN..])?.into() + } Kind::Priority | Kind::Continuation | Kind::Unknown => { diff --git a/tests/ping_pong.rs b/tests/ping_pong.rs index 4ef927624..e548797e9 100644 --- a/tests/ping_pong.rs +++ b/tests/ping_pong.rs @@ -31,10 +31,10 @@ fn recv_single_ping() { */ .build(); + /* let h2 = client::handshake(mock) .wait().unwrap(); - /* // Send the request let mut request = request::Head::default(); request.method = method::POST; diff --git a/tests/prioritization.rs b/tests/prioritization.rs index 6bb30d709..a1312181d 100644 --- a/tests/prioritization.rs +++ b/tests/prioritization.rs @@ -29,6 +29,7 @@ fn single_stream_send_large_body() { ]) .build(); + /* let h2 = client::handshake(mock) .wait().unwrap(); @@ -65,4 +66,5 @@ fn single_stream_send_large_body() { } assert!(Stream::wait(h2).next().is_none());; + */ } diff --git a/tests/server_preface.rs b/tests/server_preface.rs index 71f3ef9f7..a1a15d35e 100644 --- a/tests/server_preface.rs +++ b/tests/server_preface.rs @@ -4,7 +4,7 @@ extern crate futures; extern crate mock_io; extern crate env_logger; -use h2::server; +// use h2::server; use futures::*; @@ -13,6 +13,7 @@ const SETTINGS_ACK: &'static [u8] = &[0, 0, 0, 4, 1, 0, 0, 0, 0]; #[test] fn read_preface_in_multiple_frames() { + /* let _ = ::env_logger::init().unwrap(); let mock = mock_io::Builder::new() @@ -28,4 +29,5 @@ fn read_preface_in_multiple_frames() { .wait().unwrap(); assert!(Stream::wait(h2).next().is_none()); + */ } From 314b7a1848840ed8eee5c31bdf2b9c53a2c0b1bc Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 8 Aug 2017 13:32:36 -0700 Subject: [PATCH 17/19] Wire in PushPromise --- src/frame/headers.rs | 8 +++ src/proto/connection.rs | 5 +- src/proto/streams/prioritize.rs | 50 +++----------- src/proto/streams/recv.rs | 113 ++++++++++++++++++++++++++++---- src/proto/streams/send.rs | 4 +- src/proto/streams/state.rs | 34 +++++++++- src/proto/streams/store.rs | 93 +++++++++++++++++++++++++- src/proto/streams/stream.rs | 13 +++- src/proto/streams/streams.rs | 51 +++++--------- 9 files changed, 271 insertions(+), 100 deletions(-) diff --git a/src/frame/headers.rs b/src/frame/headers.rs index ab13fd65e..a181e18ba 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -320,6 +320,14 @@ impl PushPromise { flags: flags, }) } + + pub fn stream_id(&self) -> StreamId { + self.stream_id + } + + pub fn promised_id(&self) -> StreamId { + self.promised_id + } } impl From for Frame { diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 785171650..13d13128b 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -155,11 +155,8 @@ impl Connection */ } Some(PushPromise(frame)) => { - // TODO: implement - /* trace!("recv PUSH_PROMISE; frame={:?}", frame); - try!(self.streams.recv_push_promise(frame)); - */ + self.streams.recv_push_promise(frame)?; } Some(Settings(frame)) => { trace!("recv SETTINGS; frame={:?}", frame); diff --git a/src/proto/streams/prioritize.rs b/src/proto/streams/prioritize.rs index d88548d1e..667bbf339 100644 --- a/src/proto/streams/prioritize.rs +++ b/src/proto/streams/prioritize.rs @@ -2,24 +2,18 @@ use super::*; #[derive(Debug)] pub(super) struct Prioritize { - pending_send: Option, + pending_send: store::List, /// Holds frames that are waiting to be written to the socket buffer: Buffer, } -#[derive(Debug, Clone, Copy)] -struct Indices { - head: store::Key, - tail: store::Key, -} - impl Prioritize where B: Buf, { pub fn new() -> Prioritize { Prioritize { - pending_send: None, + pending_send: store::List::new(), buffer: Buffer::new(), } } @@ -32,7 +26,7 @@ impl Prioritize stream.pending_send.push_back(&mut self.buffer, frame); if stream.is_pending_send { - debug_assert!(self.pending_send.is_some()); + debug_assert!(!self.pending_send.is_empty()); // Already queued to have frame processed. return; @@ -84,44 +78,20 @@ impl Prioritize } fn push_sender(&mut self, stream: &mut store::Ptr) { - // The next pointer shouldn't be set - debug_assert!(stream.next_pending_send.is_none()); + debug_assert!(!stream.is_pending_send); - // Queue the stream - match self.pending_send { - Some(ref mut idxs) => { - // Update the current tail node to point to `stream` - stream.resolve(idxs.tail).next_pending_send = Some(stream.key()); - - // Update the tail pointer - idxs.tail = stream.key(); - } - None => { - self.pending_send = Some(Indices { - head: stream.key(), - tail: stream.key(), - }); - } - } + self.pending_send.push(stream); stream.is_pending_send = true; } fn pop_sender<'a>(&mut self, store: &'a mut Store) -> Option> { - if let Some(mut idxs) = self.pending_send { - let mut stream = store.resolve(idxs.head); - - if idxs.head == idxs.tail { - assert!(stream.next_pending_send.is_none()); - self.pending_send = None; - } else { - idxs.head = stream.next_pending_send.take().unwrap(); - self.pending_send = Some(idxs); + match self.pending_send.pop(store) { + Some(mut stream) => { + stream.is_pending_send = false; + Some(stream) } - - return Some(stream); + None => None, } - - None } } diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 8f319c004..318dbc60a 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -21,6 +21,8 @@ pub(super) struct Recv { /// Connection level flow control governing received data flow_control: FlowControl, + /// Streams that have pending window updates + /// TODO: don't use a VecDeque pending_window_updates: VecDeque, /// Holds frames that are waiting to be read @@ -38,6 +40,12 @@ pub(super) struct Chunk { pub pending_recv: buffer::Deque, } +#[derive(Debug, Clone, Copy)] +struct Indices { + head: store::Key, + tail: store::Key, +} + impl Recv where P: Peer, B: Buf, @@ -63,16 +71,11 @@ impl Recv try!(self.ensure_can_open(id)); - if let Some(max) = self.max_streams { - if max <= self.num_streams { - self.refused = Some(id); - return Ok(None); - } + if !self.can_inc_num_streams() { + self.refused = Some(id); + return Ok(None); } - // Increment the number of remote initiated streams - self.num_streams += 1; - Ok(Some(Stream::new(id))) } @@ -82,7 +85,16 @@ impl Recv stream: &mut store::Ptr) -> Result, ConnectionError> { - stream.state.recv_open(self.init_window_sz, frame.is_end_stream())?; + let is_initial = stream.state.recv_open(self.init_window_sz, frame.is_end_stream())?; + + if is_initial { + if !self.can_inc_num_streams() { + unimplemented!(); + } + + // Increment the number of concurrent streams + self.inc_num_streams(); + } // Only servers can receive a headers frame that initiates the stream. // This is verified in `Streams` before calling this function. @@ -105,7 +117,7 @@ impl Recv pub fn recv_data(&mut self, frame: frame::Data, - stream: &mut Stream) + stream: &mut store::Ptr) -> Result<(), ConnectionError> { let sz = frame.payload().len(); @@ -143,6 +155,48 @@ impl Recv Ok(()) } + pub fn recv_push_promise(&mut self, frame: frame::PushPromise, stream: &mut store::Ptr) + -> Result<(), ConnectionError> + { + // First, make sure that the values are legit + self.ensure_can_reserve(frame.promised_id())?; + + // Make sure that the stream state is valid + stream.state.ensure_recv_open()?; + + // TODO: Streams in the reserved states do not count towards the concurrency + // limit. However, it seems like there should be a cap otherwise this + // could grow in memory indefinitely. + + /* + if !self.inc_num_streams() { + self.refused = Some(frame.promised_id()); + return Ok(()); + } + */ + + // TODO: All earlier stream IDs should be implicitly closed. + + // Now, create a new entry for the stream + let mut new_stream = Stream::new(frame.promised_id()); + new_stream.state.reserve_remote(); + + let mut ppp = stream.pending_push_promises.take(); + + { + // Store the stream + let mut new_stream = stream.store() + .insert(frame.promised_id(), new_stream); + + ppp.push(&mut new_stream); + } + + stream.pending_push_promises = ppp; + stream.notify_recv(); + + Ok(()) + } + pub fn recv_err(&mut self, err: &ConnectionError, stream: &mut Stream) { // Receive an error stream.state.recv_err(err); @@ -151,6 +205,26 @@ impl Recv stream.notify_recv(); } + /// Returns true if the current stream concurrency can be incremetned + fn can_inc_num_streams(&self) -> bool { + if let Some(max) = self.max_streams { + max > self.num_streams + } else { + true + } + } + + /// Increments the number of concurrenty streams. Panics on failure as this + /// should have been validated before hand. + fn inc_num_streams(&mut self) { + if !self.can_inc_num_streams() { + panic!(); + } + + // Increment the number of remote initiated streams + self.num_streams += 1; + } + pub fn dec_num_streams(&mut self) { self.num_streams -= 1; } @@ -171,6 +245,21 @@ impl Recv Ok(()) } + /// Returns true if the remote peer can reserve a stream with the given ID. + fn ensure_can_reserve(&self, promised_id: StreamId) -> Result<(), ConnectionError> { + // TODO: Are there other rules? + if P::is_server() { + // The remote is a client and cannot reserve + return Err(ProtocolError.into()); + } + + if !promised_id.is_server_initiated() { + return Err(ProtocolError.into()); + } + + Ok(()) + } + /// Send any pending refusals. pub fn send_pending_refusal(&mut self, dst: &mut Codec) -> Poll<(), ConnectionError> @@ -206,7 +295,7 @@ impl Recv pub fn expand_stream_window(&mut self, id: StreamId, sz: WindowSize, - stream: &mut Stream) + stream: &mut store::Ptr) -> Result<(), ConnectionError> { // TODO: handle overflow @@ -276,7 +365,7 @@ impl Recv { while let Some(id) = self.pending_window_updates.pop_front() { let flow = streams.find_mut(&id) - .and_then(|stream| stream.recv_flow_control()); + .and_then(|stream| stream.into_mut().recv_flow_control()); if let Some(flow) = flow { diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 8650a31f4..de7bc6c7b 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -177,7 +177,7 @@ impl Send let update = self.pending_window_updates.pop_front() .and_then(|id| { streams.find_mut(&id) - .and_then(|stream| stream.send_flow_control()) + .and_then(|stream| stream.into_mut().send_flow_control()) .and_then(|flow| flow.apply_window_update()) .map(|incr| WindowUpdate::new(id, incr)) }); @@ -209,7 +209,7 @@ impl Send pub fn recv_stream_window_update(&mut self, frame: frame::WindowUpdate, - stream: &mut Stream) + stream: &mut store::Ptr) -> Result<(), ConnectionError> { if let Some(flow) = stream.send_flow_control() { diff --git a/src/proto/streams/state.rs b/src/proto/streams/state.rs index b39bd14bd..3c0d9035d 100644 --- a/src/proto/streams/state.rs +++ b/src/proto/streams/state.rs @@ -58,7 +58,7 @@ enum Inner { Idle, // TODO: these states shouldn't count against concurrency limits: //ReservedLocal, - //ReservedRemote, + ReservedRemote, Open { local: Peer, remote: Peer, @@ -126,11 +126,16 @@ impl State { /// Open the receive have of the stream, this action is taken when a HEADERS /// frame is received. - pub fn recv_open(&mut self, sz: WindowSize, eos: bool) -> Result<(), ConnectionError> { + /// + /// Returns true if this transitions the state to Open + pub fn recv_open(&mut self, sz: WindowSize, eos: bool) -> Result { let remote = Peer::streaming(sz); + let mut initial = false; self.inner = match self.inner { Idle => { + initial = true; + if eos { HalfClosedRemote(AwaitingHeaders) } else { @@ -140,6 +145,18 @@ impl State { } } } + ReservedRemote => { + initial = true; + + if eos { + Closed(None) + } else { + Open { + local: AwaitingHeaders, + remote, + } + } + } Open { local, remote: AwaitingHeaders } => { if eos { HalfClosedRemote(local) @@ -163,7 +180,18 @@ impl State { } }; - return Ok(()); + return Ok(initial); + } + + /// Transition from Idle -> ReservedRemote + pub fn reserve_remote(&mut self) -> Result<(), ConnectionError> { + match self.inner { + Idle => { + self.inner = ReservedRemote; + Ok(()) + } + _ => Err(ProtocolError.into()), + } } /// Indicates that the remote side will not send more data to the local. diff --git a/src/proto/streams/store.rs b/src/proto/streams/store.rs index 6821ff811..89900b83f 100644 --- a/src/proto/streams/store.rs +++ b/src/proto/streams/store.rs @@ -4,6 +4,7 @@ use slab; use std::ops; use std::collections::{HashMap, hash_map}; +use std::marker::PhantomData; /// Storage for streams #[derive(Debug)] @@ -22,6 +23,19 @@ pub(super) struct Ptr<'a, B: 'a> { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) struct Key(usize); +#[derive(Debug)] +pub(super) struct List { + indices: Option, + _p: PhantomData, +} + +/// A linked list +#[derive(Debug, Clone, Copy)] +struct Indices { + pub head: Key, + pub tail: Key, +} + pub(super) enum Entry<'a, B: 'a> { Occupied(OccupiedEntry<'a, B>), Vacant(VacantEntry<'a, B>), @@ -54,9 +68,12 @@ impl Store { } } - pub fn find_mut(&mut self, id: &StreamId) -> Option<&mut Stream> { - if let Some(handle) = self.ids.get(id) { - Some(&mut self.slab[*handle]) + pub fn find_mut(&mut self, id: &StreamId) -> Option> { + if let Some(&key) = self.ids.get(id) { + Some(Ptr { + key: Key(key), + store: self, + }) } else { None } @@ -100,6 +117,68 @@ impl Store { } } +// ===== impl List ===== + +impl List { + pub fn new() -> Self { + List { + indices: None, + _p: PhantomData, + } + } + + pub fn is_empty(&self) -> bool { + self.indices.is_none() + } + + pub fn take(&mut self) -> Self { + List { + indices: self.indices.take(), + _p: PhantomData, + } + } + + pub fn push(&mut self, stream: &mut store::Ptr) { + // The next pointer shouldn't be set + debug_assert!(stream.next.is_none()); + + // Queue the stream + match self.indices { + Some(ref mut idxs) => { + // Update the current tail node to point to `stream` + stream.resolve(idxs.tail).next = Some(stream.key()); + + // Update the tail pointer + idxs.tail = stream.key(); + } + None => { + self.indices = Some(store::Indices { + head: stream.key(), + tail: stream.key(), + }); + } + } + } + + pub fn pop<'a>(&mut self, store: &'a mut Store) -> Option> { + if let Some(mut idxs) = self.indices { + let mut stream = store.resolve(idxs.head); + + if idxs.head == idxs.tail { + assert!(stream.next.is_none()); + self.indices = None; + } else { + idxs.head = stream.next.take().unwrap(); + self.indices = Some(idxs); + } + + return Some(stream); + } + + None + } +} + // ===== impl Ptr ===== impl<'a, B: 'a> Ptr<'a, B> { @@ -107,12 +186,20 @@ impl<'a, B: 'a> Ptr<'a, B> { self.key } + pub fn store(&mut self) -> &mut Store { + &mut self.store + } + pub fn resolve(&mut self, key: Key) -> Ptr { Ptr { key: key, store: self.store, } } + + pub fn into_mut(self) -> &'a mut Stream { + &mut self.store.slab[self.key.0] + } } impl<'a, B: 'a> ops::Deref for Ptr<'a, B> { diff --git a/src/proto/streams/stream.rs b/src/proto/streams/stream.rs index d26266c4c..7518a483b 100644 --- a/src/proto/streams/stream.rs +++ b/src/proto/streams/stream.rs @@ -17,8 +17,14 @@ pub(super) struct Stream { /// Frames pending for this stream being sent to the socket pub pending_send: buffer::Deque, - /// Next stream pending send - pub next_pending_send: Option, + /// Next node in the `Stream` linked list. + /// + /// This field is used in different linked lists depending on the stream + /// state. + pub next: Option, + + /// The stream's pending push promises + pub pending_push_promises: store::List, /// True if the stream is currently pending send pub is_pending_send: bool, @@ -32,7 +38,8 @@ impl Stream { pending_recv: buffer::Deque::new(), recv_task: None, pending_send: buffer::Deque::new(), - next_pending_send: None, + next: None, + pending_push_promises: store::List::new(), is_pending_send: false, } } diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index edfd05855..57954a9e3 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -119,14 +119,14 @@ impl Streams let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let stream = match me.store.find_mut(&id) { + let mut stream = match me.store.find_mut(&id) { Some(stream) => stream, None => return Err(ProtocolError.into()), }; // Ensure there's enough capacity on the connection before acting on the // stream. - try!(me.actions.recv.recv_data(frame, stream)); + try!(me.actions.recv.recv_data(frame, &mut stream)); if stream.state.is_closed() { me.actions.dec_num_streams(id); @@ -160,18 +160,28 @@ impl Streams } else { // The remote may send window updates for streams that the local now // considers closed. It's ok... - if let Some(state) = me.store.find_mut(&id) { - try!(me.actions.send.recv_stream_window_update(frame, state)); + if let Some(mut stream) = me.store.find_mut(&id) { + try!(me.actions.send.recv_stream_window_update(frame, &mut stream)); } } Ok(()) } - pub fn recv_push_promise(&mut self, _frame: frame::PushPromise) + pub fn recv_push_promise(&mut self, frame: frame::PushPromise) -> Result<(), ConnectionError> { - unimplemented!(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let id = frame.stream_id(); + + let mut stream = match me.store.find_mut(&id) { + Some(stream) => stream, + None => return Err(ProtocolError.into()), + }; + + me.actions.recv.recv_push_promise(frame, &mut stream) } pub fn send_headers(&mut self, headers: frame::Headers) @@ -208,31 +218,6 @@ impl Streams */ } - /* - pub fn send_data(&mut self, frame: &frame::Data) - -> Result<(), ConnectionError> - { - let id = frame.stream_id(); - let mut me = self.inner.lock().unwrap(); - let me = &mut *me; - - let stream = match me.store.find_mut(&id) { - Some(stream) => stream, - None => return Err(UnexpectedFrameType.into()), - }; - - // Ensure there's enough capacity on the connection before acting on the - // stream. - try!(me.actions.send.send_data(frame, stream)); - - if stream.state.is_closed() { - me.actions.dec_num_streams(id); - } - - Ok(()) - } - */ - pub fn poll_window_update(&mut self) -> Poll { @@ -250,8 +235,8 @@ impl Streams if id.is_zero() { try!(me.actions.recv.expand_connection_window(sz)); } else { - if let Some(state) = me.store.find_mut(&id) { - try!(me.actions.recv.expand_stream_window(id, sz, state)); + if let Some(mut stream) = me.store.find_mut(&id) { + try!(me.actions.recv.expand_stream_window(id, sz, &mut stream)); } } From 8485bf91e771c2d94772e4830a3bbeceb4565982 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 8 Aug 2017 13:48:29 -0700 Subject: [PATCH 18/19] Start hooking up reset --- src/proto/connection.rs | 12 +----------- src/proto/streams/recv.rs | 11 +++++++++++ src/proto/streams/streams.rs | 23 ++++++++++++++++++++--- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/proto/connection.rs b/src/proto/connection.rs index 13d13128b..7ae9139d5 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -141,18 +141,8 @@ impl Connection try!(self.streams.recv_data(frame)); } Some(Reset(frame)) => { - unimplemented!(); - /* trace!("recv RST_STREAM; frame={:?}", frame); - try!(self.streams.recv_reset(&frame)); - - let frame = Frame::Reset { - id: frame.stream_id(), - error: frame.reason(), - }; - - return Ok(Some(frame).into()); - */ + try!(self.streams.recv_reset(frame)); } Some(PushPromise(frame)) => { trace!("recv PUSH_PROMISE; frame={:?}", frame); diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index 318dbc60a..cfb813f70 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -197,6 +197,17 @@ impl Recv Ok(()) } + pub fn recv_reset(&mut self, frame: frame::Reset, stream: &mut Stream) + -> Result<(), ConnectionError> + { + let err = ConnectionError::Proto(frame.reason()); + + // Notify the stream + stream.state.recv_err(&err); + stream.notify_recv(); + Ok(()) + } + pub fn recv_err(&mut self, err: &ConnectionError, stream: &mut Stream) { // Receive an error stream.state.recv_err(err); diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 57954a9e3..bbfee6d98 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -115,10 +115,11 @@ impl Streams pub fn recv_data(&mut self, frame: frame::Data) -> Result<(), ConnectionError> { - let id = frame.stream_id(); let mut me = self.inner.lock().unwrap(); let me = &mut *me; + let id = frame.stream_id(); + let mut stream = match me.store.find_mut(&id) { Some(stream) => stream, None => return Err(ProtocolError.into()), @@ -135,10 +136,26 @@ impl Streams Ok(()) } - pub fn recv_reset(&mut self, _frame: &frame::Reset) + pub fn recv_reset(&mut self, frame: frame::Reset) -> Result<(), ConnectionError> { - unimplemented!(); + let mut me = self.inner.lock().unwrap(); + let me = &mut *me; + + let id = frame.stream_id(); + + let mut stream = match me.store.find_mut(&id) { + Some(stream) => stream, + // TODO: should this be an error? + None => return Ok(()), + }; + + me.actions.recv.recv_reset(frame, &mut stream)?; + + assert!(stream.state.is_closed()); + me.actions.dec_num_streams(id); + + Ok(()) } pub fn recv_err(&mut self, err: &ConnectionError) { From 8a5e0c3046f0d759155ab0d4e681635b0f0eacfe Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 8 Aug 2017 14:26:38 -0700 Subject: [PATCH 19/19] More tweaks --- src/proto/streams/state.rs | 11 +++++ src/proto/streams/streams.rs | 86 ++++++++++++++++++------------------ 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/proto/streams/state.rs b/src/proto/streams/state.rs index 3c0d9035d..7b3bdc37d 100644 --- a/src/proto/streams/state.rs +++ b/src/proto/streams/state.rs @@ -243,6 +243,17 @@ impl State { } } + /// Returns true if a stream with the current state counts against the + /// concurrency limit. + pub fn is_counted(&self) -> bool { + match self.inner { + Open { .. } => true, + HalfClosedLocal(..) => true, + HalfClosedRemote(..) => true, + _ => false, + } + } + pub fn is_closed(&self) -> bool { match self.inner { Closed(_) => true, diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index bbfee6d98..fd3a47c9f 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -88,28 +88,23 @@ impl Streams } }; - let mut stream = me.store.resolve(key); + let stream = me.store.resolve(key); - let ret = if frame.is_trailers() { - unimplemented!(); - /* - if !frame.is_end_stream() { - // TODO: What error should this return? + me.actions.transition(stream, |actions, stream| { + if frame.is_trailers() { unimplemented!(); - } - - try!(me.actions.recv.recv_eos(stream)); - */ - } else { - try!(me.actions.recv.recv_headers(frame, &mut stream)) - }; - - // TODO: move this into a fn - if stream.state.is_closed() { - me.actions.dec_num_streams(id); - } + /* + if !frame.is_end_stream() { + // TODO: What error should this return? + unimplemented!(); + } - Ok(ret) + try!(me.actions.recv.recv_eos(stream)); + */ + } else { + actions.recv.recv_headers(frame, stream) + } + }) } pub fn recv_data(&mut self, frame: frame::Data) @@ -120,20 +115,14 @@ impl Streams let id = frame.stream_id(); - let mut stream = match me.store.find_mut(&id) { + let stream = match me.store.find_mut(&id) { Some(stream) => stream, None => return Err(ProtocolError.into()), }; - // Ensure there's enough capacity on the connection before acting on the - // stream. - try!(me.actions.recv.recv_data(frame, &mut stream)); - - if stream.state.is_closed() { - me.actions.dec_num_streams(id); - } - - Ok(()) + me.actions.transition(stream, |actions, stream| { + actions.recv.recv_data(frame, stream) + }) } pub fn recv_reset(&mut self, frame: frame::Reset) @@ -150,12 +139,11 @@ impl Streams None => return Ok(()), }; - me.actions.recv.recv_reset(frame, &mut stream)?; - - assert!(stream.state.is_closed()); - me.actions.dec_num_streams(id); - - Ok(()) + me.actions.transition(stream, |actions, stream| { + actions.recv.recv_reset(frame, stream)?; + assert!(stream.state.is_closed()); + Ok(()) + }) } pub fn recv_err(&mut self, err: &ConnectionError) { @@ -338,19 +326,15 @@ impl StreamRef let mut me = self.inner.lock().unwrap(); let me = &mut *me; - let mut stream = me.store.resolve(self.key); + let stream = me.store.resolve(self.key); // Create the data frame let frame = frame::Data::from_buf(stream.id, data, end_of_stream); - // Send the data frame - me.actions.send.send_data(frame, &mut stream)?; - - if stream.state.is_closed() { - me.actions.dec_num_streams(stream.id); - } - - Ok(()) + me.actions.transition(stream, |actions, stream| { + // Send the data frame + actions.send.send_data(frame, stream) + }) } pub fn poll_data(&mut self) -> Poll>, ConnectionError> { @@ -445,4 +429,18 @@ impl Actions assert!(!id.is_zero()); P::is_server() == id.is_server_initiated() } + + fn transition(&mut self, mut stream: store::Ptr, f: F) -> U + where F: FnOnce(&mut Self, &mut store::Ptr) -> U, + { + let is_counted = stream.state.is_counted(); + + let ret = f(self, &mut stream); + + if is_counted && stream.state.is_closed() { + self.dec_num_streams(stream.id); + } + + ret + } }