hyper icon indicating copy to clipboard operation
hyper copied to clipboard

hyper-util: client connection pool idle timeout does not work as expected

Open yuriy-odonnell-epic opened this issue 1 year ago • 5 comments

Version Latest hyper (1.3.1) & hyper-util (0.1.3)

Platform Windows11

Description The issue reproduces in a modified hyper-util client example:

use std::env;

use http_body_util::Empty;
use hyper::Request;
use hyper_util::client::legacy::{connect::HttpConnector, Client};

#[tokio::main(flavor = "current_thread")]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let url = match env::args().nth(1) {
        Some(url) => url,
        None => {
            eprintln!("Usage: client <url>");
            return Ok(());
        }
    };

    // HTTPS requires picking a TLS implementation, so give a better
    // warning if the user tries to request an 'https' URL.
    let url = url.parse::<hyper::Uri>()?;
    if url.scheme_str() != Some("http") {
        eprintln!("This example only works with 'http' URLs.");
        return Ok(());
    }

    let client = Client::builder(hyper_util::rt::TokioExecutor::new())
        .pool_timer(hyper_util::rt::TokioTimer::new())
        .pool_idle_timeout(std::time::Duration::from_secs(1))
        .build(HttpConnector::new());

    let req = Request::builder()
        .uri(url)
        .body(Empty::<bytes::Bytes>::new())?;

    for _ in 0..4 {
        eprintln!("Running request");
        let resp = client.request(req.clone()).await?;

        eprintln!("{:?} {:?}", resp.version(), resp.status());
        eprintln!("{:#?}", resp.headers());

        eprintln!("Waiting 5 seconds");
        _ = tokio::time::sleep(std::time::Duration::from_secs(5)).await;
    }

    eprintln!("Exiting");

    Ok(())
}

Client is configured with idle timeout of 1 sec and the example performs several requests with 5 second delay. The expectation is that a connection should be dropped from the pool after 1 second during each loop iteration. In practice, the connections start to get dropped only on the second iteration of the loop.

In a real application, this results in connections lingering until a new request is issued.

yuriy-odonnell-epic avatar Apr 19 '24 14:04 yuriy-odonnell-epic

Looks like pool_timer() should be used to actually trigger the idle interval checking (just timer() is not enough). However, it does not kick in until the 2nd iteration of the loop, which means that if the client only performs a single request, the idle connection lingers.

I've updated the initial bug report to reflect this.

yuriy-odonnell-epic avatar Apr 19 '24 17:04 yuriy-odonnell-epic

Interesting. I can't remember why, maybe it was so a task wasn't started if the client never sent a request? Might be a simple fix, have you tried that modification?

seanmonstar avatar Apr 19 '24 17:04 seanmonstar

It looks like dropping resp returns the connection to the pool and triggers the idle trimming task.

yuriy-odonnell-epic avatar Apr 19 '24 18:04 yuriy-odonnell-epic

Honestly, it might be reasonable to maintain the connection until response future is dropped. It may be just worth adding a small note to the docs, for posterity. In real applications, I don't expect response futures to stick around indefinitely.

I'm happy to close the issue, if you deem this "working as designed".

yuriy-odonnell-epic avatar Apr 19 '24 18:04 yuriy-odonnell-epic

I also encountered the same problem. Once a request is initiated, the TCP connection will always exist and will not time out and close.

hegbear avatar Oct 07 '24 07:10 hegbear