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

[yuriy-odonnell-epic]

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]

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.

[seanmonstar]

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?

[yuriy-odonnell-epic]

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

[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".