diff --git a/crates/codex-runtime/tests/runtime_binary.rs b/crates/codex-runtime/tests/runtime_binary.rs index 1e3b73d..073855b 100644 --- a/crates/codex-runtime/tests/runtime_binary.rs +++ b/crates/codex-runtime/tests/runtime_binary.rs @@ -20,14 +20,20 @@ fn runner_socket(dir: &Path) -> PathBuf { dir.join("runner.sock") } +/// The worker's socket, once it accepts. A worker binary never run before can take macOS a few +/// seconds to start: it checks a new executable file on its first exec, and cargo places the +/// binary anew for each test run. async fn wait_connect(socket: &Path) -> UnixStream { - for _ in 0..100 { + let deadline = tokio::time::Instant::now() + Duration::from_secs(10); + loop { if let Ok(stream) = UnixStream::connect(socket).await { return stream; } + if tokio::time::Instant::now() >= deadline { + panic!("worker socket not ready at {}", socket.display()); + } tokio::time::sleep(Duration::from_millis(20)).await; } - panic!("worker socket not ready at {}", socket.display()); } async fn spawn_worker(socket: &Path) -> tokio::process::Child { diff --git a/crates/runner/src/launch.rs b/crates/runner/src/launch.rs index d20b843..c5d9b56 100644 --- a/crates/runner/src/launch.rs +++ b/crates/runner/src/launch.rs @@ -688,10 +688,15 @@ mod tests { /// consumes the permit, then runs `behavior`, which writes transcript lines with `report` /// as the real helper does. It cannot present a grant, so it tests CodeSpace's side only. /// It runs under bash: the carriers' numbers can exceed 9, which dash, Linux's `sh`, cannot - /// redirect. + /// redirect. On macOS it is run once when written, with `--warm`: macOS checks a new + /// executable file on its first exec, which can take seconds while other tests start theirs, + /// and that time must not count against the launches it serves. Not on Linux, where the + /// check does not exist and running a file this process has just written can fail with + /// `ETXTBSY` (see `write_script` in `linux_sandbox`). fn stand_in(dir: &Path, name: &str, entry: &str, behavior: &str) -> PathBuf { let script = format!( r#"#!/bin/bash +if [ "$1" = --warm ]; then exit 0; fi while [ "$1" != "--" ]; do case "$1" in --permit-fd) permit="$2" ;; @@ -710,6 +715,13 @@ report() {{ eval "printf '%s\n' \"\$1\" >&$report"; }} let path = dir.join(name); std::fs::write(&path, script).unwrap(); std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o700)).unwrap(); + if cfg!(target_os = "macos") { + let warmed = std::process::Command::new(&path) + .arg("--warm") + .status() + .unwrap(); + assert!(warmed.success(), "{warmed}"); + } path } diff --git a/crates/runner/src/linux_sandbox.rs b/crates/runner/src/linux_sandbox.rs index 6b0e2d7..234b931 100644 --- a/crates/runner/src/linux_sandbox.rs +++ b/crates/runner/src/linux_sandbox.rs @@ -609,9 +609,11 @@ mod tests { #[test] fn prepare_stdout_flood_is_too_large() { - let dir = tempfile::tempdir().unwrap(); - let helper = write_script(dir.path(), "#!/bin/sh\nwhile :; do printf x; done\n"); - let spawned = Command::new(&helper) + // The system's `yes` floods in large writes, from a file executed before. Under load, + // a byte per write, or the first exec of a script written for the test, could take + // long enough to reach the deadline before the limit. + let helper = Path::new("/usr/bin/yes"); + let spawned = Command::new(helper) .arg("prepare") .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -619,7 +621,7 @@ mod tests { .spawn() .unwrap(); let start = Instant::now(); - let err = prepare_from_child(&helper, spawned, b"{}", Duration::from_secs(2), 64 * 1024) + let err = prepare_from_child(helper, spawned, b"{}", Duration::from_secs(2), 64 * 1024) .unwrap_err(); assert_eq!(err.code, ErrorCode::ProcessSpawnFailed); assert!(err.message.contains("output too large"), "{}", err.message); diff --git a/crates/server/src/runtime.rs b/crates/server/src/runtime.rs index a871f1a..07fbc1b 100644 --- a/crates/server/src/runtime.rs +++ b/crates/server/src/runtime.rs @@ -178,17 +178,25 @@ pub(crate) fn worker_command(bin: &Path, socket: &Path) -> Command { command } +/// How long the gateway waits for a started worker's socket. A worker binary never run before +/// can take macOS a few seconds to start, as it checks a new executable file on its first exec. +const WORKER_READY_BOUND: Duration = Duration::from_secs(10); + +/// The worker's socket once it accepts; an error at once if the worker exits first. async fn wait_for_socket(socket: &Path, wait: &JoinHandle<()>) -> Result { - for _ in 0..100 { + let deadline = tokio::time::Instant::now() + WORKER_READY_BOUND; + loop { if wait.is_finished() { return Err(anyhow!("worker exited before the runner socket was ready")); } match UnixStream::connect(socket).await { Ok(stream) => return Ok(stream), - Err(_) => tokio::time::sleep(Duration::from_millis(20)).await, + Err(_) if tokio::time::Instant::now() < deadline => { + tokio::time::sleep(Duration::from_millis(20)).await + } + Err(_) => return Err(anyhow!("worker socket not ready at {}", socket.display())), } } - Err(anyhow!("worker socket not ready at {}", socket.display())) } #[cfg(test)] @@ -498,4 +506,26 @@ mod tests { .expect("process leases released after worker death"); assert!(!dir.exists() || std::fs::read_dir(&dir).map(|d| d.count()).unwrap_or(0) == 0); } + + /// The wait for a worker's socket is long enough for a first exec, but a worker that exits + /// first fails the start at once. + #[tokio::test] + async fn a_worker_that_exits_before_its_socket_fails_at_once() { + let store = Arc::new(Store::memory().unwrap()); + let started = std::time::Instant::now(); + let error = + RuntimeProcess::spawn(Path::new("/usr/bin/false"), None, Arc::new(|_| {}), store) + .await + .err() + .expect("a worker that exits cannot serve"); + assert!( + format!("{error:#}").contains("worker exited before"), + "{error:#}" + ); + assert!( + started.elapsed() < Duration::from_secs(5), + "{:?}", + started.elapsed() + ); + } }