From 5d04dcb812495c45c5d52c3fdfd51eda03502af6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 22:48:54 -0800 Subject: [PATCH 1/6] chore(quest): claim quest/m0/browser-helper-exit.md Quest-Claim: 327987dd2ac055a2 From c2d1747d4f8d6cae5e5b81fdff1ffde9e68c3b02 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 23:37:51 -0800 Subject: [PATCH 2/6] fix(scene): a browser helper exits with the studio, even while CEF starts The helper read the studio's socket only once CEF's initialize returned, so a helper stuck there (a Wayland display that never answers) never heard the studio die and kept running with its zygotes. The helper now reads from before CEF starts, the studio stops it with an explicit Stop and holds the socket open until it exits, and the socket's end without one makes the helper kill the process group it leads, CEF's processes included. Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 1 - quest/m0/browser-helper-exit.md | 24 ----- quest/m0/browser-source.md | 6 ++ rs/moq-scene/src/browser/host.rs | 155 +++++++++++++++++++-------- rs/moq-scene/src/browser/outbox.rs | 16 ++- rs/moq-scene/src/browser/protocol.rs | 4 + rs/moq-scene/src/browser/run.rs | 16 +-- rs/moq-studio/tests/browser.rs | 125 +++++++++++++++++++-- 8 files changed, 255 insertions(+), 92 deletions(-) delete mode 100644 quest/m0/browser-helper-exit.md diff --git a/quest/m0/README.md b/quest/m0/README.md index c4f42cf0..24a6c67a 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -101,7 +101,6 @@ repository. - [Virtual camera](/quest/m0/vcam/README.md) - the program or a published scene exposed as a system camera on Linux, Windows, and macOS - [Browser sources on the GPU under Mesa](/quest/m0/cef-headless.md) - shared-texture pages paint on Intel and AMD under Wayland, and a page that cannot paint fails loudly - [Browser sources on the GPU under X11 on NVIDIA](/quest/m0/cef-x11-nvidia.md) - the X11 GPU path paints on NVIDIA, or falls back to the CPU path loudly -- [A browser helper exits with the studio](/quest/m0/browser-helper-exit.md) - no orphaned CEF helper after a crash, kill, or timed-out test - [Nightly and the browser suite pass](/quest/m0/nightly-green.md) - render-sw, cookies, and the X11 clipboard test fixed at their causes - [GPU tests exit cleanly on NVIDIA](/quest/m0/gpu-tests-exit.md) - compositor tests no longer crash on exit on NVIDIA - [The Apple signing identity is available](/quest/m0/signing.md) - the Apple credentials packaging and the macOS camera need; Windows ships unsigned diff --git a/quest/m0/browser-helper-exit.md b/quest/m0/browser-helper-exit.md deleted file mode 100644 index 335993ad..00000000 --- a/quest/m0/browser-helper-exit.md +++ /dev/null @@ -1,24 +0,0 @@ -# [S] A browser helper exits with the studio - -## Goal - -When the studio process dies for any reason (a crash, a kill, or a test that -times out), its CEF browser helper and the helper's children exit too, on -Linux, macOS, and Windows. No orphaned helper keeps running against the -desktop. - -## Plan - -From [moq-dev/studio#73](https://github.com/moq-dev/studio/pull/73), picked -2026-11-07 (maintainer, /quest-complete), m0 by the rule that fixes join m0. - -- The helper runs in its own process group - (`rs/moq-scene/src/browser/run.rs`), which is killed only when the studio - drops it in an orderly way. On nixos2, three test runs that timed out left - helpers spinning against GNOME until they were killed by hand. -- Reproduce first: kill the studio while a page is open and see what the - helper does. Its channel to the studio closes then; find why that does not - end it. -- Fix the cause in a way that holds on every desktop, such as the helper - exiting when that channel closes. Linux's `PR_SET_PDEATHSIG` alone would - leave macOS and Windows uncovered. diff --git a/quest/m0/browser-source.md b/quest/m0/browser-source.md index 7d6578da..7c4c5247 100644 --- a/quest/m0/browser-source.md +++ b/quest/m0/browser-source.md @@ -35,6 +35,12 @@ directory instead. What the other platforms need on top: with CEF's helper apps (`cef-rs`'s `bundle-cef-app`) and its own NSApplication. An IOSurface crosses processes only as a Mach port (`IOSurfaceCreateMachPort`), so the channel needs a Mach port rendezvous. +- **Exiting with the studio:** the studio holds the channel open until the + helper exits and stops it with `Command::Stop`, so the helper reads the + channel's end, from before CEF starts, as the studio dying, and kills the + process group it leads with CEF's processes. macOS keeps that as is. + Windows has no process groups: start the helper in a job object with + `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, which the studio's handle keeps alive. ## Required diff --git a/rs/moq-scene/src/browser/host.rs b/rs/moq-scene/src/browser/host.rs index 378812f9..7fb581ef 100644 --- a/rs/moq-scene/src/browser/host.rs +++ b/rs/moq-scene/src/browser/host.rs @@ -11,7 +11,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, mpsc}; use cef::*; -use rustix::process::{Resource, Rlimit, getrlimit, setrlimit}; +use rustix::process::{ + Resource, Rlimit, Signal, getpgrp, getpid, getrlimit, kill_current_process_group, setrlimit, +}; use super::outbox::Commands; use super::protocol::{Area, Channel, Command, Display, Event, Format, Id, Layer, Open, Plane, Raster, Start, Texture}; @@ -32,7 +34,7 @@ pub(super) fn subprocess() -> ExitCode { ExitCode::from(code.clamp(0, 255) as u8) } -/// Run the helper until the studio closes the socket or the page fails. +/// Run the helper until the studio stops it or the page fails. pub(super) fn run() -> ExitCode { match serve() { Ok(()) => ExitCode::SUCCESS, @@ -87,7 +89,7 @@ impl Paged { thread_local! { /// The open pages, on CEF's UI thread, where input is delivered. static PAGES: RefCell)>> = RefCell::new(HashMap::new()); - /// Whether the studio is gone, so the helper quits once its last page + /// Whether the studio stopped the helper, so it quits once its last page /// closes. static QUITTING: Cell = const { Cell::new(false) }; /// The number of the last copy asked of a renderer. @@ -131,8 +133,14 @@ fn serve() -> Result<(), String> { } /// Run CEF on the profile `start` names, opening and closing pages as the -/// studio asks, until it closes the socket and the last page closes. +/// studio asks, until it stops the helper and the last page closes. fn browse(shared: &Arc, start: Start, reader: Channel, painted_tx: mpsc::Sender<()>) -> Result<(), String> { + // The helper ends CEF's processes along with itself by killing the group + // it leads, which they join, and never the studio's. + if getpgrp() != getpid() { + return Err("the helper does not lead its own process group".into()); + } + // A crashed process is reported only once it is gone, and one dumping // core is not gone yet: systemd-coredump took 20 to 70 seconds over a // renderer, which left its page Starting meanwhile. CEF's subprocesses @@ -151,6 +159,21 @@ fn browse(shared: &Arc, start: Start, reader: Channel, painted_tx: mpsc: ) .map_err(|err| format!("cannot turn off core dumps: {err}"))?; + // Read before CEF starts, which can block for good, as on a display that + // never answers, so the studio's end is heard even then. + let inbox = Arc::new(Inbox { + pending: Commands::default(), + posted: AtomicBool::new(true), + up: AtomicBool::new(false), + }); + { + let (shared, inbox) = (shared.clone(), inbox.clone()); + std::thread::Builder::new() + .name("browser-commands".into()) + .spawn(move || commands(shared, reader, painted_tx, inbox)) + .map_err(|err| format!("cannot start the helper's command thread: {err}"))?; + } + let _ = api_hash(sys::CEF_API_VERSION_LAST, 0); let args = args::Args::new(); let mut app = HelperApp::new(HelperProcess::new(), start.raster); @@ -186,17 +209,9 @@ fn browse(shared: &Arc, start: Start, reader: Channel, painted_tx: mpsc: }); } - let commands = { - let shared = shared.clone(); - std::thread::Builder::new() - .name("browser-commands".into()) - .spawn(move || commands(shared, reader, painted_tx)) - }; - if let Err(err) = commands { - shutdown(); - return Err(format!("cannot start the helper's command thread: {err}")); - } - + // Pages asked for meanwhile open now. + inbox.up.store(true, Ordering::Release); + inbox.post(shared); run_message_loop(); PAGES.with_borrow_mut(HashMap::clear); // Saves the profile, such as cookies not yet written. @@ -204,44 +219,73 @@ fn browse(shared: &Arc, start: Start, reader: Channel, painted_tx: mpsc: Ok(()) } -/// Read the studio's commands until it closes the socket, then close every +/// The studio's commands the UI thread has yet to apply. +/// +/// They wait coalesced, as in the studio, with one task at a time posted to +/// apply them, so a stalled UI thread holds what the pages open need rather +/// than a task per command. Input past the backlog drops, as for a hung page. +struct Inbox { + pending: Commands, + /// Whether a task applying `pending` is posted, or CEF is not up to run + /// one. + posted: AtomicBool, + /// Whether CEF is up. + up: AtomicBool, +} + +impl Inbox { + /// Have the UI thread apply what is pending, unless a task already will. + fn schedule(self: &Arc, shared: &Arc) { + if !self.posted.swap(true, Ordering::AcqRel) { + self.post(shared); + } + } + + /// Post a task applying what is pending. + fn post(self: &Arc, shared: &Arc) { + let (inbox, shared) = (self.clone(), shared.clone()); + post(move || { + // Cleared first, so anything queued from here on posts again. + inbox.posted.store(false, Ordering::Release); + apply(&shared, inbox.pending.take()); + }); + } +} + +/// Read the studio's commands until it stops the helper, then close every /// page, which ends the message loop. /// -/// What the UI thread has yet to apply waits coalesced, as in the studio, with -/// one task at a time posted to apply it, so a stalled UI thread holds what -/// the pages open need rather than a task per command. Input past the -/// backlog drops, as for a hung page. -fn commands(shared: Arc, reader: Channel, painted: mpsc::Sender<()>) { - let pending = Commands::default(); - let posted = Arc::new(AtomicBool::new(false)); - let schedule = || { - if !posted.swap(true, Ordering::AcqRel) { - let (shared, pending, posted) = (shared.clone(), pending.clone(), posted.clone()); - post(move || { - // Cleared first, so anything queued from here on posts again. - posted.store(false, Ordering::Release); - apply(&shared, pending.take()); - }); - } - }; +/// The studio holds its end of the socket until the helper has exited, so +/// the socket ends early only once the studio is gone, crashed or killed. +/// Nobody is left then to show a page to or to wait for the profile, so the +/// helper exits at once with every process CEF started, wherever CEF is. +fn commands(shared: Arc, reader: Channel, painted: mpsc::Sender<()>, inbox: Arc) { loop { - match reader.recv::() { - Ok(Some((Command::Painted, _))) => { + match recv(&reader) { + Command::Painted => { let _ = painted.send(()); continue; } - Ok(Some((Command::Open(id, open), _))) => pending.open(id, open), - Ok(Some((Command::Close(id), _))) => pending.close(id), - Ok(Some((Command::Input(id, input), _))) => _ = pending.input(id, input), - Ok(Some((Command::FrameRate(id, rate), _))) => _ = pending.rate(id, rate), - Ok(Some((Command::Start(_), _))) | Ok(None) | Err(_) => break, + Command::Open(id, open) => inbox.pending.open(id, open), + Command::Close(id) => inbox.pending.close(id), + Command::Input(id, input) => _ = inbox.pending.input(id, input), + Command::FrameRate(id, rate) => _ = inbox.pending.rate(id, rate), + Command::Stop => break, + Command::Start(_) => { + eprintln!("browser source helper: a second Start"); + exit_now(); + } } - schedule(); + inbox.schedule(&shared); } - // Unblocks a paint waiting on the studio. + // No page opens before CEF is up, so nothing waits to be saved. + if !inbox.up.load(Ordering::Acquire) { + exit_now(); + } + // Unblocks a paint waiting on the studio, which answers no more. drop(painted); post(move || { - apply(&shared, pending.take()); + apply(&shared, inbox.pending.take()); QUITTING.set(true); let ids: Vec = PAGES.with_borrow(|pages| pages.keys().copied().collect()); if ids.is_empty() { @@ -251,6 +295,29 @@ fn commands(shared: Arc, reader: Channel, painted: mpsc::Sender<()>) { close(id); } }); + let command = recv(&reader); + eprintln!("browser source helper: {command:?} after Stop"); + exit_now(); +} + +/// The studio's next command, or [`exit_now`] once the socket ends. +fn recv(reader: &Channel) -> Command { + use std::io::ErrorKind::*; + + match reader.recv::() { + Ok(Some((command, _))) => return command, + Ok(None) => {} + Err(err) if matches!(err.kind(), BrokenPipe | ConnectionReset | UnexpectedEof) => {} + Err(err) => eprintln!("browser source helper: the studio's socket: {err}"), + } + exit_now() +} + +/// End the helper and every process CEF started, at once. +fn exit_now() -> ! { + // The helper leads the group, so this ends it before returning. + let _ = kill_current_process_group(Signal::KILL); + std::process::abort() } /// Apply the studio's commands. Runs on the UI thread. @@ -261,7 +328,7 @@ fn apply(shared: &Arc, batch: Vec) { Command::Close(id) => close(id), Command::Input(id, input) => deliver(id, input), Command::FrameRate(id, rate) => retime(id, rate), - Command::Start(_) | Command::Painted => unreachable!("never pending"), + Command::Start(_) | Command::Painted | Command::Stop => unreachable!("never pending"), } } } diff --git a/rs/moq-scene/src/browser/outbox.rs b/rs/moq-scene/src/browser/outbox.rs index 519d5df7..5b9d1642 100644 --- a/rs/moq-scene/src/browser/outbox.rs +++ b/rs/moq-scene/src/browser/outbox.rs @@ -73,6 +73,8 @@ struct Outbox { painted: usize, /// Open pages to close. closes: Vec, + /// Whether the helper is to exit, after everything else. + quit: bool, /// Whether the writer stopped, so nothing more is taken. stopped: bool, } @@ -142,6 +144,12 @@ impl Commands { Ok(()) } + /// Have the helper close every page and exit, after what is queued, and + /// take nothing more. + pub(crate) fn quit(&self) { + self.push(|outbox| outbox.quit = true); + } + /// Stop taking commands, so [`next`](Self::next) returns `None`. pub(crate) fn stop(&self) { let (outbox, ready) = &*self.0; @@ -175,6 +183,7 @@ impl Commands { && outbox.owed.is_empty() && outbox.painted == 0 && outbox.closes.is_empty() + && !outbox.quit } /// The commands waiting, without waiting for any. @@ -185,7 +194,8 @@ impl Commands { } impl Outbox { - /// Everything waiting, each page's open ahead of anything else for it. + /// Everything waiting, each page's open ahead of anything else for it, + /// and the helper's exit last. fn take(&mut self) -> Vec { let mut batch = Vec::new(); for (id, open) in self.opens.drain(..) { @@ -200,6 +210,10 @@ impl Outbox { self.open.remove(&id); batch.push(Command::Close(id)); } + if self.quit { + batch.push(Command::Stop); + self.stopped = true; + } batch } } diff --git a/rs/moq-scene/src/browser/protocol.rs b/rs/moq-scene/src/browser/protocol.rs index c46e4da5..06961e11 100644 --- a/rs/moq-scene/src/browser/protocol.rs +++ b/rs/moq-scene/src/browser/protocol.rs @@ -45,6 +45,10 @@ pub(crate) enum Command { Input(Id, Input), /// The canvas's frame rate changed, so the page paints at this one. FrameRate(Id, u32), + /// The last message: close every page, save the profile, and exit. The + /// studio holds the socket open until then, so the helper reads its end + /// as the studio dying and exits at once. + Stop, } /// How a helper runs every page it opens. diff --git a/rs/moq-scene/src/browser/run.rs b/rs/moq-scene/src/browser/run.rs index c242580a..20efade9 100644 --- a/rs/moq-scene/src/browser/run.rs +++ b/rs/moq-scene/src/browser/run.rs @@ -91,8 +91,6 @@ pub(crate) struct Instance { routes: Arc>, /// The next page's number. next: AtomicU64, - /// The socket, shut down to stop the helper. - socket: Channel, /// The thread delivering the helper's events, which ends once the helper /// has exited. reader: Mutex>>, @@ -207,8 +205,7 @@ impl Instance { let (process, events) = spawn(helper).map_err(|err| failed(&format!("cannot start {}", helper.program.display()), err))?; - let socket = events.try_clone().map_err(|err| failed("socket", err))?; - let commands = socket.try_clone().map_err(|err| failed("socket", err))?; + let commands = events.try_clone().map_err(|err| failed("socket", err))?; commands .send(&Command::Start(start), &[]) .map_err(|err| failed("the browser process's socket", err))?; @@ -233,7 +230,6 @@ impl Instance { commands, routes, next: AtomicU64::new(0), - socket, reader: Mutex::new(Some(reader)), }) } @@ -282,7 +278,7 @@ impl Instance { /// Ask the helper to exit, which it does once it has closed every page. fn end(&self) { - let _ = rustix::net::shutdown(&self.socket, rustix::net::Shutdown::Write); + self.commands.quit(); } } @@ -350,8 +346,9 @@ impl Drop for Process { /// Write the commands queued on the returned handle to `channel`, on a thread /// of its own, so a helper that stops reading blocks only that thread, never a /// sender holding the studio's state. The thread ends once the handle is -/// stopped or a write fails, as one does when the helper exits, and ends the -/// socket on its way out. +/// stopped or quit, or a write fails, as one does when the helper exits. +/// +/// It leaves the socket open: the helper reads its end as the studio dying. fn writer(channel: Channel) -> std::io::Result { let commands = Commands::default(); let outbox = commands.clone(); @@ -365,10 +362,7 @@ fn writer(channel: Channel) -> std::io::Result { } } } - // The helper exits once it reads the end, so one the studio can no - // longer reach never keeps running unheard. outbox.stop(); - let _ = rustix::net::shutdown(&channel, rustix::net::Shutdown::Write); })?; Ok(commands) } diff --git a/rs/moq-studio/tests/browser.rs b/rs/moq-studio/tests/browser.rs index e35729cc..fa9ae3a2 100644 --- a/rs/moq-studio/tests/browser.rs +++ b/rs/moq-studio/tests/browser.rs @@ -446,7 +446,7 @@ async fn a_crash_while_loading_drops_only_its_source() { assert!(reason.contains("renderer"), "{reason}"); studio.until_center([255, 0, 0, 255], &["renderer"]).await; - let helper = helper().expect("the page's helper is running"); + let helper = helper(std::process::id()).expect("the page's helper is running"); let killed = std::process::Command::new("kill") .args(["-KILL", &helper.to_string()]) .status(); @@ -476,7 +476,7 @@ async fn a_crash_after_drawing_leaves_the_canvas() { // Abort the page's renderer, and any spare one CEF keeps ready, which // shows no page, as a failed assertion would: a crash that dumps core. // (V8 takes a SIGSEGV that no fault raised and carries on.) - let helper = helper().expect("the page's helper is running"); + let helper = helper(std::process::id()).expect("the page's helper is running"); let renderers = renderers(helper); assert!(!renderers.is_empty(), "the page has a renderer"); for renderer in renderers { @@ -571,13 +571,105 @@ async fn serve(html: &'static str) -> String { url } -/// The one helper this test process still runs: a child started with the -/// helper's argument. -fn helper() -> Option { - let ours = std::process::id(); +/// Where [`a_studio_that_dies`] keeps its pages' profile, which also tells it +/// to run. +const DYING: &str = "MOQ_STUDIO_TEST_DYING_PROFILE"; + +/// A helper exits, with every process CEF started, once its studio dies +/// without stopping it, as a crashed studio or a timed-out test does. That +/// holds even while CEF is still starting, which can last for good, as on a +/// display that never answers. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "needs CEF and a Vulkan device; run by just render-sw"] +async fn a_helper_exits_with_its_studio() { + let profile = tempfile::tempdir().unwrap(); + let _stall = stall(profile.path()); + let mut studio = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "a_studio_that_dies", "--ignored"]) + .env(DYING, profile.path()) + .stdout(std::process::Stdio::null()) + .spawn() + .unwrap(); + let helper = until_stalled(studio.id(), profile.path()).await; + studio.kill().unwrap(); + studio.wait().unwrap(); + + // The helper leads a process group CEF's processes join. + let deadline = Instant::now() + TIMEOUT; + while processes().iter().any(|process| process.group == helper) { + assert!(Instant::now() < deadline, "the helper outlived its studio"); + kio::clock::sleep(Duration::from_millis(30)).await; + } +} + +/// Not a test alone: the studio [`a_helper_exits_with_its_studio`] starts and +/// kills, which opens a page and waits. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "run by a_helper_exits_with_its_studio"] +async fn a_studio_that_dies() { + let Some(profile) = std::env::var_os(DYING) else { + return; + }; + local(async { + let _studio = Studio::on(Path::new(&profile), &[("page", page(""))]).await; + std::future::pending::<()>().await; + }) + .await; +} + +/// A studio stops a helper still starting CEF without waiting for it to +/// finish starting, which can take forever. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "needs CEF and a Vulkan device; run by just render-sw"] +async fn a_helper_stops_while_starting() { + local(async { + let profile = tempfile::tempdir().unwrap(); + let _stall = stall(profile.path()); + let studio = Studio::on(profile.path(), &[("page", page(""))]).await; + until_stalled(std::process::id(), profile.path()).await; + drop(studio); + }) + .await; +} + +/// Leave CEF stuck starting on `profile` while the returned file is open: it +/// reads the profile's `Local State` before `initialize` returns, and a FIFO +/// with a writer and nothing written blocks that read. +fn stall(profile: &Path) -> std::fs::File { + let state = profile.join("Local State"); + let made = std::process::Command::new("mkfifo").arg(&state).status().unwrap(); + assert!(made.success()); + // Read and write, which Linux opens without waiting for a reader. + std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(state) + .unwrap() +} + +/// Wait until `studio`'s helper is stuck reading [`stall`]'s FIFO in +/// `profile`, and return the helper. +async fn until_stalled(studio: u32, profile: &Path) -> u32 { + let state = std::fs::canonicalize(profile).unwrap().join("Local State"); + let deadline = Instant::now() + TIMEOUT; + loop { + if let Some(helper) = helper(studio) { + let fds = std::fs::read_dir(format!("/proc/{helper}/fd")).into_iter().flatten().flatten(); + if fds.filter_map(|fd| std::fs::read_link(fd.path()).ok()).any(|path| path == state) { + return helper; + } + } + assert!(Instant::now() < deadline, "no helper reads {}", state.display()); + kio::clock::sleep(Duration::from_millis(30)).await; + } +} + +/// The one helper `studio` still runs: a child started with the helper's +/// argument. +fn helper(studio: u32) -> Option { processes() .into_iter() - .find(|process| process.parent == ours && process.args.iter().any(|arg| arg == b"--moq-browser-helper")) + .find(|process| process.parent == studio && process.args.iter().any(|arg| arg == b"--moq-browser-helper")) .map(|process| process.pid) } @@ -614,10 +706,11 @@ fn renderers(helper: u32) -> Vec { struct Process { pid: u32, parent: u32, + group: u32, args: Vec>, } -/// Every process running. +/// Every process running, leaving out the dead ones waiting to be reaped. fn processes() -> Vec { let Ok(entries) = std::fs::read_dir("/proc") else { return Vec::new(); @@ -627,11 +720,21 @@ fn processes() -> Vec { .filter_map(|entry| { let pid = entry.file_name().to_str()?.parse().ok()?; let stat = std::fs::read_to_string(entry.path().join("stat")).ok()?; - // The parent is the second field after the parenthesized command. - let parent = stat.rsplit_once(')')?.1.split_whitespace().nth(1)?.parse().ok()?; + // The state, parent, and group follow the parenthesized command. + let mut fields = stat.rsplit_once(')')?.1.split_whitespace(); + if fields.next()? == "Z" { + return None; + } + let parent = fields.next()?.parse().ok()?; + let group = fields.next()?.parse().ok()?; let cmdline = std::fs::read(entry.path().join("cmdline")).ok()?; let args = cmdline.split(|byte| *byte == 0).map(<[u8]>::to_vec).collect(); - Some(Process { pid, parent, args }) + Some(Process { + pid, + parent, + group, + args, + }) }) .collect() } From c72f86c107cc637c1375c86c8ab9413859e7f26d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 23:39:26 -0800 Subject: [PATCH 3/6] refactor(scene): signal the helper's own group by its id, with no leader check Co-Authored-By: Claude Opus 5.5 --- rs/moq-scene/src/browser/host.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/rs/moq-scene/src/browser/host.rs b/rs/moq-scene/src/browser/host.rs index 1e4c9450..442a1b71 100644 --- a/rs/moq-scene/src/browser/host.rs +++ b/rs/moq-scene/src/browser/host.rs @@ -11,9 +11,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, mpsc}; use cef::*; -use rustix::process::{ - Resource, Rlimit, Signal, getpgrp, getpid, getrlimit, kill_current_process_group, setrlimit, -}; +use rustix::process::{Resource, Rlimit, Signal, getpid, getrlimit, kill_process_group, setrlimit}; use super::outbox::Commands; use super::protocol::{Area, Channel, Command, Display, Event, Format, Id, Layer, Open, Plane, Raster, Start, Texture}; @@ -135,12 +133,6 @@ fn serve() -> Result<(), String> { /// Run CEF on the profile `start` names, opening and closing pages as the /// studio asks, until it stops the helper and the last page closes. fn browse(shared: &Arc, start: Start, reader: Channel, painted_tx: mpsc::Sender<()>) -> Result<(), String> { - // The helper ends CEF's processes along with itself by killing the group - // it leads, which they join, and never the studio's. - if getpgrp() != getpid() { - return Err("the helper does not lead its own process group".into()); - } - // A crashed process is reported only once it is gone, and one dumping // core is not gone yet: systemd-coredump took 20 to 70 seconds over a // renderer, which left its page Starting meanwhile. CEF's subprocesses @@ -315,8 +307,10 @@ fn recv(reader: &Channel) -> Command { /// End the helper and every process CEF started, at once. fn exit_now() -> ! { - // The helper leads the group, so this ends it before returning. - let _ = kill_current_process_group(Signal::KILL); + // The studio starts the helper leading a group of its own, which CEF's + // processes join; a group with the helper's id is only ever that one. + // The kill ends the helper too, before returning. + let _ = kill_process_group(getpid(), Signal::KILL); std::process::abort() } From e3c3608b9ae66b8afa3ba13e2e09bf7d22bd23f2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 23:41:00 -0800 Subject: [PATCH 4/6] style: cargo fmt Co-Authored-By: Claude Opus 5.5 --- rs/moq-studio/tests/browser.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/rs/moq-studio/tests/browser.rs b/rs/moq-studio/tests/browser.rs index 5e5ee7e7..d48f10e0 100644 --- a/rs/moq-studio/tests/browser.rs +++ b/rs/moq-studio/tests/browser.rs @@ -682,11 +682,7 @@ fn stall(profile: &Path) -> std::fs::File { let made = std::process::Command::new("mkfifo").arg(&state).status().unwrap(); assert!(made.success()); // Read and write, which Linux opens without waiting for a reader. - std::fs::OpenOptions::new() - .read(true) - .write(true) - .open(state) - .unwrap() + std::fs::OpenOptions::new().read(true).write(true).open(state).unwrap() } /// Wait until `studio`'s helper is stuck reading [`stall`]'s FIFO in @@ -696,8 +692,14 @@ async fn until_stalled(studio: u32, profile: &Path) -> u32 { let deadline = Instant::now() + TIMEOUT; loop { if let Some(helper) = helper(studio) { - let fds = std::fs::read_dir(format!("/proc/{helper}/fd")).into_iter().flatten().flatten(); - if fds.filter_map(|fd| std::fs::read_link(fd.path()).ok()).any(|path| path == state) { + let fds = std::fs::read_dir(format!("/proc/{helper}/fd")) + .into_iter() + .flatten() + .flatten(); + if fds + .filter_map(|fd| std::fs::read_link(fd.path()).ok()) + .any(|path| path == state) + { return helper; } } From e5879b42f1b0bcc38136c96d15e1e45d5e13f0c4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 6 Nov 2026 23:51:39 -0800 Subject: [PATCH 5/6] fix(scene): a command the studio cannot send still ends the helper Co-Authored-By: Claude Opus 5.5 --- rs/moq-scene/src/browser/protocol.rs | 2 +- rs/moq-scene/src/browser/run.rs | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/rs/moq-scene/src/browser/protocol.rs b/rs/moq-scene/src/browser/protocol.rs index 9dc22bac..82d3883d 100644 --- a/rs/moq-scene/src/browser/protocol.rs +++ b/rs/moq-scene/src/browser/protocol.rs @@ -25,7 +25,7 @@ pub(crate) const MAX_PLANES: usize = 4; /// The longest message either side accepts: the largest page's pixels, with /// room for the rest of the message. Bounds what a misbehaving peer can make /// the other allocate. -const MAX_MESSAGE: usize = (super::MAX_SIDE * super::MAX_SIDE * 4) as usize + 1024; +pub(crate) const MAX_MESSAGE: usize = (super::MAX_SIDE * super::MAX_SIDE * 4) as usize + 1024; /// A page's number, unique among those one helper opens. pub(crate) type Id = u64; diff --git a/rs/moq-scene/src/browser/run.rs b/rs/moq-scene/src/browser/run.rs index e2336c1d..83d0300f 100644 --- a/rs/moq-scene/src/browser/run.rs +++ b/rs/moq-scene/src/browser/run.rs @@ -354,7 +354,8 @@ impl Drop for Process { /// sender holding the studio's state. The thread ends once the handle is /// stopped or quit, or a write fails, as one does when the helper exits. /// -/// It leaves the socket open: the helper reads its end as the studio dying. +/// Once quit it leaves the socket open, since the helper reads its end as the +/// studio dying, and exits at once. fn writer(channel: Channel) -> std::io::Result { let commands = Commands::default(); let outbox = commands.clone(); @@ -364,6 +365,10 @@ fn writer(channel: Channel) -> std::io::Result { 'write: while let Some(batch) = outbox.next() { for command in batch { if channel.send(&command, &[]).is_err() { + // Such as a command too long to send: a helper the + // studio can no longer reach never keeps running + // unheard. + let _ = rustix::net::shutdown(&channel, rustix::net::Shutdown::Write); break 'write; } } @@ -480,6 +485,23 @@ mod tests { use super::*; use crate::browser::Input; + /// A command the writer cannot send ends the socket, which the helper + /// reads as the studio gone, rather than leave it waiting unheard. + #[test] + fn a_command_too_long_to_send_ends_the_socket() { + let (ours, theirs) = UnixStream::pair().unwrap(); + let ours = Channel::new(ours); + // Held open, as the studio's reader does. + let _events = ours.try_clone().unwrap(); + let commands = writer(ours).unwrap(); + let long = Open { + url: "x".repeat(crate::browser::protocol::MAX_MESSAGE), + ..Default::default() + }; + commands.open(0, long); + assert!(Channel::new(theirs).recv::().unwrap().is_none()); + } + /// A helper that stops reading costs dropped input, never a sender stuck /// on a full socket while it holds the studio's state. Pages still open /// and close without waiting, and reloads meanwhile leave only the newest From 525012398a97912a27d8b6fc8b26a6ea8a11214d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 00:14:01 -0800 Subject: [PATCH 6/6] fix(scene): no page opens once the helper is told to exit A page opened after quit was sent ahead of Stop, so the helper loaded it only to close it. The regression test also kills its child studio when it fails, rather than leave it waiting forever. Co-Authored-By: Claude Opus 5.5 --- rs/moq-scene/src/browser/outbox.rs | 20 +++++++++++++++++--- rs/moq-scene/src/browser/run.rs | 5 +++-- rs/moq-studio/tests/browser.rs | 29 ++++++++++++++++++++--------- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/rs/moq-scene/src/browser/outbox.rs b/rs/moq-scene/src/browser/outbox.rs index 5b9d1642..6bd63595 100644 --- a/rs/moq-scene/src/browser/outbox.rs +++ b/rs/moq-scene/src/browser/outbox.rs @@ -92,9 +92,13 @@ impl Commands { true } - /// Open page `id`. + /// Open page `id`, unless the helper is to exit, which would only close it. pub(crate) fn open(&self, id: Id, open: Open) { - self.push(|outbox| outbox.opens.push((id, open))); + self.push(|outbox| { + if !outbox.quit { + outbox.opens.push((id, open)); + } + }); } /// Close page `id`, which drops what it has waiting, its open too if @@ -145,7 +149,7 @@ impl Commands { } /// Have the helper close every page and exit, after what is queued, and - /// take nothing more. + /// open no more. pub(crate) fn quit(&self) { self.push(|outbox| outbox.quit = true); } @@ -317,4 +321,14 @@ mod tests { commands.close(1); assert!(matches!(commands.take()[..], [Command::Close(1)])); } + + /// A page opened once the helper is to exit never opens: the helper would + /// only close it again. + #[test] + fn a_page_opened_after_quit_never_opens() { + let commands = Commands::default(); + commands.quit(); + commands.open(0, open("late")); + assert!(matches!(commands.take()[..], [Command::Stop])); + } } diff --git a/rs/moq-scene/src/browser/run.rs b/rs/moq-scene/src/browser/run.rs index 83d0300f..5eab539e 100644 --- a/rs/moq-scene/src/browser/run.rs +++ b/rs/moq-scene/src/browser/run.rs @@ -258,8 +258,9 @@ impl Instance { page: page.clone(), id, }; - // A closed queue means the writer lost the socket, so the helper is - // gone and the reader ends the page. + // An open the queue refuses, since the writer lost the socket or the + // helper is exiting, ends with the helper, when the reader ends the + // page. self.commands.open(id, open); // After the page opens, since the helper delivers only to an open page. let mut state = page.state.lock().unwrap(); diff --git a/rs/moq-studio/tests/browser.rs b/rs/moq-studio/tests/browser.rs index d48f10e0..709788fc 100644 --- a/rs/moq-studio/tests/browser.rs +++ b/rs/moq-studio/tests/browser.rs @@ -626,15 +626,26 @@ const DYING: &str = "MOQ_STUDIO_TEST_DYING_PROFILE"; async fn a_helper_exits_with_its_studio() { let profile = tempfile::tempdir().unwrap(); let _stall = stall(profile.path()); - let mut studio = std::process::Command::new(std::env::current_exe().unwrap()) - .args(["--exact", "a_studio_that_dies", "--ignored"]) - .env(DYING, profile.path()) - .stdout(std::process::Stdio::null()) - .spawn() - .unwrap(); - let helper = until_stalled(studio.id(), profile.path()).await; - studio.kill().unwrap(); - studio.wait().unwrap(); + /// Killed and reaped once dropped, so a failing test leaves no studio + /// waiting forever. + struct Dying(std::process::Child); + impl Drop for Dying { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + let studio = Dying( + std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "a_studio_that_dies", "--ignored"]) + .env(DYING, profile.path()) + .stdout(std::process::Stdio::null()) + .spawn() + .unwrap(), + ); + let helper = until_stalled(studio.0.id(), profile.path()).await; + drop(studio); // The helper leads a process group CEF's processes join. let deadline = Instant::now() + TIMEOUT;