Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reproducible panic "dispatch dropped without returning error" #3030

Open
e00E opened this issue Oct 28, 2022 · 2 comments
Open

reproducible panic "dispatch dropped without returning error" #3030

e00E opened this issue Oct 28, 2022 · 2 comments
Labels
C-bug Category: bug. Something is wrong. This is bad!

Comments

@e00E
Copy link

e00E commented Oct 28, 2022

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. #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 e00E added the C-bug Category: bug. Something is wrong. This is bad! label Oct 28, 2022
@e00E
Copy link
Author

e00E commented Oct 28, 2022

This kind of thing was also reported in #2649 (didn't see before making this issue).

@e00E
Copy link
Author

e00E commented Oct 28, 2022

Fixed on 0.14.x by #2790 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug. Something is wrong. This is bad!
Projects
None yet
Development

No branches or pull requests

1 participant