HTTP/1 client connection is never closed or pooled when a response completes while the request body is still unsent ((Reading::KeepAlive, Writing::Body) is a terminal state)

[BlackRabbitCoder]

Version

hyper 1.11.1

Platform

Darwin 6c7e67e8e325 25.6.0 Darwin Kernel Version 25.6.0: Fri Jul 31 19:18:48 PDT 2026; root:xnu-12377.161.14~5/RELEASE_ARM64_T6020 arm64

Summary

On an HTTP/1 client connection, if the server sends a complete response while the client has not finished writing the request body, the connection lands in (Reading::KeepAlive, Writing::Body) and gets permanently stuck there:

• the response is delivered to the caller successfully (the request "succeeds"),
• but the connection is never closed (no FIN, no RST — it stays ESTABLISHED),
• and it is never returned to the pool (with hyper_util's legacy::Client, SendRequest::poll_ready never resolves again).

The connection leaks silently until the peer eventually closes it or the process exits. With a pooled client this manifests as a slow socket/fd leak: each affected request permanently consumes a connection while the caller believes it completed normally.

This is reachable in practice whenever a server responds before draining the request body (e.g. a fast 4xx/413) and the request body is large enough to fill the socket buffers, so the client's write stalls in Writing::Body.

Root cause

For a client (!T::should_read_first()), Dispatcher::is_done() (src/proto/h1/dispatch.rs) can only return true when read_done is set:

let read_done = self.conn.is_read_closed();
if !T::should_read_first() && read_done { true }
else { let write_done = ...; read_done && write_done }

But is_read_closed() is matches!(self.state.reading, Reading::Closed) (src/proto/h1/conn.rs), and a fully-read response leaves reading in Reading::KeepAlive, not Reading::Closed. So in (Reading::KeepAlive, Writing::Body):

• is_done() is false → poll_inner returns Poll::Pending → conn.poll_shutdown() is never reached → no FIN/RST.
• State::try_keep_alive() has arms only for (KeepAlive, KeepAlive), (Closed, KeepAlive), (KeepAlive, Closed) — nothing for (KeepAlive, Body), so it can't idle-and-pool either.
• The dispatch/cancel check (poll_read_head → Dispatch::poll_ready → poll_canceled) is gated by Conn::can_read_head(), which requires Reading::Init; in KeepAlive it's unreachable. And the response callback was already consumed when the response was delivered, so nothing observes the caller dropping the future either.

The read side falls through to poll_read_keep_alive → mid_message_detect_eof and parks. Nothing remaining can drive the connection to shutdown or to idle. It's a terminal park state.

Note this is distinct from #4085 (100% CPU busy-loop on peer FIN with no response) — here the peer sends a full response, there is no CPU spin, and the connection is silently stuck rather than busy-looping.

Steps to reproduce

Cargo.toml:

[dependencies]
hyper = { version = "=1.11.1", features = ["client", "http1"] }
hyper-util = { version = "0.1", features = ["tokio"] }
http-body-util = "0.1"
bytes = "1"
tokio = { version = "1", features = ["full"] }

src/main.rs: see the code below...

Running it prints:

sender.ready() within 5s: false
connection task finished within 5s: false
BUG REPRODUCED: connection stuck in (Reading::KeepAlive, Writing::Body)

Code Sample

use std::convert::Infallible;
use std::pin::Pin;
use std::task::{Context, Poll};
use std::time::Duration;

use bytes::Bytes;
use http_body_util::BodyExt;
use hyper::body::{Body, Frame, SizeHint};
use hyper::client::conn::http1;
use hyper::Request;
use hyper_util::rt::TokioIo;
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::net::{TcpListener, TcpStream};
use tokio::time::timeout;

/// A request body that emits one chunk, then parks forever — modeling a request whose
/// write has not completed (in the real world this happens via TCP backpressure when the
/// peer responds without draining a large body; here it's made deterministic).
struct StallBody {
    sent: bool,
}
impl Body for StallBody {
    type Data = Bytes;
    type Error = Infallible;
    fn poll_frame(
        mut self: Pin<&mut Self>,
        _: &mut Context<'_>,
    ) -> Poll<Option<Result<Frame<Bytes>, Infallible>>> {
        if !self.sent {
            self.sent = true;
            Poll::Ready(Some(Ok(Frame::data(Bytes::from_static(b"x")))))
        } else {
            Poll::Pending
        }
    }
    fn is_end_stream(&self) -> bool {
        false
    }
    fn size_hint(&self) -> SizeHint {
        SizeHint::default()
    }
}

#[tokio::main(flavor = "multi_thread")]
async fn main() {
    let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
    let addr = listener.local_addr().unwrap();

    // Server: read the request head, send a full response, then stop reading and hold open.
    tokio::spawn(async move {
        let (mut s, _) = listener.accept().await.unwrap();
        let mut buf = [0u8; 1024];
        loop {
            let n = s.read(&mut buf).await.unwrap();
            if n == 0 || buf[..n].windows(4).any(|w| w == b"\r\n\r\n") {
                break;
            }
        }
        s.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: keep-alive\r\n\r\nhi")
            .await
            .unwrap();
        s.flush().await.unwrap();
        tokio::time::sleep(Duration::from_secs(30)).await; // hold; never drain the request body
    });

    let tcp = TcpStream::connect(addr).await.unwrap();
    let (mut sender, conn) = http1::handshake::<_, StallBody>(TokioIo::new(tcp)).await.unwrap();
    let conn_done = tokio::spawn(conn); // background connection task

    let req = Request::builder()
        .method("POST")
        .uri("/")
        .header("host", "repro")
        .body(StallBody { sent: false })
        .unwrap();

    // Response head + body arrive fine, even though the request body write is stalled.
    let resp = sender.send_request(req).await.expect("send_request");
    let _ = resp.into_body().collect().await.expect("collect body"); // full response consumed

    // The request "succeeded". Now: is the connection reusable, or closed?
    let ready = timeout(Duration::from_secs(5), sender.ready()).await;
    let done = timeout(Duration::from_secs(5), conn_done).await;

    println!("sender.ready() within 5s: {:?}", ready.is_ok()); // false  -> never reusable
    println!("connection task finished within 5s: {}", done.is_ok()); // false  -> never closed
    assert!(
        ready.is_err() && done.is_err(),
        "BUG: response delivered, but the connection is neither reusable nor closed (leaked)"
    );
    println!("BUG REPRODUCED: connection stuck in (Reading::KeepAlive, Writing::Body)");
}

Expected Behavior

After the response is fully received, the client connection should reach a terminal outcome: either shut down (send FIN/RST) if the request body can't be completed, or become reusable. It should not remain ESTABLISHED and non-reusable indefinitely.

Actual Behavior

In actuality:

• sender.ready() never resolves,
• the Connection task never completes (no shutdown, socket stays ESTABLISHED),
• the request appears to have succeeded to the caller.

The same leak occurs through hyper_util::client::legacy::Client: the on_idle task (poll_fn(|cx| pooled.poll_ready(cx))) never resolves, so the connection is never inserted into the idle pool — a subsequent request dials a new socket and the wedged one leaks.

Additional Context

Proposed fix

Once a full response has been received, an HTTP/1 connection whose request body was not fully sent is not keep-alive-eligible (HTTP/1 requires the request to be fully sent before the connection can carry another request, and the peer has already responded and stopped reading). Per RFC 7230 §6.5–§6.6 a client in this situation may close the connection. So the correct action in (Reading::KeepAlive, Writing::Body) is to stop preserving the connection and shut it down rather than park.

Concretely, add an arm to State::try_keep_alive() in src/proto/h1/conn.rs:

// response fully read, but the request body never finished sending:
// the connection cannot be reused, and continuing to write is pointless
// (the peer already responded). Close instead of parking forever.
(&Reading::KeepAlive, &Writing::Body(_)) => {
    self.close();
}

self.close() sets both sides to Closed → is_done() becomes true → poll_shutdown() runs (FIN, or RST if there is undeliverable data). Via the pooled legacy::Client, SendRequest::poll_ready then resolves (error), so the connection is dropped rather than leaked. A regression test belongs next to client_flushing_is_not_ready_for_next_request in src/proto/h1/dispatch.rs: drive a client to (KeepAlive, Body) and assert the dispatcher reaches shutdown instead of returning Pending.

Caveat / open question for maintainers

This is an abort-vs-drain decision. The arm above aborts the in-flight request upload. That is almost always the right choice (the request already received its response), but alternatives exist:

• attempt a cheap drain of the remaining request body first (mirroring the existing read-side poll_drain_or_close_read pattern) and only close if it can't complete, or
• only close when the caller has dropped the response body / future, keeping the current behavior otherwise.

Happy to adjust the patch to whichever behavior you prefer before opening a PR.

Real-world trigger (for context)

No misbehaving body is needed. With a normal buffered request body, the same (KeepAlive, Body) state is reached when the peer sends a complete response and stops reading, and the request body is larger than the socket send buffer + peer's receive window — hyper's write stalls in Writing::Body. (A body that fits in the buffers flushes fully, reaches Writing::KeepAlive, and the connection idles normally, so it's specifically the un-flushable case.)

The attached repro uses a request body that stops yielding, which reaches the identical (KeepAlive, Body) state deterministically without depending on OS socket-buffer sizing.

[seanmonstar]

Yea interesting. So, I'm not sure this is a bug in hyper, but probably more 'working as intended'.

The request body has not finished flushing, so it will continue to do, waiting on the user-supplied body. Even though the server responded with a connection close, that does NOT indicate that the current request should abort. And I'm not so sure hyper should do that proactively. The server has a very simple option it could do: close the TCP connection itself. Since it hasn't, I think we should keep trying to write.

Is there some additional detail that I missed that makes this different?

[BlackRabbitCoder]

Thanks for looking. Agreed it's not a protocol violation as the client is allowed to keep writing after an early response, and the server could close the connection itself.

I think the "waiting on the user-supplied body" read is what's throwing this off. In the minimal repro I used a body that stops yielding so it'd reproduce deterministically, but that's not the situation we actually hit in real life. In our real life case the whole request body is already buffered and handed to hyper; it just can't finish flushing because the peer sent its full response and stopped reading, so its receive window is zero and we're parked in poll_flush waiting on the peer, not on the application. And the repro's server sends a normal keep-alive response (Content-Length, Connection: keep-alive) and never closes, with no Connection: close anywhere.

What makes it feel like more than "just keep writing": by the time we're stuck, the response is fully delivered and the caller has moved on. The recv_msg already took the response callback, and the only cancel path goes through can_read_head(), which needs Reading::Init and is unreachable once we're in Reading::KeepAlive. So there's no future left to cancel and no timeout, which leaves nothing the application can do to get the connection back. A peer that answers and then just stops reading, never sending a FIN, holds our connection and fd open until it decides to close. We saw this in prod: both ends ESTABLISHED, no FIN, the server tore it down minutes later. "The server could close TCP" is true, but when it doesn't, the client has no way out today, even on the timeout() path, which is problematic for a high TPS service like ours where FDs stacking up can quickly become a problem.

I get the abort-vs-drain tension, the main thing is even if the server is misbehaving a connection idle timeout should reclaim the connection, and it doesn't. As far as the requester (client) is concerned, it completed successfully but the connection is wedged and doesn't go back in the pool, nor does it idle out from the client side. Question: since the caller has nothing left to cancel once the response is in hand, would you be open to either closing the connection when a full response has been read and the request body can't complete, or some opt-in bound (a write/overall deadline, or a handle to abort)? And if neither, what's the intended way for a client to put a ceiling on this?

[seanmonstar]

OK, so the problem is that the server is, let's say 'misbehaving', because it's not reading, but it's also not closing. Right?

A write timeout makes sense. It could be something done in hyper. But it's also general enough that it likely could happen at the IO layer. Basically, in poll_write (and poll_write_vectored), if the underlying socket is returning Pending, then it could start a timeout, and if it triggers before another write is able to make any progress, then you have a stalled connection, I suppose.