From 32e09a04292b0247456a8fb9003a75a6abaa998e Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 2 Sep 2015 09:26:46 -0700 Subject: [PATCH] fix(client): EofReader by nature means the connection is closed --- src/client/pool.rs | 23 ++++++++++++++++++++++- src/client/response.rs | 11 +++-------- src/http/h1.rs | 1 + 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/client/pool.rs b/src/client/pool.rs index 1012f23d..71800950 100644 --- a/src/client/pool.rs +++ b/src/client/pool.rs @@ -145,7 +145,16 @@ struct PooledStreamInner { impl Read for PooledStream { #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.inner.as_mut().unwrap().stream.read(buf) + match self.inner.as_mut().unwrap().stream.read(buf) { + Ok(0) => { + // if the wrapped stream returns EOF (Ok(0)), that means the + // server has closed the stream. we must be sure this stream + // is dropped and not put back into the pool. + self.is_closed = true; + Ok(0) + }, + r => r + } } } @@ -216,6 +225,7 @@ impl Drop for PooledStream { #[cfg(test)] mod tests { use std::net::Shutdown; + use std::io::Read; use mock::{MockConnector}; use net::{NetworkConnector, NetworkStream}; @@ -254,4 +264,15 @@ mod tests { let locked = pool.inner.lock().unwrap(); assert_eq!(locked.conns.len(), 0); } + + #[test] + fn test_eof_closes() { + let pool = mocked!(); + + let mut stream = pool.connect("127.0.0.1", 3000, "http").unwrap(); + assert_eq!(stream.read(&mut [0]).unwrap(), 0); + drop(stream); + let locked = pool.inner.lock().unwrap(); + assert_eq!(locked.conns.len(), 0); + } } diff --git a/src/client/response.rs b/src/client/response.rs index 89376e4a..05bdf4cd 100644 --- a/src/client/response.rs +++ b/src/client/response.rs @@ -23,7 +23,6 @@ pub struct Response { pub url: Url, status_raw: RawStatus, message: Box, - is_drained: bool, } impl Response { @@ -54,7 +53,6 @@ impl Response { headers: headers, url: url, status_raw: raw_status, - is_drained: !message.has_body(), message: message, }) } @@ -70,10 +68,6 @@ impl Read for Response { #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result { match self.message.read(buf) { - Ok(0) => { - self.is_drained = true; - Ok(0) - }, Err(e) => { let _ = self.message.close_connection(); Err(e) @@ -90,8 +84,9 @@ impl Drop for Response { // // otherwise, the response has been drained. we should check that the // server has agreed to keep the connection open - trace!("Response.drop is_drained={}", self.is_drained); - if !(self.is_drained && http::should_keep_alive(self.version, &self.headers)) { + let is_drained = !self.message.has_body(); + trace!("Response.drop is_drained={}", is_drained); + if !(is_drained && http::should_keep_alive(self.version, &self.headers)) { trace!("Response.drop closing connection"); if let Err(e) = self.message.close_connection() { error!("Response.drop error closing connection: {}", e); diff --git a/src/http/h1.rs b/src/http/h1.rs index 92103748..14f25b78 100644 --- a/src/http/h1.rs +++ b/src/http/h1.rs @@ -247,6 +247,7 @@ impl HttpMessage for Http11Message { Some(EmptyReader(..)) | Some(SizedReader(_, 0)) | Some(ChunkedReader(_, Some(0))) => false, + // specifically EofReader is always true _ => true } }