Skip to content

Commit

Permalink
fix: [#588] broken grateful shutdown for tracker API
Browse files Browse the repository at this point in the history
The internal halt channel was nor working becuase the sender was being
droped just after starting the server.

That also made the `shutdown_signal` fail.

```rust
pub async fn shutdown_signal(rx_halt: tokio::sync::oneshot::Receiver<Halted>) {
    let halt = async {
        match rx_halt.await {
            Ok(signal) => signal,
            Err(err) => panic!("Failed to install stop signal: {err}"),
        }
    };

    tokio::select! {
        signal = halt => { info!("Halt signal processed: {}", signal) },
        () = global_shutdown_signal() => { info!("Global shutdown signal processed") }
    }
}
```

Since the signal branch in the `tokio::select!` was finishing the
global_shutdown_signal did not work either. So you had to kill the
process manually to stop the tracker.

It seems Rust droped partially the `Running::halt_taks` attribute and
that closed the channel.
  • Loading branch information
josecelano committed Jan 9, 2024
1 parent 5f6eed7 commit cf613b8
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/bootstrap/jobs/tracker_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ async fn start_v1(socket: SocketAddr, tls: Option<RustlsConfig>, tracker: Arc<co
.expect("it should be able to start to the tracker api");

tokio::spawn(async move {
assert!(!server.state.halt_task.is_closed(), "Halt channel should be open");

Check warning on line 83 in src/bootstrap/jobs/tracker_apis.rs

View check run for this annotation

Codecov / codecov/patch

src/bootstrap/jobs/tracker_apis.rs#L83

Added line #L83 was not covered by tests
server.state.task.await.expect("failed to close service");
})
}
Expand Down
23 changes: 17 additions & 6 deletions src/servers/apis/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use axum_server::tls_rustls::RustlsConfig;
use axum_server::Handle;
use derive_more::Constructor;
use futures::future::BoxFuture;
use log::error;
use tokio::sync::oneshot::{Receiver, Sender};

use super::routes::router;
Expand Down Expand Up @@ -101,13 +102,23 @@ impl ApiServer<Stopped> {
launcher
});

Ok(ApiServer {
state: Running {
binding: rx_start.await.expect("unable to start service").address,
halt_task: tx_halt,
task,
//let address = rx_start.await.expect("unable to start service").address;
let api_server = match rx_start.await {
Ok(started) => ApiServer {
state: Running {
binding: started.address,
halt_task: tx_halt,
task,
},
},
})
Err(err) => {
let msg = format!("unable to start API server: {err}");
error!("{}", msg);
panic!("{}", msg);

Check warning on line 117 in src/servers/apis/server.rs

View check run for this annotation

Codecov / codecov/patch

src/servers/apis/server.rs#L115-L117

Added lines #L115 - L117 were not covered by tests
}
};

Ok(api_server)
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/servers/signals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,16 @@ pub async fn global_shutdown_signal() {
///
/// Will panic if the `stop_receiver` resolves with an error.
pub async fn shutdown_signal(rx_halt: tokio::sync::oneshot::Receiver<Halted>) {
let halt = async { rx_halt.await.expect("Failed to install stop signal.") };
let halt = async {
match rx_halt.await {
Ok(signal) => signal,
Err(err) => panic!("Failed to install stop signal: {err}"),

Check warning on line 52 in src/servers/signals.rs

View check run for this annotation

Codecov / codecov/patch

src/servers/signals.rs#L52

Added line #L52 was not covered by tests
}
};

tokio::select! {
_ = halt => {},
() = global_shutdown_signal() => {}
signal = halt => { info!("Halt signal processed: {}", signal) },
() = global_shutdown_signal() => { info!("Global shutdown signal processed") }
}
}

Expand Down

0 comments on commit cf613b8

Please sign in to comment.