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

fix(client): don't panic in DNS resolution when task cancelled #2229

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

dae
Copy link
Contributor

@dae dae commented Jun 11, 2020

If the runtime is dropped while a DNS request is being made, it can
lead to a panic. This patch checks if the task was cancelled, and
returns a generic IO error instead of panicking in that case.

The following code reproduces the problem on my macOS machine:

use hyper::Client;
use tokio::runtime;

type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;

fn main() {
    let rt = runtime::Builder::new()
        .threaded_scheduler()
        .core_threads(1)
        .enable_all()
        .build()
        .unwrap();

    // spawn a request and then drop the runtime immediately
    rt.spawn(fetch_url());
}

async fn fetch_url() -> Result<()> {
    let url: hyper::Uri = "http://example.com".parse()?;
    let client = Client::builder().build_http::<hyper::Body>();

    let res = client.get(url).await?;
    println!("Response: {}", res.status());

    Ok(())
}

If you think this should be handled differently, please let me know.

If the runtime is dropped while a DNS request is being made, it can
lead to a panic. This patch checks if the task was cancelled, and
returns a generic IO error instead of panicking in that case.

The following code reproduces the problem on my macOS machine:

```
use hyper::Client;
use tokio::runtime;

type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;

fn main() {
    let rt = runtime::Builder::new()
        .threaded_scheduler()
        .core_threads(1)
        .enable_all()
        .build()
        .unwrap();

    // spawn a request and then drop the runtime immediately
    rt.spawn(fetch_url());
}

async fn fetch_url() -> Result<()> {
    let url: hyper::Uri = "http://example.com".parse()?;
    let client = Client::builder().build_http::<hyper::Body>();

    let res = client.get(url).await?;
    println!("Response: {}", res.status());

    Ok(())
}
```
@seanmonstar seanmonstar merged commit 0d0d363 into hyperium:master Jun 11, 2020
@seanmonstar
Copy link
Member

Thanks, very clear explanation!

BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
…ium#2229)

If the runtime is dropped while a DNS request is being made, it can
lead to a panic. This patch checks if the task was cancelled, and
returns a generic IO error instead of panicking in that case.

The following code reproduces the problem on my macOS machine:

```
use hyper::Client;
use tokio::runtime;

type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;

fn main() {
    let rt = runtime::Builder::new()
        .threaded_scheduler()
        .core_threads(1)
        .enable_all()
        .build()
        .unwrap();

    // spawn a request and then drop the runtime immediately
    rt.spawn(fetch_url());
}

async fn fetch_url() -> Result<()> {
    let url: hyper::Uri = "http://example.com".parse()?;
    let client = Client::builder().build_http::<hyper::Body>();

    let res = client.get(url).await?;
    println!("Response: {}", res.status());

    Ok(())
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants