Skip to content

Commit f4b5130

Browse files
seanmonstarRobin Seitz
andauthored
fix(http1): http1 server graceful shutdown fix (#3261)
fix issue in the graceful shutdown logic which causes the connection future to hang when graceful shutdown is called prior to any requests being made. This fix checks to see if the connection is still in its initial state when disable_keep_alive is called, and starts the shutdown process if it is. This addresses issue #2730 Co-authored-by: Robin Seitz <roseitz@microsoft.com>
1 parent d92d391 commit f4b5130

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

src/proto/h1/conn.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,13 @@ where
175175
}
176176
}
177177

178+
#[cfg(feature = "server")]
179+
pub(crate) fn has_initial_read_write_state(&self) -> bool {
180+
matches!(self.state.reading, Reading::Init)
181+
&& matches!(self.state.writing, Writing::Init)
182+
&& self.io.read_buf().is_empty()
183+
}
184+
178185
fn should_error_on_eof(&self) -> bool {
179186
// If we're idle, it's probably just the connection closing gracefully.
180187
T::should_error_on_parse_eof() && !self.state.is_idle()

src/proto/h1/dispatch.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ where
8282
#[cfg(feature = "server")]
8383
pub(crate) fn disable_keep_alive(&mut self) {
8484
self.conn.disable_keep_alive();
85-
if self.conn.is_write_closed() {
85+
86+
// If keep alive has been disabled and no read or write has been seen on
87+
// the connection yet, we must be in a state where the server is being asked to
88+
// shut down before any data has been seen on the connection
89+
if self.conn.is_write_closed() || self.conn.has_initial_read_write_state() {
8690
self.close();
8791
}
8892
}

tests/server.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use hyper::body::{Body, Incoming as IncomingBody};
3131
use hyper::server::conn::{http1, http2};
3232
use hyper::service::{service_fn, Service};
3333
use hyper::{Method, Request, Response, StatusCode, Uri, Version};
34+
use tokio::pin;
3435

3536
mod support;
3637

@@ -1139,11 +1140,17 @@ async fn disable_keep_alive_mid_request() {
11391140
let child = thread::spawn(move || {
11401141
let mut req = connect(&addr);
11411142
req.write_all(b"GET / HTTP/1.1\r\n").unwrap();
1143+
thread::sleep(Duration::from_millis(10));
11421144
tx1.send(()).unwrap();
11431145
rx2.recv().unwrap();
11441146
req.write_all(b"Host: localhost\r\n\r\n").unwrap();
11451147
let mut buf = vec![];
11461148
req.read_to_end(&mut buf).unwrap();
1149+
assert!(
1150+
buf.starts_with(b"HTTP/1.1 200 OK\r\n"),
1151+
"should receive OK response, but buf: {:?}",
1152+
buf,
1153+
);
11471154
});
11481155

11491156
let (socket, _) = listener.accept().await.unwrap();
@@ -2152,6 +2159,31 @@ async fn max_buf_size() {
21522159
.expect_err("should TooLarge error");
21532160
}
21542161

2162+
#[cfg(feature = "http1")]
2163+
#[tokio::test]
2164+
async fn graceful_shutdown_before_first_request_no_block() {
2165+
let (listener, addr) = setup_tcp_listener();
2166+
2167+
tokio::spawn(async move {
2168+
let socket = listener.accept().await.unwrap().0;
2169+
2170+
let future = http1::Builder::new().serve_connection(socket, HelloWorld);
2171+
pin!(future);
2172+
future.as_mut().graceful_shutdown();
2173+
2174+
future.await.unwrap();
2175+
});
2176+
2177+
let mut stream = TkTcpStream::connect(addr).await.unwrap();
2178+
2179+
let mut buf = vec![];
2180+
2181+
tokio::time::timeout(Duration::from_secs(5), stream.read_to_end(&mut buf))
2182+
.await
2183+
.expect("timed out waiting for graceful shutdown")
2184+
.expect("error receiving response");
2185+
}
2186+
21552187
#[test]
21562188
fn streaming_body() {
21572189
use futures_util::StreamExt;

0 commit comments

Comments
 (0)