mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Fix remote app-server shutdown race (#18936)
## Why A Mac Bazel CI run saw `remote_notifications_arrive_over_websocket` fail during shutdown with `remote app-server shutdown channel is closed` (https://app.buildbuddy.io/invocation/9dac05d6-ae20-40f9-b627-fca6e91cf127). The remote websocket worker can legitimately finish while `shutdown()` is waiting for the shutdown acknowledgement: after the test server sends a notification and exits, the worker may deliver the required disconnect event, observe that the caller has dropped the event receiver, and exit before it sends the shutdown one-shot. That state is already terminal cleanup, not a failed shutdown, so callers should not see a `BrokenPipe` from the acknowledgement channel. ## What Changed - Treat a closed remote shutdown acknowledgement as an already-exited worker while still propagating websocket close errors when the worker returns them. - Added a deterministic regression test for the interleaving where the shutdown command is received and the worker exits before replying. ## Verification - `cargo test -p codex-app-server-client` - New test: `remote::tests::shutdown_tolerates_worker_exit_after_command_is_queued`
This commit is contained in:
committed by
GitHub
Unverified
parent
a978e411f6
commit
8fea372c77
@@ -612,14 +612,9 @@ impl RemoteAppServerClient {
|
||||
.send(RemoteClientCommand::Shutdown { response_tx })
|
||||
.await
|
||||
.is_ok()
|
||||
&& let Ok(command_result) = timeout(SHUTDOWN_TIMEOUT, response_rx).await
|
||||
&& let Ok(Ok(close_result)) = timeout(SHUTDOWN_TIMEOUT, response_rx).await
|
||||
{
|
||||
command_result.map_err(|_| {
|
||||
IoError::new(
|
||||
ErrorKind::BrokenPipe,
|
||||
"remote app-server shutdown channel is closed",
|
||||
)
|
||||
})??;
|
||||
close_result?;
|
||||
}
|
||||
|
||||
if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut worker_handle).await {
|
||||
@@ -981,4 +976,24 @@ mod tests {
|
||||
skipped: 1
|
||||
}));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_tolerates_worker_exit_after_command_is_queued() {
|
||||
let (command_tx, mut command_rx) = mpsc::channel(1);
|
||||
let (_event_tx, event_rx) = mpsc::channel(1);
|
||||
let worker_handle = tokio::spawn(async move {
|
||||
let _ = command_rx.recv().await;
|
||||
});
|
||||
let client = RemoteAppServerClient {
|
||||
command_tx,
|
||||
event_rx,
|
||||
pending_events: VecDeque::new(),
|
||||
worker_handle,
|
||||
};
|
||||
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("shutdown should complete when worker exits first");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user