fix: wait for daemon exit during server shutdown - #266
Conversation
|
🔍 OpenCodeReview found 2 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| self.inner | ||
| .daemon_exited | ||
| .subscribe() | ||
| .wait_for(|exited| *exited) | ||
| .await | ||
| .context("wait for ublk daemon exit")?; |
There was a problem hiding this comment.
This wait has no deadline or fallback, unlike the 5-second RPC call above. If the daemon acknowledges (or ignores) Shutdown but remains alive, or the watchdog never publishes its notification, every caller of shutdown() will hang indefinitely and can block application teardown. Bound this wait (and ideally terminate/escalate the child on timeout) so best-effort shutdown cannot become an unbounded cleanup operation.
| daemon_dead: AtomicBool::new(daemon_dead), | ||
| daemon_exited: tokio::sync::watch::channel(true).0, |
There was a problem hiding this comment.
The fixture marks daemon_exited true regardless of daemon_dead. Thus new_for_test(..., false) models a live daemon as already exited, and any shutdown test that does not manually reset the channel can pass without exercising the watchdog wait (the existing shutdown_succeeds_even_without_response test does exactly that). Initialize this channel from daemon_dead, or make the fixture explicitly configure the exit state.
2 participants
What
Wait for the ublk daemon process to fully exit before completing server shutdown.
Why
The daemon may acknowledge shutdown before finishing cleanup. If the server restarts during this window, the old daemon can remove the new daemon's Unix socket, causing subsequent requests to fail.
Related issue
Closes #158
Scope and non-goals
This change only coordinates shutdown between the server and its managed ublk daemon. It does not add daemon auto-restart or change systemd configuration.
Design and behavior changes
The daemon watchdog publishes a retained exit notification after
child.wait()completes.shutdown()sends the shutdown request and then waits for that notification.Compatibility and operations
Validation
make fmtmake clippymake test-unitmake -C services test(required whenservices/changes)maketargetSkipped checks and reasons:
No generated code, services, documentation, or performance-sensitive behavior changed.
Risks and reviewer notes
The server shutdown now waits until the daemon process has actually exited.
Checklist