1.8.0 concurrent downloading of large files exhausts all CPU resources

[18o]

Version
1.8.0

Platform
Linux x86_64

Description
I have a video file forwarding tool that uses the requests+salvo libraries to directly pass the stream obtained via request into the response stream of salvo. However, after upgrading hyper to version 1.8.0, the CPU usage spikes to 100% regardless of how many tokio worker threads I enable. When I revert to version 1.7.0, the CPU usage returns to normal, stabilizing below 10%. In the test environment, the tool was handling approximately 300 concurrent user requests at the time.

I also tested the same functionality using a pure hyper server and client with tokio, and observed the same 100% CPU utilization in 1.8.0.

This issue only seems to occur when forwarding large files, another function, which is only used for JSON responses, is unaffected.

[short summary of the bug]

I tried this code:

[code sample that causes the bug]

I expected to see this happen: [explanation]

Instead, this happened: [explanation]

[seanmonstar]

Thanks for the report! Hm, looking at the changes, the relevant one looks like #3952

cc @lthiery, if you have a moment to look.

[mattklein123]

Pretty sure I'm hitting this also. I would recommend yanking 1.8.0 until this is sorted. Thank you.

[shikhar]

We also hit this and reverted back to 1.7.0

[seanmonstar]

v1.8.1 has been published with that change reverted.

[lthiery]

@seanmonstar I'm so sorry about this! I'm going to look into it.

My best guess is that I botched some coordination between our readiness to write a body and the Writers ability to be written to and it's somehow hot polling. What I don't understand though is why the backing out after 16 loops isn't protecting something this severe from happening...

If @mattklein123 @18o @mattklein123 or @shikhar can provide a minimum repro based off of 1.8.0 that would be a huge help.

And just to clarify, your server is sending down a large stream, correct?

Is everyone using Tokio? Any other details would helpful even if you can't provide a repro.

Thanks!

[18o]

@seanmonstar I'm so sorry about this! I'm going to look into it.

My best guess is that I botched some coordination between our readiness to write a body and the Writers ability to be written to and it's somehow hot polling. What I don't understand though is why the backing out after 16 loops isn't protecting something this severe from happening...

If @mattklein123 @18o @mattklein123 or @shikhar can provide a minimum repro based off of 1.8.0 that would be a huge help.

And just to clarify, your server is sending down a large stream, correct?

Is everyone using Tokio? Any other details would helpful even if you can't provide a repro.

Thanks!

Yes, I'm using Tokio as the runtime. My scenario doesn't involve handling a single large stream; instead, I'm processing 300 streams simultaneously, each around 100MB in size. The issue was resolved once hyper was updated to version 1.8.1.

For example, a minimal example like this:

use reqwest::Client;
use salvo::prelude::Stream;
use reqwest::header::{CONTENT_DISPOSITION, CONTENT_TYPE};

pub async fn download_and_stream(url: &str, res: &mut salvo::Response) -> Result<(), String> {
    let client = Client::new();
    let response = client.get(url).send().await.map_err(|e| format!("Request failed: {}", e))?;

    if response.status().as_u16() != 200 {
        return Err(format!("HTTP error: {}", response.status()));
    }

    let content_type = response
        .headers()
        .get(CONTENT_TYPE)
        .and_then(|v| v.to_str().ok())
        .unwrap_or("application/octet-stream");

    let content_length = response.content_length().unwrap_or(0);

    res.headers_mut().insert(CONTENT_TYPE, content_type.parse().unwrap());
    if content_length > 0 {
        res.headers_mut().insert("Content-Length", content_length.into());
    }
    res.headers_mut().insert(
        CONTENT_DISPOSITION,
        "attachment; filename=download".parse().unwrap()
    );

    res.stream(response.bytes_stream());

    Ok(())
}

[seanmonstar]

Thanks for the confirmation! I'm going to close this since the new release fixed it.

@lthiery please don't stress, it was easy to resolve, and the code is complicated. ❤️

[mattklein123]

Sorry I don't have any specific repro. I deployed this change to canary on our edge fleet and CPU went to 100%.