Hyper client does not clean up idle connections with non-tokio executor

[erickt]

Version

Hyper 0.13.10

Platform The output of uname -a (UNIX), or version and 32 or 64-bit (Windows)

Fuchsia, as well as on Linux and OS-X

Description

Fuchsia uses hyper as our http client in a few places. Inside the Fuchsia operating system, we use a Fuchsia-specific executor. Our host-side development tools also use async-executor instead of tokio. Because of this, we've discovered that our client connection pool is not closing idle connections. This is because the pool implementation only runs the periodic idle connection cleaner if the runtime feature, which pulls in tokio, is enabled:

https://github.com/hyperium/hyper/blob/master/src/client/pool.rs#L385

We'd be happy to work around this on our side, but unfortunately hyper doesn't expose the pool implementation, or any method to remove idle connections.

One possible way we could support this is to add a function Client::close_idle_connections(), which under the covers calls PoolInner::clear_expired. We could set up a task using our timer interfaces to periodically close these connections. Would you be interested in patch that implements this? Or would you prefer a separate mechanism to expose this functionality?

[seanmonstar]

Yea, I agree, I wrote about runtime woes in the 1.0 proposal. There, I proposed a couple things to try to fix this:

• Adding a Timer trait, similar to the Executor trait, and have the timeouts and intervals use that instead of requiring Tokio.
• Moving the Client into hyper-util, and breaking it up into composable parts that could allow a more customized Pool.

I realize that 1.0 is probably farther away than you'd like to fix it. A stopgap could be a fine temporary solution... You said hyper v0.13? That branch doesn't see any more support. A temporary solution in 0.14.x seems feasible though.

[dhobsd]

We discussed this a while ago in Discord and then I never got the time to prioritize doing this that I thought I'd have. My apologies.

Copying over the suggestion at the time:

pub trait Timer {
    /// Return a future that completes in `dur` time.
    fn sleep(&self, dur: Duration) -> Pin<Box<dyn Future<Output = ()>>>;
}

#[derive(Clone)]
pub(crate) enum TimerImpl {
    Default,
    Timer(Arc<dyn Timer + Send + Sync>),
}

impl Builder {
    pub fn timer<T: Timer + Send + Sync>(mut self, timer: T) -> Self {
        self.inner.timer = TimerImpl::Timer(Arc::new(timer));
        self
    }
}

One issue with implementing this for Fuchsia is that our timers don't support re-arming before they've fired, while Tokio's do. This behavior would be relied upon by hyper, since h2 pings and acks would generally arrive before the timers expire in the normal happy case. ISTR a good solution was at least not trivial, but I am at this point pretty hazy on what happened in December 2020 :).

Erick, given the effort it took to do a 0.13 point update, what are your thoughts on updating to 0.14.x?

[erickt]

@dhobsd - I ended up somewhat working around this in Fuchsia by migrating off the high level Server, over to the lower level API in https://fuchsia-review.googlesource.com/c/fuchsia/+/669708. My patch doesn't time out connections, but it does allow us to tear down all the connections when the server is stopped. I don't think it'd be too hard to extend my CL to also support timing out connections.

That said, it'd be nicer to incorporate this into hyper proper.

Erick, given the effort it took to do a 0.13 point update, what are your thoughts on updating to 0.14.x?

Yeah, I'm spinning up a work group to do this, I ping you offline to see if you're interested in helping out.