web::Data leaks when a slow request is ongoing during graceful shutdown

[liskin]

Expected Behavior

I have roughly the following code (a full reproducer will be linked further down):

struct State {
    _x: Arc<String>,
}

let (x_weak, data) = {
    let x: Arc<String> = Arc::new(String::from("x"));
    (Arc::downgrade(&x), web::Data::new(State { _x: x }))
};

let server = HttpServer::new(move || {
    App::new()
        .app_data(actix_web::web::JsonConfig::default().limit(1024 * 1024))
        .app_data(data.clone())
        .service(app::echo)
})
.shutdown_timeout(2)
.bind(("127.0.0.1", 0))?;

let server = server.run();
let server_handle = server.handle();

let graceful_stop = async move {
    rt::time::sleep(Duration::from_secs(1)).await;
    server_handle.stop(/* graceful */ true).await;
};

join!(server, graceful_stop);

and normally all the data gets dropped, so x_weak.upgrade().is_none() becomes true shortly after.

Current Behavior

If, on the other hand, there is an in-flight request throughout the graceful shutdown, the data isn't dropped, and x_weak.upgrade().is_some() holds forever afterwards. I suspect that the whole factory given to HttpServer::new is leaked somewhere.

Possible Solution

I haven't investigated this much further. 🙁

Steps to Reproduce (for bugs)

cargo test in https://github.com/liskin/actix-web-data-leak-2023-08/ will reproduce the issue. A rough outline of what it does (in the failure case):

1. set up and run a server like the code above
2. sleep 0.1s and initiate a request that sends its body really slowly
3. sleep 1s and initiate graceful shutdown
4. wait for all to conclude and then keep checking if the data behind the weak Arc get dropped within 2 seconds

There are also additional 2 test cases that demonstrate that the data indeed get dropped normally when no request ever happens or when a normal quick request concludes before the graceful shutdown.

Context

Our codebase has a background task that periodically cleans up something that is pointed to by a weak Arc and stops when it's dropped. After a recent refactoring we discovered that we can't await that background task because it never finishes if there are ongoing requests during the graceful shutdown. Not a huge issue, we'll use tokio cancellation tokens (those weren't a thing back when the code was last touched) instead or something, but thought it's worth reporting nonetheless.

Your Environment

• rustc 1.71.0 (8ede3aae2 2023-07-12)
• actix-web 4.3.1, reproducible with today's master as well
• Linux x86_64