From 20fac49aa91307b6c60b43b6f3b4657e4621c93c Mon Sep 17 00:00:00 2001
From: Joe Wilm <joe@jwilm.com>
Date: Thu, 13 Oct 2016 10:02:59 -0700
Subject: [PATCH] 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.
---
 src/http/conn.rs | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/http/conn.rs b/src/http/conn.rs
index c7b57581..05792d5c 100644
--- a/src/http/conn.rs
+++ b/src/http/conn.rs
@@ -37,6 +37,10 @@ struct ConnInner<K: Key, T: Transport, H: MessageHandler<T>> {
     key: K,
     state: State<H, 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> {
@@ -153,7 +157,10 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> {
                     Ok(head) => head,
                     Err(::Error::Io(e)) => match e.kind() {
                         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);
                             return State::Closed;
@@ -250,6 +257,7 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> {
                     Err(e) => match e.kind() {
                         io::ErrorKind::WouldBlock => {
                             // This is the expected case reading in this state
+                            self.read_would_block = true;
                             state
                         },
                         _ => {
@@ -288,7 +296,10 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> {
                         },
                         Err(::Error::Io(e)) => match e.kind() {
                             io::ErrorKind::WouldBlock |
-                            io::ErrorKind::Interrupted => None,
+                            io::ErrorKind::Interrupted => {
+                                self.read_would_block = true;
+                                None
+                            },
                             _ => {
                                 debug!("io error trying to parse {:?}", e);
                                 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 {
-        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(),
             _ => !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> {
@@ -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>)
     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);
         let state = mem::replace(&mut self.state, State::Closed);
         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),
             },
             transport: transport,
+            read_would_block: false,
         }))
     }