Skip to content

Commit 0c3a0dd

Browse files
committed
fix(codex): recover crash sockets on relaunch
1 parent d521e87 commit 0c3a0dd

1 file changed

Lines changed: 27 additions & 13 deletions

File tree

src/codex_app_server.rs

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3991,7 +3991,8 @@ fn set_close_on_exec(fd: libc::c_int) -> std::io::Result<()> {
39913991
///
39923992
/// Explicit cleanup covers normal returns and Rust errors. The in-group watchdog covers wrapper
39933993
/// crashes, SIGKILL, and supervisor teardown. The watchdog holds the group ID until cleanup, so a
3994-
/// stale PID can never identify a process group that belongs to another live owner.
3994+
/// stale PID can never identify a process group that belongs to another live owner. A crash can
3995+
/// leave one dead socket file; the next launch proves that it has no listener and removes it.
39953996
fn spawn_process_group(
39963997
command: &mut Command,
39973998
socket_path: Option<&Path>,
@@ -4002,9 +4003,8 @@ fn spawn_process_group(
40024003
let mut watchdog_command = Command::new("/bin/sh");
40034004
watchdog_command
40044005
.arg("-c")
4005-
.arg("IFS= read -r ignored; if [ -n \"$1\" ]; then /bin/rm -f -- \"$1\"; fi; kill -KILL 0")
4006+
.arg("IFS= read -r ignored; kill -KILL 0")
40064007
.arg("st2-codex-watchdog")
4007-
.arg(socket_path.unwrap_or_else(|| Path::new("")))
40084008
.stdin(Stdio::from(std::os::fd::OwnedFd::from(watchdog_read)))
40094009
.stdout(Stdio::null())
40104010
.stderr(Stdio::null());
@@ -7684,9 +7684,8 @@ mod tests {
76847684
}
76857685

76867686
#[test]
7687-
fn an_app_server_group_dies_when_its_wrapper_is_killed() {
7688-
const TEST_NAME: &str =
7689-
"codex_app_server::tests::an_app_server_group_dies_when_its_wrapper_is_killed";
7687+
fn a_killed_wrapper_reaps_its_app_server_and_the_next_launch_recovers_its_socket() {
7688+
const TEST_NAME: &str = "codex_app_server::tests::a_killed_wrapper_reaps_its_app_server_and_the_next_launch_recovers_its_socket";
76907689
const ROLE: &str = "ST2_CODEX_ORPHAN_TEST_ROLE";
76917690
const SOCKET_PATH: &str = "ST2_CODEX_ORPHAN_TEST_SOCKET";
76927691
const PID_PATH: &str = "ST2_CODEX_ORPHAN_TEST_PID";
@@ -7773,24 +7772,39 @@ mod tests {
77737772
}
77747773
let _ = wrapper.wait();
77757774
let deadline = Instant::now() + Duration::from_secs(2);
7776-
while (process_can_retain_cleanup_resources(server_pid) || socket_path.exists())
7777-
&& Instant::now() < deadline
7778-
{
7775+
while process_can_retain_cleanup_resources(server_pid) && Instant::now() < deadline {
77797776
std::thread::sleep(Duration::from_millis(10));
77807777
}
77817778
let server_survived = process_can_retain_cleanup_resources(server_pid);
7782-
let socket_survived = socket_path.exists();
77837779
if server_survived {
77847780
unsafe {
77857781
libc::kill(server_pid, libc::SIGKILL);
77867782
}
77877783
}
7788-
let _ = fs::remove_file(&socket_path);
77897784
assert!(!server_survived, "the app-server survived its wrapper");
77907785
assert!(
7791-
!socket_survived,
7792-
"the app-server socket survived its wrapper"
7786+
socket_path.exists(),
7787+
"the app-server did not leave the expected recoverable socket"
7788+
);
7789+
assert_eq!(
7790+
UnixStream::connect(&socket_path).unwrap_err().kind(),
7791+
std::io::ErrorKind::ConnectionRefused,
7792+
"the residual socket still had a live listener"
7793+
);
7794+
7795+
prepare_socket_for_launch(&socket_path)
7796+
.expect("the next launch did not recover the residual socket");
7797+
assert!(
7798+
!socket_path.exists(),
7799+
"the next launch did not remove the residual socket"
7800+
);
7801+
let replacement = UnixListener::bind(&socket_path)
7802+
.expect("the next app-server could not bind the recovered socket");
7803+
assert!(
7804+
UnixStream::connect(&socket_path).is_ok(),
7805+
"the replacement app-server socket did not accept a connection"
77937806
);
7807+
drop(replacement);
77947808
}
77957809

77967810
#[test]

0 commit comments

Comments
 (0)