diff --git a/desktop/src-tauri/src/claim.rs b/desktop/src-tauri/src/claim.rs index 22d0cfcbc24..d7e2d53ef51 100644 --- a/desktop/src-tauri/src/claim.rs +++ b/desktop/src-tauri/src/claim.rs @@ -163,7 +163,12 @@ pub async fn run(app: &AppHandle, argv: Vec, deadline: Instant) -> Claim return ClaimResult::Failed(format!("the bundled CLI could not be started ({error})")) } }; - match timeout_at(deadline, command.output()).await { + match timeout_at( + deadline, + crate::process_spawn::with_spawn_async(command.output()), + ) + .await + { Ok(Ok(output)) => read(output.status.code(), &output.stdout, &output.stderr), Ok(Err(error)) => { ClaimResult::Failed(format!("the bundled CLI could not be run ({error})")) diff --git a/desktop/src-tauri/src/cli_command.rs b/desktop/src-tauri/src/cli_command.rs index a89361e9fd7..7e05e54f85e 100644 --- a/desktop/src-tauri/src/cli_command.rs +++ b/desktop/src-tauri/src/cli_command.rs @@ -454,18 +454,20 @@ mod tests { } fn selected_observation_case(case: &str) { let t = Temp::new(); - let result = std::process::Command::new(std::env::current_exe().unwrap()) - .args([ - "--exact", - "cli_command::tests::selected_observation_child", - "--nocapture", - ]) - .env("OCX_CLI_OBSERVATION_TEST", case) - .env("HOME", &t.0) - .env("ZDOTDIR", &t.0) - .env("XDG_CONFIG_HOME", t.0.join(".config")) - .output() - .unwrap(); + let result = crate::process_spawn::with_spawn(|| { + std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "cli_command::tests::selected_observation_child", + "--nocapture", + ]) + .env("OCX_CLI_OBSERVATION_TEST", case) + .env("HOME", &t.0) + .env("ZDOTDIR", &t.0) + .env("XDG_CONFIG_HOME", t.0.join(".config")) + .output() + }) + .unwrap(); assert!(result.status.success(), "selected-observation-failed"); assert!( String::from_utf8_lossy(&result.stdout).contains("1 passed"), @@ -578,16 +580,18 @@ mod tests { } } fn disabled_selector_case(action: &str) { - let result = std::process::Command::new(std::env::current_exe().unwrap()) - .args([ - "--exact", - "cli_command::tests::disabled_selector_child", - "--nocapture", - ]) - .env("OCX_CLI_SELECTOR_TEST", action) - .env("ZDOTDIR", "/invalid:zdotdir") - .output() - .unwrap(); + let result = crate::process_spawn::with_spawn(|| { + std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "cli_command::tests::disabled_selector_child", + "--nocapture", + ]) + .env("OCX_CLI_SELECTOR_TEST", action) + .env("ZDOTDIR", "/invalid:zdotdir") + .output() + }) + .unwrap(); assert!(result.status.success(), "disabled-selector-failed"); assert!( String::from_utf8_lossy(&result.stdout).contains("1 passed"), diff --git a/desktop/src-tauri/src/cli_command_posix.rs b/desktop/src-tauri/src/cli_command_posix.rs index 8ab0fcf8295..b3016bd9ff4 100644 --- a/desktop/src-tauri/src/cli_command_posix.rs +++ b/desktop/src-tauri/src/cli_command_posix.rs @@ -673,6 +673,7 @@ pub fn remove_plan(store: &Store, current: &Record) -> Result<(Record, Vec Temp { @@ -701,13 +702,13 @@ mod tests { command.arg("-N"); } assert!( - command + with_spawn(|| command .arg(path) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) - .status() - .unwrap() - .success(), + .status()) + .unwrap() + .success(), "acl-command-failed" ); } @@ -969,7 +970,7 @@ mod tests { if let Some(path) = path { command.env("PATH", path); } - let output = command.output().unwrap(); + let output = with_spawn(|| command.output()).unwrap(); assert!(output.status.success()); assert_eq!(String::from_utf8(output.stdout).unwrap(), expected); } @@ -988,14 +989,16 @@ mod tests { 0o700, ) .unwrap(); - let out = std::process::Command::new(&shim) - .env_clear() - .env("ANTHROPIC_AUTH_TOKEN", "dummy") - .env("ANTHROPIC_API_KEY", "") - .env("OCX_NODE_LAUNCH_CONTEXT", "untrusted") - .arg("status") - .output() - .unwrap(); + let out = with_spawn(|| { + std::process::Command::new(&shim) + .env_clear() + .env("ANTHROPIC_AUTH_TOKEN", "dummy") + .env("ANTHROPIC_API_KEY", "") + .env("OCX_NODE_LAUNCH_CONTEXT", "untrusted") + .arg("status") + .output() + }) + .unwrap(); assert!(out.status.success()); let text = String::from_utf8(out.stdout).unwrap(); let lines: Vec<_> = text.lines().collect(); @@ -1011,9 +1014,7 @@ mod tests { assert!(!text.contains("dummy")); fs::remove_file(cli).unwrap(); assert_eq!( - std::process::Command::new(&shim) - .env_clear() - .output() + with_spawn(|| std::process::Command::new(&shim).env_clear().output()) .unwrap() .status .code(), @@ -1035,13 +1036,18 @@ mod tests { .replace("-N32", "-N1"), ] { record::atomic(&t.0.join("cli"), None, inspect, 0o700).unwrap(); - fs::write(&shim, rendered).unwrap(); - let out = std::process::Command::new(&shim) - .env_clear() - .env("OCX_NODE_LAUNCH_CONTEXT", "inherited") - .arg("status") - .output() - .unwrap(); + { + let _writer = crate::process_spawn::executable_write(); + fs::write(&shim, rendered).unwrap(); + } + let out = with_spawn(|| { + std::process::Command::new(&shim) + .env_clear() + .env("OCX_NODE_LAUNCH_CONTEXT", "inherited") + .arg("status") + .output() + }) + .unwrap(); assert!(out.status.success()); assert_eq!(out.stdout, b"\nstatus\n"); fs::remove_file(t.0.join("cli")).unwrap(); @@ -1197,15 +1203,17 @@ mod tests { // System rc files may print to stdout (Ubuntu's /etc/bash.bashrc sudo hint), so only // the marked result lines count; a failed lookup or CLI run still fails the shell, and // the end marker makes any extra CLI output line fail the comparison. - let output = command - .args([ - "-i", - "-c", - "p=$(command -v ocx) || exit 11; o=$(ocx hello) || exit 12; \ + let output = with_spawn(|| { + command + .args([ + "-i", + "-c", + "p=$(command -v ocx) || exit 11; o=$(ocx hello) || exit 12; \ printf 'OCX-PATH:%s\\nOCX-OUT:%s:OCX-END\\n' \"$p\" \"$o\"", - ]) - .output() - .expect("cannot start real shell"); + ]) + .output() + }) + .expect("cannot start real shell"); assert!(output.status.success(), "{shell}: command failed"); let stdout = String::from_utf8(output.stdout).expect("shell stdout is not UTF-8"); let marked = |tag: &str| { diff --git a/desktop/src-tauri/src/cli_command_record.rs b/desktop/src-tauri/src/cli_command_record.rs index 294c0d86346..950993a3b4f 100644 --- a/desktop/src-tauri/src/cli_command_record.rs +++ b/desktop/src-tauri/src/cli_command_record.rs @@ -548,6 +548,8 @@ fn replace(from: &Path, to: &Path) -> Result<()> { io(fs::rename(from, to)) } pub fn atomic(path: &Path, before: Option<&[u8]>, after: &[u8], mode: u32) -> Result<()> { + #[cfg(unix)] + let _executable_write = (mode & 0o111 != 0).then(crate::process_spawn::executable_write); let parent = path.parent().ok_or("unsafe-file")?; check(parent, true)?; rc_acl_check(path)?; @@ -593,6 +595,8 @@ pub fn atomic(path: &Path, before: Option<&[u8]>, after: &[u8], mode: u32) -> Re if read_bytes(path)?.as_deref() != before { return Err("concurrent-edit".into()); } + // Publish only after releasing our writer; a terminal may execute the new shim immediately. + drop(f); replace(&tmp, path)?; #[cfg(unix)] io(fs::File::open(parent).and_then(|f| f.sync_all()))?; @@ -1145,6 +1149,40 @@ impl Store { #[cfg(test)] pub(crate) mod tests { use super::*; + #[cfg(unix)] + use crate::process_spawn::with_spawn; + #[cfg(unix)] + #[test] + fn executable_writes_wait_for_process_spawns_but_record_writes_do_not() { + use std::{sync::mpsc, thread, time::Duration}; + let t = Temp::new(); + let path = t.0.join("shim"); + let (started_tx, started_rx) = mpsc::channel(); + let (done_tx, done_rx) = mpsc::channel(); + let (writer, early_completion) = with_spawn(|| { + // Non-executable record writes must still overlap the process-spawn stress test. + atomic(&t.0.join("record"), None, b"record", 0o600).unwrap(); + let path = path.clone(); + let writer = thread::spawn(move || { + started_tx.send(()).unwrap(); + let result = atomic(&path, None, b"#!/bin/sh\nexit 0\n", 0o700); + done_tx.send(()).unwrap(); + result + }); + started_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + let early_completion = done_rx.recv_timeout(Duration::from_millis(100)); + (writer, early_completion) + }); + if early_completion.is_err() { + done_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + } + writer.join().unwrap().unwrap(); + assert!(matches!( + early_completion, + Err(mpsc::RecvTimeoutError::Timeout) + )); + assert_eq!(fs::read(path).unwrap(), b"#!/bin/sh\nexit 0\n"); + } pub(crate) struct Temp(pub PathBuf); impl Temp { pub(crate) fn new() -> Self { @@ -1554,13 +1592,13 @@ pub(crate) mod tests { command.arg("-N"); } assert!( - command + with_spawn(|| command .arg(path) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) - .status() - .unwrap() - .success(), + .status()) + .unwrap() + .success(), "acl-command-failed" ); } @@ -1631,12 +1669,14 @@ pub(crate) mod tests { fn linux_acl_inheritance_clear_and_existing_child_refusal() { let t = Temp::new(); let set = |path: &Path, spec: &str| { - std::process::Command::new("setfacl") - .args(["-m", spec]) - .arg(path) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() + with_spawn(|| { + std::process::Command::new("setfacl") + .args(["-m", spec]) + .arg(path) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + }) }; let result = set(&t.0, "d:u:65534:rwx"); if result @@ -1906,7 +1946,9 @@ pub(crate) mod tests { ready.wait(); let mut spawns = 0; while !stop.load(Ordering::Relaxed) { - assert!(Command::new(TRUE).status().unwrap().success()); + assert!(with_spawn(|| Command::new(TRUE).status()) + .unwrap() + .success()); spawns += 1; } spawns diff --git a/desktop/src-tauri/src/exit.rs b/desktop/src-tauri/src/exit.rs index 8950c338638..9c3d0040881 100644 --- a/desktop/src-tauri/src/exit.rs +++ b/desktop/src-tauri/src/exit.rs @@ -535,7 +535,7 @@ async fn finish_and_exit_after(app: &AppHandle, reason: ExitReason) { match (reason, verdict) { (ExitReason::UserQuit, _) => app.exit(0), (ExitReason::CoordinatedRestart, DrainVerdict::Drained) => { - app.restart(); + crate::process_spawn::with_spawn(|| app.restart()); } (ExitReason::CoordinatedRestart, _) => { crate::logging::log_once("the update restart was refused", verdict.describe()); @@ -574,7 +574,7 @@ pub async fn prepare_restart(app: &AppHandle) -> RestartReadiness { /// Come back, once the installer has finished and returned. pub fn complete_restart(app: &AppHandle) -> ! { - app.restart() + crate::process_spawn::with_spawn(|| app.restart()) } /// Stop the runtime this app owns and confirm it is gone. diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index d4e3dec7e32..1aa62294d56 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -46,6 +46,7 @@ mod tray_login; #[allow(dead_code)] #[path = "popup.rs"] mod popup_portable_test; +mod process_spawn; mod proxy; mod resolve; mod runtime_stop; diff --git a/desktop/src-tauri/src/login_registration.rs b/desktop/src-tauri/src/login_registration.rs index 43ea2ae71e2..4adf3f78550 100644 --- a/desktop/src-tauri/src/login_registration.rs +++ b/desktop/src-tauri/src/login_registration.rs @@ -527,13 +527,16 @@ mod command { pub(super) fn run(program: &str, args: &[&str], timeout: Duration) -> Result { let slot = Slot::take().ok_or(RunError::Failed)?; let deadline = Instant::now() + timeout; - let mut child = Command::new(program) - .args(args) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|_| RunError::Failed)?; + let spawn = || { + Command::new(program) + .args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + }; + let child = crate::process_spawn::with_spawn(spawn); + let mut child = child.map_err(|_| RunError::Failed)?; let readers = [drain(child.stdout.take()), drain(child.stderr.take())]; let status = loop { match child.try_wait() { @@ -579,6 +582,29 @@ mod command { #[test] fn a_command_past_its_deadline_holds_the_only_slot_until_it_ends() { + // Measure command deadlines in a fresh process: the parent's executable-fixture + // writers may hold the test spawn lock longer than this test's timing budget. + const CHILD: &str = "OCX_LOGIN_COMMAND_DEADLINE_CHILD"; + if std::env::var_os(CHILD).is_none() { + let output = crate::process_spawn::with_spawn(|| { + Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "login_registration::command::tests::a_command_past_its_deadline_holds_the_only_slot_until_it_ends", + "--nocapture", + ]) + .env(CHILD, "1") + .output() + }) + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed")); + return; + } let started = Instant::now(); assert_eq!( run("/bin/sleep", &["1"], Duration::from_millis(50)), diff --git a/desktop/src-tauri/src/process_spawn.rs b/desktop/src-tauri/src/process_spawn.rs new file mode 100644 index 00000000000..94ad6ab5488 --- /dev/null +++ b/desktop/src-tauri/src/process_spawn.rs @@ -0,0 +1,209 @@ +//! Keep Unix forks from inheriting a writable descriptor for a soon-to-be-published executable. +//! CLOEXEC only closes it at exec; closing the parent's copy does not close a child's copy. + +// A queued writer must not block another spawner: a plugin launcher can wait for the main +// thread, where a second launcher may run. std::RwLock may prefer the queued writer and deadlock +// those two readers. Use reader preference; only an active writer excludes process creation. +#[cfg(unix)] +static EXECUTABLE_IO: Gate = Gate::new(); + +#[cfg(unix)] +struct Gate { + access: std::sync::Mutex, + changed: std::sync::Condvar, +} + +#[cfg(unix)] +struct Access { + spawners: usize, + writing: bool, +} + +#[cfg(unix)] +struct Guard<'a> { + gate: &'a Gate, + writing: bool, +} + +#[cfg(unix)] +impl Gate { + const fn new() -> Self { + Self { + access: std::sync::Mutex::new(Access { + spawners: 0, + writing: false, + }), + changed: std::sync::Condvar::new(), + } + } + + fn access(&self) -> std::sync::MutexGuard<'_, Access> { + self.access + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } + + fn acquire(&self, writing: bool) -> Guard<'_> { + let mut access = self + .changed + .wait_while(self.access(), |s| s.writing || (writing && s.spawners != 0)) + .unwrap_or_else(std::sync::PoisonError::into_inner); + if writing { + access.writing = true; + } else { + access.spawners += 1; + } + Guard { + gate: self, + writing, + } + } +} + +#[cfg(unix)] +impl Drop for Guard<'_> { + fn drop(&mut self) { + let mut access = self.gate.access(); + if self.writing { + access.writing = false; + } else { + access.spawners -= 1; + } + self.gate.changed.notify_all(); + } +} + +#[cfg(unix)] +pub(crate) fn executable_write() -> impl Drop { + EXECUTABLE_IO.acquire(true) +} + +/// Cover process creation through exec, not the subsequent wait for the child's exit. +/// Never request an executable writer from a spawner, or call this from a writer/pre_exec hook. +pub(crate) fn with_spawn(spawn: impl FnOnce() -> T) -> T { + #[cfg(unix)] + let _guard = EXECUTABLE_IO.acquire(false); + spawn() +} + +/// The pinned shell plugin's output future spawns synchronously when polled. Guard each poll +/// so creation is covered even for a lazy future; release before Pending, never across await. +pub(crate) async fn with_spawn_async(future: F) -> F::Output { + let mut future = std::pin::pin!(future); + std::future::poll_fn(|cx| with_spawn(|| future.as_mut().poll(cx))).await +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::{ + future::{poll_fn, Future}, + sync::{mpsc, Arc}, + task::{Context, Poll, Wake, Waker}, + time::Duration, + }; + + #[test] + fn a_spawn_waits_for_the_writer_and_releases_before_child_exit() { + let writer = executable_write(); + let (started_tx, started_rx) = mpsc::channel(); + let (spawned_tx, spawned_rx) = mpsc::channel(); + let spawner = std::thread::spawn(move || { + started_tx.send(()).unwrap(); + let child = with_spawn(|| { + std::process::Command::new("/bin/cat") + .stdin(std::process::Stdio::piped()) + .spawn() + }); + spawned_tx.send(child).unwrap(); + }); + started_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + let early = spawned_rx.recv_timeout(Duration::from_millis(100)); + drop(writer); + let mut child = match early { + Err(mpsc::RecvTimeoutError::Timeout) => spawned_rx + .recv_timeout(Duration::from_secs(5)) + .unwrap() + .unwrap(), + Ok(Ok(mut child)) => { + drop(child.stdin.take()); + let _ = child.wait(); + panic!("spawn overlapped an executable writer"); + } + _ => panic!("spawn failed before the writer was released"), + }; + spawner.join().unwrap(); + // cat is still waiting for EOF, but executable publication must already be unblocked. + let writer = executable_write(); + assert!(child.try_wait().unwrap().is_none()); + drop(writer); + drop(child.stdin.take()); + assert!(child.wait().unwrap().success()); + } + + #[test] + fn queued_writer_does_not_block_a_launchers_main_thread_callback() { + // An independent instance of the production gate keeps unrelated fixture/spawn stress + // from consuming this scheduling test's deadline. + let gate = Arc::new(Gate::new()); + let launcher = gate.acquire(false); + let writer_gate = gate.clone(); + let (started_tx, started_rx) = mpsc::channel(); + let (done_tx, done_rx) = mpsc::channel(); + let writer = std::thread::spawn(move || { + started_tx.send(()).unwrap(); + let _writer = writer_gate.acquire(true); + done_tx.send(()).unwrap(); + }); + started_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert!(matches!( + done_rx.recv_timeout(Duration::from_millis(100)), + Err(mpsc::RecvTimeoutError::Timeout) + )); + // Model an install helper waiting for a main-thread callback that launches a URL. + let callback = gate.acquire(false); + assert_eq!(gate.access().spawners, 2); + drop(callback); + drop(launcher); + done_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + writer.join().unwrap(); + } + + struct Noop; + impl Wake for Noop { + fn wake(self: Arc) {} + } + + #[test] + fn async_spawn_is_guarded_on_poll_without_holding_a_lock_across_pending() { + let mut polled = false; + let future = with_spawn_async(poll_fn(|_| { + assert!( + EXECUTABLE_IO.access().spawners > 0, + "async spawn poll must hold shared access" + ); + if polled { + Poll::Ready(42) + } else { + polled = true; + Poll::Pending + } + })); + fn assert_send(_: &impl Send) {} + assert_send(&future); + let mut future = std::pin::pin!(future); + let waker = Waker::from(Arc::new(Noop)); + let mut cx = Context::from_waker(&waker); + assert_eq!(future.as_mut().poll(&mut cx), Poll::Pending); + // A paused output reader cannot stop a reconcile from writing its shim. + drop(executable_write()); + assert_eq!(future.as_mut().poll(&mut cx), Poll::Ready(42)); + } + + #[test] + fn failed_spawns_release_the_guard() { + let result = with_spawn(|| std::process::Command::new("/ocx-missing-executable").spawn()); + assert!(result.is_err()); + drop(executable_write()); + } +} diff --git a/desktop/src-tauri/src/resolve.rs b/desktop/src-tauri/src/resolve.rs index 1f9ddcfdda0..7e590e53189 100644 --- a/desktop/src-tauri/src/resolve.rs +++ b/desktop/src-tauri/src/resolve.rs @@ -304,7 +304,12 @@ pub async fn run(app: &AppHandle, deadline: Instant) -> Resolution { return Resolution::Unknown(format!("the bundled CLI could not be started ({error})")) } }; - match timeout_at(deadline, command.output()).await { + match timeout_at( + deadline, + crate::process_spawn::with_spawn_async(command.output()), + ) + .await + { Ok(Ok(output)) => read(output.status.code(), &output.stdout, &output.stderr), Ok(Err(error)) => { Resolution::Unknown(format!("the bundled CLI could not be run ({error})")) diff --git a/desktop/src-tauri/src/runtime_stop.rs b/desktop/src-tauri/src/runtime_stop.rs index c92528a5f4b..664da927067 100644 --- a/desktop/src-tauri/src/runtime_stop.rs +++ b/desktop/src-tauri/src/runtime_stop.rs @@ -270,7 +270,12 @@ async fn run_with_args(app: &AppHandle, deadline: Instant, argv: Vec) -> return StopResult::Failed(format!("the bundled CLI could not be started ({error})")) } }; - match timeout_at(deadline, command.output()).await { + match timeout_at( + deadline, + crate::process_spawn::with_spawn_async(command.output()), + ) + .await + { Ok(Ok(output)) => read(output.status.code(), &output.stdout, &output.stderr), Ok(Err(error)) => StopResult::Failed(format!("the bundled CLI could not be run ({error})")), Err(_) => { diff --git a/desktop/src-tauri/src/sidecar.rs b/desktop/src-tauri/src/sidecar.rs index 71f16db8f0a..e601498fe0f 100644 --- a/desktop/src-tauri/src/sidecar.rs +++ b/desktop/src-tauri/src/sidecar.rs @@ -205,7 +205,8 @@ pub fn start( .args(["start", "--port", &endpoint.port.to_string()]) .env("OPENCODEX_GUI_DIST", gui_dist) .env(SUPERVISED_ENV, "1"); - let (events, child) = command.spawn().map_err(|error| error.to_string())?; + let (events, child) = + crate::process_spawn::with_spawn(|| command.spawn()).map_err(|error| error.to_string())?; let watch = watch.for_child(child.pid()); watch.follow(events); Ok(child) diff --git a/desktop/src-tauri/src/tray.rs b/desktop/src-tauri/src/tray.rs index c702bc7c240..ec76efdf4a7 100644 --- a/desktop/src-tauri/src/tray.rs +++ b/desktop/src-tauri/src/tray.rs @@ -224,9 +224,10 @@ pub fn install(app: &AppHandle) -> tauri::Result<()> { else { return; }; - let _ = app - .opener() - .open_url(format!("{}#/usage", endpoint.url("/")), None::); + let _ = crate::process_spawn::with_spawn(|| { + app.opener() + .open_url(format!("{}#/usage", endpoint.url("/")), None::) + }); } "start-at-login" => { #[cfg(target_os = "macos")] diff --git a/desktop/src-tauri/src/tray_login.rs b/desktop/src-tauri/src/tray_login.rs index 5126c1f44e7..3a1948ed855 100644 --- a/desktop/src-tauri/src/tray_login.rs +++ b/desktop/src-tauri/src/tray_login.rs @@ -151,10 +151,11 @@ fn perform(app: &AppHandle, action: ClickAction) { .map_err(|error| error.to_string()), ClickAction::Enable => crate::login_autostart::enable(app), // launchd keeps its override whatever the plist says; the person turns it back on. - ClickAction::OpenLoginItems => app - .opener() - .open_url(login_registration::LOGIN_ITEMS_SETTINGS, None::) - .map_err(|error| error.to_string()), + ClickAction::OpenLoginItems => crate::process_spawn::with_spawn(|| { + app.opener() + .open_url(login_registration::LOGIN_ITEMS_SETTINGS, None::) + }) + .map_err(|error| error.to_string()), ClickAction::Nothing => Ok(()), }; if let Err(error) = result { diff --git a/desktop/src-tauri/src/updater.rs b/desktop/src-tauri/src/updater.rs index bcaf3349533..cc02ae9e9fc 100644 --- a/desktop/src-tauri/src/updater.rs +++ b/desktop/src-tauri/src/updater.rs @@ -410,7 +410,7 @@ pub async fn install(app: &AppHandle, update: Update) -> Result<(), String> { )); } - if let Err(error) = update.install(package) { + if let Err(error) = crate::process_spawn::with_spawn(|| update.install(package)) { // The installer returned a failure. The runtime was stopped for an install that did not // happen, so the app brings one back instead of sitting drained. recover_after_failed_install(app); diff --git a/desktop/src-tauri/src/window.rs b/desktop/src-tauri/src/window.rs index 5c69ae17fe7..7b71614c64a 100644 --- a/desktop/src-tauri/src/window.rs +++ b/desktop/src-tauri/src/window.rs @@ -67,7 +67,9 @@ fn opens_in_default_browser(url: &Url) -> bool { pub fn open_in_default_browser(url: &Url) { if opens_in_default_browser(url) { - let _ = tauri_plugin_opener::open_url(url.as_str(), None::<&str>); + let _ = crate::process_spawn::with_spawn(|| { + tauri_plugin_opener::open_url(url.as_str(), None::<&str>) + }); } } diff --git a/structure/desktop-shell.md b/structure/desktop-shell.md index 9b86ae2b237..482aac3340a 100644 --- a/structure/desktop-shell.md +++ b/structure/desktop-shell.md @@ -55,6 +55,7 @@ a page with an overlay title bar and no strip cannot be dragged or zoomed at all Linux keep the native title bar: the shell ships no min/max/close widgets of its own, and the sidebar-top layout applies unchanged beneath it. +Process creation shares the Unix guard with executable publication; see [Desktop-owned terminal command](desktop-terminal-command.md). ## Startup, quit and the tray The window is created and shown before anything is registered, resolved, probed or started, and diff --git a/structure/desktop-terminal-command.md b/structure/desktop-terminal-command.md index 2003a27bd53..d4159c3ef0b 100644 --- a/structure/desktop-terminal-command.md +++ b/structure/desktop-terminal-command.md @@ -21,6 +21,22 @@ disabled intent wins over pending work. Invalid records and journal conflicts bl unreferenced journals are deleted only under the lock after a valid (or absent) record read. Every private path must be user-owned without extended ACLs (macOS/Linux) or carry a protected user/SYSTEM/Administrators DACL (Windows root; children inherit it), otherwise nothing is written. +Atomic replacement flushes and closes its write handle before publishing the new file by rename, +so the published executable is never held open for writing by the replacing thread. +`desktop/src-tauri/src/process_spawn.rs` owns a production process-wide Unix read/write gate. +Executable atomic writes take it exclusively before opening the temporary file and release it after +closing and publishing; ordinary record and rc writes do not take it. Process creation takes a shared +guard through exec, so no child inherits the writable executable descriptor. Closing only the parent's +close-on-exec descriptor would leave a forked duplicate writable until that child's exec. +The same guard covers runtime start (`sidecar.rs`), resolve, claim and stop output futures, macOS +launchctl/plutil probes (`login_registration.rs`), browser and Login Items launchers, updater install +helpers and app restart. Shell-plugin output futures take the guard while being polled, including the +initial synchronous spawn, and release it before yielding; waiting for output never retains a guard. +The configured LaunchAgent registration and Linux autostart write files without spawning commands. +Windows uses the same call sites without a lock. Rust fixture writers and spawners use this production +mechanism too; tests cover exclusion in both directions, spawn errors and async Pending release. +A queued writer permits further spawns until existing launchers finish, avoiding deadlock when a plugin +waits for a main-thread callback that launches another process. Only an active writer blocks spawns. `desktop/src-tauri/src/cli_command_posix.rs` generates a fail-closed shim, shared PATH helper and final managed zsh/bash/fish blocks. Reconcile separates desired rc targets from recorded diff --git a/tests/clients/desktop-cli-contracts.test.ts b/tests/clients/desktop-cli-contracts.test.ts index a4934212d7a..edf9c884c9f 100644 --- a/tests/clients/desktop-cli-contracts.test.ts +++ b/tests/clients/desktop-cli-contracts.test.ts @@ -108,7 +108,7 @@ describe("desktop CLI contracts", () => { const body = runner.slice(0, runner.indexOf("\n}")); // A missing binary, a failed spawn and a deadline all answer the same way. expect(body.match(/Resolution::Unknown/g) || []).toHaveLength(3); - expect(body).toContain("timeout_at(deadline, command.output())"); + expect(body).toContain("crate::process_spawn::with_spawn_async(command.output())"); }); test("the startup sequence refuses to start on anything but a proven absence", () => { diff --git a/tests/clients/desktop-startup-surface.test.ts b/tests/clients/desktop-startup-surface.test.ts index 76cc454bb2a..41f869cae95 100644 --- a/tests/clients/desktop-startup-surface.test.ts +++ b/tests/clients/desktop-startup-surface.test.ts @@ -85,7 +85,7 @@ describe("desktop startup surface", () => { test("the spawn event stream is consumed rather than discarded", () => { const sidecar = code(SIDECAR); expect(sidecar).not.toContain("_events"); - expect(sidecar).toContain("let (events, child) = command.spawn()"); + expect(sidecar).toMatch(/let \(events, child\) =\s*crate::process_spawn::with_spawn\(\|\| command.spawn\(\)\)/); expect(sidecar).toContain("watch.follow(events)"); expect(sidecar).toContain("CommandEvent::Terminated(payload)"); }); @@ -121,7 +121,7 @@ describe("desktop startup surface", () => { expect(proxy).toContain("timeout_at(deadline, self.is_alive())"); // The stop is the bundled CLI's now, under its own deadline. const stop = code(repoPath("desktop/src-tauri/src/runtime_stop.rs")); - expect(stop).toContain("timeout_at(deadline, command.output())"); + expect(stop).toContain("crate::process_spawn::with_spawn_async(command.output())"); expect(stop).toContain("pub const DEADLINE: Duration"); });