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 server leaking IO resources #2705

Closed
wants to merge 1 commit into from

Conversation

nicolaiunrein
Copy link

@nicolaiunrein nicolaiunrein commented Apr 10, 2024

Motivation

TcpStreams are not dropped when the future returned from serve is dropped. This is the case with and without graceful shutdown. If there are open connections for long running requests these will possibly hang on to the TcpStream forever making it imposibble to start a new server or listen on this port until the process exits and the OS cleans up the port.

Solution

I propose adding a drop signal and using tokio::select on this and the body of task::spawn

Example

use std::sync::Arc;

use axum::{routing::get, Router};

#[tokio::main]
async fn main() {
    let notify = Arc::new(tokio::sync::Notify::new());
    let cancel = notify.clone();
    let app = Router::new().route(
        "/",
        get(move || async move {
            notify.notify_waiters();
            loop {
                tokio::time::sleep(std::time::Duration::from_secs(1)).await;
            }
        }),
    );

    println!("starting server");

    let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await.unwrap();

    axum::serve(listener, app)
        .with_graceful_shutdown(async move { cancel.notified().await })
        .await
        .unwrap();

    println!("server is shut down");

    // but port 3000 is still blocked

    loop {
        tokio::time::sleep(std::time::Duration::from_secs(1)).await;
    }
}

@nicolaiunrein nicolaiunrein changed the title drop listening tasks on server drop fix server leaking IO resources Apr 10, 2024
@nicolaiunrein
Copy link
Author

I just realised that the implementation and example is wrong for the graceful case. But in both cases I think it would very nice if there was a way to shutdown the server forcefully dropping all open connections. In our project we use a lot of sse streams in child processes spawned on windows. Letting the OS close the connections does not work in our case because windows seems to not close connections if the parent process is still alive. I can think of other situations where it is desirable to close all open connections. And the current implementation is at least very surprising! I will update my PR tomorrow.

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.

None yet

1 participant