actix-client doesn't close HTTP2 connection when run out of conn_lifetime

[ailisp]

It happens in our staging server. A minimum example to reproduce:

Dependency:

[dependencies]
actix = "0.8.3"
actix-web = { version = "1.0", features = ["ssl"] }
futures = "0.1"
tokio-timer = "0.2"
serde_json = "*"
serde = "*"

main.rs:

use actix::System;
use actix_web::client::{Client, Connector};
use futures::future::{lazy, Future};
use std::time::Duration;
const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
use futures::stream::Stream;
use tokio_timer::Interval;

#[macro_use]
extern crate serde_json;

fn main() {
    let client = Client::build()
        .timeout(CONNECT_TIMEOUT)
        .connector(
            Connector::new()
                .conn_lifetime(Duration::from_secs(2))
                .conn_keep_alive(Duration::from_secs(1))
                .finish(),
        )
        .finish();
    System::new("test")
        .block_on(lazy(|| {
            Interval::new_interval(Duration::from_millis(100))
                .map_err(|e| println!("{}", e))
                .for_each(|_| {
                    client
                        .post("https://explorer.nearprotocol.com/api/node")
                        .header("Content-Type", "application/json")
                        .send_json(&json!("aaa"))
                        .map_err(|_| ())
                        .map(|_| ())
                })
                .map(|_| ())
        }))
        .unwrap();
}

cargo build
target/debug/actix-test # assume project name is actix-test
ps aux | grep actix-test # get pid
watch 'lsof -i -a -p <pid>'

You will see unclosed connection is growing every 2s.

I found this only happens when server is http/2, and did some basic debugging. My finding: in actix-http/src/client/pool.rs, it only closes connection when connection type is http1, and http2 case was not handled.

[pythoneer]

Thanks for bringing this up to our attention!

the issue from upstream looks relevant.

Seems like h2 connections are not explicitly handled here. Maybe the intention was to implicitly drop the connection like it was suggested in the upstream issue but fails somehow.

    fn release_close(&mut self, io: ConnectionType<Io>) {
        self.acquired -= 1;
        if let Some(timeout) = self.disconnect_timeout {
            if let ConnectionType::H1(io) = io {
                actix_rt::spawn(CloseConnection::new(io, timeout))
            }
        }
        self.check_availibility();
    }

[fakeshadow]

ConnectionType::H2 should carry a waker to wake up the h2 Connection future and end it properly when dropping. This can be done with a typed future stores a shared waker or a select on oneshot channel.

Below would be the future need to be resolved on ConnectionType drop

actix-web/actix-http/src/client/pool.rs

Line 142 in c201c15

actix_rt::spawn(connection.map(|_| ()));