fix(http): Prevent busy looping
We encountered some issues where the `Conn::ready()` would busy loop on reads. Previously, the `ConnInner::can_read_more()` would not consider whether the previous read got a WouldBlock error, and it didn't consider whether the transport was blocked. Accounting for this additional state fixes the busy loop problem.
This commit is contained in:
		| @@ -37,6 +37,10 @@ struct ConnInner<K: Key, T: Transport, H: MessageHandler<T>> { | |||||||
|     key: K, |     key: K, | ||||||
|     state: State<H, T>, |     state: State<H, T>, | ||||||
|     transport: T, |     transport: T, | ||||||
|  |     /// Records a WouldBlock error when trying to read | ||||||
|  |     /// | ||||||
|  |     /// This flag is used to prevent busy looping | ||||||
|  |     read_would_block: bool, | ||||||
| } | } | ||||||
|  |  | ||||||
| impl<K: Key, T: Transport, H: MessageHandler<T>> fmt::Debug for ConnInner<K, T, H> { | impl<K: Key, T: Transport, H: MessageHandler<T>> fmt::Debug for ConnInner<K, T, H> { | ||||||
| @@ -153,7 +157,10 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> { | |||||||
|                     Ok(head) => head, |                     Ok(head) => head, | ||||||
|                     Err(::Error::Io(e)) => match e.kind() { |                     Err(::Error::Io(e)) => match e.kind() { | ||||||
|                         io::ErrorKind::WouldBlock | |                         io::ErrorKind::WouldBlock | | ||||||
|                         io::ErrorKind::Interrupted => return state, |                         io::ErrorKind::Interrupted => { | ||||||
|  |                             self.read_would_block = true; | ||||||
|  |                             return state; | ||||||
|  |                         }, | ||||||
|                         _ => { |                         _ => { | ||||||
|                             debug!("io error trying to parse {:?}", e); |                             debug!("io error trying to parse {:?}", e); | ||||||
|                             return State::Closed; |                             return State::Closed; | ||||||
| @@ -250,6 +257,7 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> { | |||||||
|                     Err(e) => match e.kind() { |                     Err(e) => match e.kind() { | ||||||
|                         io::ErrorKind::WouldBlock => { |                         io::ErrorKind::WouldBlock => { | ||||||
|                             // This is the expected case reading in this state |                             // This is the expected case reading in this state | ||||||
|  |                             self.read_would_block = true; | ||||||
|                             state |                             state | ||||||
|                         }, |                         }, | ||||||
|                         _ => { |                         _ => { | ||||||
| @@ -288,7 +296,10 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> { | |||||||
|                         }, |                         }, | ||||||
|                         Err(::Error::Io(e)) => match e.kind() { |                         Err(::Error::Io(e)) => match e.kind() { | ||||||
|                             io::ErrorKind::WouldBlock | |                             io::ErrorKind::WouldBlock | | ||||||
|                             io::ErrorKind::Interrupted => None, |                             io::ErrorKind::Interrupted => { | ||||||
|  |                                 self.read_would_block = true; | ||||||
|  |                                 None | ||||||
|  |                             }, | ||||||
|                             _ => { |                             _ => { | ||||||
|                                 debug!("io error trying to parse {:?}", e); |                                 debug!("io error trying to parse {:?}", e); | ||||||
|                                 return State::Closed; |                                 return State::Closed; | ||||||
| @@ -482,10 +493,15 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> { | |||||||
|     } |     } | ||||||
|  |  | ||||||
|     fn can_read_more(&self, was_init: bool) -> bool { |     fn can_read_more(&self, was_init: bool) -> bool { | ||||||
|         match self.state { |         let transport_blocked = self.transport.blocked().is_some(); | ||||||
|  |         let read_would_block = self.read_would_block; | ||||||
|  |  | ||||||
|  |         let state_machine_ok = match self.state { | ||||||
|             State::Init { .. } => !was_init && !self.buf.is_empty(), |             State::Init { .. } => !was_init && !self.buf.is_empty(), | ||||||
|             _ => !self.buf.is_empty() |             _ => !self.buf.is_empty() | ||||||
|         } |         }; | ||||||
|  |  | ||||||
|  |         !transport_blocked && !read_would_block && state_machine_ok | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     fn on_error<F>(&mut self, err: ::Error, factory: &F) where F: MessageHandlerFactory<K, T> { |     fn on_error<F>(&mut self, err: ::Error, factory: &F) where F: MessageHandlerFactory<K, T> { | ||||||
| @@ -501,6 +517,8 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> { | |||||||
|  |  | ||||||
|     fn on_readable<F>(&mut self, scope: &mut Scope<F>) |     fn on_readable<F>(&mut self, scope: &mut Scope<F>) | ||||||
|     where F: MessageHandlerFactory<K, T, Output=H> { |     where F: MessageHandlerFactory<K, T, Output=H> { | ||||||
|  |         // Clear would_block flag so state is clear going into read | ||||||
|  |         self.read_would_block = false; | ||||||
|         trace!("on_readable -> {:?}", self.state); |         trace!("on_readable -> {:?}", self.state); | ||||||
|         let state = mem::replace(&mut self.state, State::Closed); |         let state = mem::replace(&mut self.state, State::Closed); | ||||||
|         self.state = self.read(scope, state); |         self.state = self.read(scope, state); | ||||||
| @@ -549,6 +567,7 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> Conn<K, T, H> { | |||||||
|                 timeout_start: Some(now), |                 timeout_start: Some(now), | ||||||
|             }, |             }, | ||||||
|             transport: transport, |             transport: transport, | ||||||
|  |             read_would_block: false, | ||||||
|         })) |         })) | ||||||
|     } |     } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user