reproducible panic "dispatch dropped without returning error"

[e00E]

This panic affects 0.14 and master.

Minimal reproduction on master:

#[tokio::main]
async fn main() {
    let (mut request_sender, mut connection) =
        hyper::client::conn::http1::handshake(Io).await.unwrap();
    let request = hyper::Request::builder().body("".to_string()).unwrap();
    let response = request_sender.send_request(request);
    futures_util::pin_mut!(response);

    let _ = futures_util::poll!(&mut connection);
    std::mem::drop(connection);
    // this line panics
    let _ = futures_util::poll!(&mut response);
}

My fork has a runnable example at https://github.com/e00E/hyper/blob/panic-example/examples/panic.rs using an always pending IO implementation. Note that this branch also changes Cargo.toml. The panic can also be observed with real IO like tokio::TcpStream. I observed this panic in real code using reqwest and researched this issue afterwards.

On master the panic message is

thread 'main' panicked at 'dispatch dropped without returning error', /home/e/temp/hyper/src/client/conn/http1.rs:200:39

On 0.14. the panic message is

thread 'main' panicked at 'dispatch dropped without returning error', src/client/conn.rs:1023:39

The panicking code assumes that a channel is never dropped before a value is sent. However if proto::h1::dispatch::Client is dropped at the right moment this condition is violated. The problem is in src/proto/h1/dispatch.rs around line 580 (master and 0.14.x) this.callback = Some(cb);. The Callback is removed from the Envelope and stored in Client. If Client is then dropped the Callback and the contained Sender are dropped without sending a value.

I made a PR to master with a fix. https://github.com/hyperium/hyper/pull/3031 . The same fix also works on 0.14.x . We can discuss the master PR first and when it is good I can backport the change. I am not very familiar with the hyper internals so there might be a better solution. I wasn't sure if the PRs should contain a test case and where it should reside, and whether the commit messages should include a full description of the issue.

I am unsure if http2 is affected. On master I see that we spawn a task on the executor that calls the callback. This seems problematic too because when the executor shuts down it will drop the task.

[e00E]

This kind of thing was also reported in https://github.com/hyperium/hyper/issues/2649 (didn't see before making this issue).

[e00E]

Fixed on 0.14.x by https://github.com/hyperium/hyper/pull/2790 .