From aa61555698a7749404d06736885d8b06bf717c5a Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 01:14:22 -0400 Subject: [PATCH 1/4] feat(desktop): add WebKit renderer recovery ladder for Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some Linux GPU and compositor combinations crash the WebKit web process during startup, leaving Buzz with an invisible window and no way out (#2338, #2643). WebKit memoizes each renderer environment variable once per process, so a safer configuration can only ever be applied by launching a fresh process — the recovery is therefore built out of explicit relaunches rather than runtime toggles. On a crashed web process inside the startup window, the app prepares a durable episode record, releases the single-instance D-Bus name, proves the name is free, spawns a child with the next tier's exact environment, and exits. The child claims the record exclusively and confirms once it has started cleanly; a confirmed tier is reused on later launches. Recovery is crash-driven only. A window that never paints but also never crashes is not detected, because no available signal distinguishes it from a healthy window. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/Cargo.lock | 2 + desktop/src-tauri/Cargo.toml | 7 + desktop/src-tauri/src/lib.rs | 30 +- .../src-tauri/src/render_recovery/classify.rs | 172 ++++ desktop/src-tauri/src/render_recovery/cli.rs | 28 + desktop/src-tauri/src/render_recovery/dbus.rs | 89 ++ .../src-tauri/src/render_recovery/episode.rs | 156 +++ .../src-tauri/src/render_recovery/launcher.rs | 173 ++++ desktop/src-tauri/src/render_recovery/mod.rs | 45 + .../src-tauri/src/render_recovery/profiles.rs | 155 +++ .../src-tauri/src/render_recovery/session.rs | 551 ++++++++++ .../src-tauri/src/render_recovery/state.rs | 217 ++++ .../src-tauri/src/render_recovery/tests.rs | 950 ++++++++++++++++++ .../src-tauri/src/render_recovery/wiring.rs | 85 ++ 14 files changed, 2659 insertions(+), 1 deletion(-) create mode 100644 desktop/src-tauri/src/render_recovery/classify.rs create mode 100644 desktop/src-tauri/src/render_recovery/cli.rs create mode 100644 desktop/src-tauri/src/render_recovery/dbus.rs create mode 100644 desktop/src-tauri/src/render_recovery/episode.rs create mode 100644 desktop/src-tauri/src/render_recovery/launcher.rs create mode 100644 desktop/src-tauri/src/render_recovery/mod.rs create mode 100644 desktop/src-tauri/src/render_recovery/profiles.rs create mode 100644 desktop/src-tauri/src/render_recovery/session.rs create mode 100644 desktop/src-tauri/src/render_recovery/state.rs create mode 100644 desktop/src-tauri/src/render_recovery/tests.rs create mode 100644 desktop/src-tauri/src/render_recovery/wiring.rs diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 074b8f739e..f06fb8a2d5 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1085,8 +1085,10 @@ dependencies = [ "url", "user-idle", "uuid", + "webkit2gtk", "window-vibrancy", "windows-sys 0.61.2", + "zbus 5.17.0", "zeroize", "zip 8.6.0", ] diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index d689544688..8d0f2a3ea6 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -40,6 +40,13 @@ keyring = { version = "3.6.3", default-features = false, features = ["sync-secre # connection is dropped, which the plugin does immediately. Default features # keep the pure-Rust zbus backend, matching the plugin (no libdbus needed). notify-rust = "4" +# The renderer recovery ladder's crash edge. `WebView::connect_web_process_terminated` +# is gated on `v2_20`; wry already enables `v2_40` on the same crate version, so +# this adds no native dependency beyond the webkit2gtk wry links against. +webkit2gtk = { version = "2.0.2", features = ["v2_40"] } +# Session-bus observation of the single-instance well-known name, at the version +# tauri-plugin-single-instance already resolves, so both share one zbus stack. +zbus = "5.17" [target.'cfg(target_os = "macos")'.dependencies] objc2-app-kit = { version = "0.3.2", default-features = false, features = ["NSHapticFeedback"] } diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 5346791ccf..0eddc2293c 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -24,6 +24,8 @@ mod prevent_sleep; mod ptt_shortcut; mod relay; mod relay_admission; +#[cfg(target_os = "linux")] +mod render_recovery; mod reset; mod secret_store; mod shutdown; @@ -135,6 +137,24 @@ async fn wait_for_stable_initial_window_geometry(window: &tau #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { + let context = tauri::generate_context!(); + + // Linux renderer recovery runs before anything else: it may hand this + // launch off to a child carrying a different WebKit environment, which is + // only safe while the process is still single threaded and owns no + // single-instance name. + #[cfg(target_os = "linux")] + let render_recovery_session = match render_recovery::boot( + &context.config().identifier, + &context.package_info().version.to_string(), + ) { + render_recovery::Boot::Run(session) => Some(session), + // Recovery is off for this launch; the app still starts normally. + render_recovery::Boot::Off(_) => None, + // A child is Buzz now, or --reset-rendering-mode did its work. + render_recovery::Boot::HandedOff | render_recovery::Boot::Reset => return, + }; + // mesh-llm's async chains (model download, node start/join) overflow // tokio's default 2 MiB worker stacks — a stack-guard SIGABRT, not a // panic. Upstream mesh-llm and mesh-console both run on 8 MiB worker @@ -342,6 +362,14 @@ pub fn run() { #[cfg(not(buzz_updater_enabled))] let builder = builder; + // Registered after the single-instance plugin so the well-known name is + // already held when the ladder writes its `owned` receipt. + #[cfg(target_os = "linux")] + let builder = match render_recovery_session { + Some(session) => builder.plugin(render_recovery::plugin(session)), + None => builder, + }; + let app = builder .register_asynchronous_uri_scheme_protocol("buzz-media", |ctx, request, responder| { let app = ctx.app_handle().clone(); @@ -915,7 +943,7 @@ pub fn run() { is_auto_update_supported, set_window_vibrancy, ]) - .build(tauri::generate_context!()) + .build(context) .expect("error while building tauri application"); let shutdown_done = Arc::new(AtomicBool::new(false)); diff --git a/desktop/src-tauri/src/render_recovery/classify.rs b/desktop/src-tauri/src/render_recovery/classify.rs new file mode 100644 index 0000000000..54243d0953 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/classify.rs @@ -0,0 +1,172 @@ +//! Turning a durable record plus a bus observation into a boot decision. +//! +//! Pure over its inputs — the bus observation and process liveness are passed +//! in — so every arm of the decision table is a unit test rather than a +//! process-orchestration exercise. + +use super::dbus::Observation; +use super::state::{Phase, Record, RETRY_GENERATION}; + +/// How the current name owner relates to the child the record names. +/// +/// The `Uncorrelatable` arm is the one that is easy to get wrong: an owner that +/// exists but cannot be tied to our record is a handoff failure and a +/// diagnostic, and it is *never* evidence that the name is free. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Ownership { + /// Nobody owns the name. + Absent, + /// The owner is the child this record names. + Correlated, + /// A different process owns the name. + Mismatched, + /// Somebody owns the name but the record cannot be matched against them — + /// no recorded identity, no owner pid, or a receipt from another bus. + Uncorrelatable, +} + +/// What a fresh, untagged process should do about the record it found. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Decision { + /// A live episode owns this app right now; this process is a duplicate. + Defer(&'static str), + /// Re-run `tier` under the same token, one generation on. + RetrySameTier { reason: &'static str, tier: usize }, + /// The attempt at `tier` failed; move one rung down, or stop if `tier` was + /// the last one. + AdvanceOrStop { reason: &'static str, tier: usize }, + /// A tier started cleanly under this version. Run it again. + ReuseProfile { tier: usize }, + /// The ladder is spent. Run `tier` and stop — no advance, no episode. + StopExhausted { tier: usize }, + /// The record is unusable; delete it and launch from the baseline. + DiscardAndBaseline { reason: &'static str }, +} + +/// Correlate the recorded child against what the bus reports. +pub(crate) fn correlate( + record: &Record, + observation: &Observation, + bus_id: Option<&str>, +) -> Ownership { + let owner = match observation { + Observation::Free => return Ownership::Absent, + // An unreachable bus proves nothing about the name. Treat it as an + // owner we cannot identify rather than as a free name. + Observation::Unavailable(_) => return Ownership::Uncorrelatable, + Observation::Owned(owner) => owner, + }; + + match record.phase { + // `owned` and later carry a unique name, which is the precise + // identity — but only on the bus that issued it. + Phase::Owned | Phase::Confirmed => match (&record.unique_name, &record.bus_id) { + (Some(unique_name), Some(recorded_bus)) => { + if bus_id != Some(recorded_bus.as_str()) { + Ownership::Uncorrelatable + } else if *unique_name == owner.unique_name { + Ownership::Correlated + } else { + Ownership::Mismatched + } + } + _ => Ownership::Uncorrelatable, + }, + // Before ownership the only identity is the pid. + _ => match (record.pid, owner.pid) { + (Some(recorded), Some(actual)) if recorded == actual => Ownership::Correlated, + (Some(_), Some(_)) => Ownership::Mismatched, + _ => Ownership::Uncorrelatable, + }, + } +} + +/// Decide what to do with `record`. +/// +/// `pid_alive` answers whether a recorded pre-ownership child is still running; +/// `version` is the current app version, which invalidates a persisted profile +/// across an upgrade. +pub(crate) fn decide( + record: &Record, + observation: &Observation, + bus_id: Option<&str>, + version: &str, + pid_alive: &dyn Fn(u32) -> bool, +) -> Decision { + if record.version != version { + return Decision::DiscardAndBaseline { + reason: "app version changed since the record was written", + }; + } + + let ownership = correlate(record, observation, bus_id); + let owner_present = !matches!(observation, Observation::Free); + + match record.phase { + // Terminal. Re-running the ladder from here would loop forever + // against a machine no profile satisfies. + Phase::Exhausted => Decision::StopExhausted { tier: record.tier }, + + // Nobody ever claimed this attempt: the child died before its first + // action, or never started. One retry, same tier, next generation. + Phase::Prepared if record.generation < RETRY_GENERATION => Decision::RetrySameTier { + reason: "prepared attempt was never claimed", + tier: record.tier, + }, + Phase::Prepared => Decision::AdvanceOrStop { + reason: "prepared attempt was never claimed and the retry is spent", + tier: record.tier, + }, + + // `started` is the bounded window between the claim and name + // acquisition. Correlation against the *recorded* child is what + // separates an episode in flight from a handoff failure. + Phase::Started => match ownership { + Ownership::Correlated => Decision::Defer("recovery child owns the name"), + Ownership::Mismatched => Decision::AdvanceOrStop { + reason: "a different process owns the name: handoff failed", + tier: record.tier, + }, + Ownership::Uncorrelatable if owner_present => Decision::AdvanceOrStop { + reason: "the name has an owner that cannot be correlated: handoff failed", + tier: record.tier, + }, + // Name unowned: the child is either still in the pre-ownership + // interval or gone. + _ => match record.pid { + Some(pid) if pid_alive(pid) => { + Decision::Defer("recovery child is alive in the pre-ownership interval") + } + _ => Decision::AdvanceOrStop { + reason: "recovery child is gone before acquiring the name", + tier: record.tier, + }, + }, + }, + + // `owned` means the child really did take over. Only the same + // connection still holding the name keeps the episode in flight. + Phase::Owned => match ownership { + Ownership::Correlated => Decision::Defer("recovery child still owns the name"), + Ownership::Mismatched => Decision::AdvanceOrStop { + reason: "the recorded owner was replaced by another process", + tier: record.tier, + }, + Ownership::Uncorrelatable if owner_present => Decision::AdvanceOrStop { + reason: "the name owner cannot be correlated to the recorded child", + tier: record.tier, + }, + _ => Decision::AdvanceOrStop { + reason: "the recorded owner released the name", + tier: record.tier, + }, + }, + + // A tier that reached the startup boundary. This is the persisted + // "last crash-free startup profile" fact. + Phase::Confirmed => match ownership { + Ownership::Correlated => Decision::Defer("the confirmed instance is still running"), + _ => Decision::ReuseProfile { tier: record.tier }, + }, + } +} diff --git a/desktop/src-tauri/src/render_recovery/cli.rs b/desktop/src-tauri/src/render_recovery/cli.rs new file mode 100644 index 0000000000..5d4b55cecf --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/cli.rs @@ -0,0 +1,28 @@ +//! The two renderer flags. +//! +//! Parsed from raw argv before Tauri starts, because the tier they select has +//! to be in the process environment before WebKit initializes. + +/// Force the safest profile for this launch only. The ladder does not advance +/// and the persisted record is untouched. +pub(crate) const SAFE_RENDERING: &str = "--safe-rendering"; +/// Delete the persisted profile and episode state, report it, and exit. +pub(crate) const RESET_RENDERING_MODE: &str = "--reset-rendering-mode"; + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct Flags { + pub safe_rendering: bool, + pub reset_rendering_mode: bool, +} + +pub(crate) fn parse<'a>(args: impl IntoIterator) -> Flags { + let mut flags = Flags::default(); + for arg in args { + match arg.to_str() { + Some(SAFE_RENDERING) => flags.safe_rendering = true, + Some(RESET_RENDERING_MODE) => flags.reset_rendering_mode = true, + _ => {} + } + } + flags +} diff --git a/desktop/src-tauri/src/render_recovery/dbus.rs b/desktop/src-tauri/src/render_recovery/dbus.rs new file mode 100644 index 0000000000..ed1201dc04 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/dbus.rs @@ -0,0 +1,89 @@ +//! Facts about the single-instance well-known name, read from the session bus. +//! +//! This module only *observes*; correlating an observation against a recorded +//! child is `classify`'s job. The distinction matters because "somebody owns +//! the name" is never evidence that our recovery child survived, and "the bus +//! did not answer" is never evidence that the name is free. + +/// The single-instance name `tauri-plugin-single-instance` registers: the app +/// identifier plus `.SingleInstance` (the `semver` feature, which would append +/// a version suffix, is not enabled in this build). +pub(crate) fn single_instance_name(identifier: &str) -> String { + format!("{identifier}.SingleInstance") +} + +/// Who currently holds the well-known name. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Owner { + pub unique_name: String, + /// `None` when the bus refused `GetConnectionUnixProcessID` — the owner + /// exists but cannot be identified. + pub pid: Option, +} + +/// What the bus said about the name. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Observation { + /// `GetNameOwner` reported no owner. The only state in which a handoff may + /// proceed. + Free, + Owned(Owner), + /// The bus could not be reached or the call failed. Deliberately distinct + /// from `Free`. + Unavailable(String), +} + +/// Identity of the session bus itself. A unique name like `:1.4` is only +/// meaningful against the bus that issued it, so `owned` receipts are bound to +/// this value as well as to the unique name. +pub(crate) fn bus_id() -> Option { + call::("GetId", &()) +} + +/// Observe the current owner of `name`. +pub(crate) fn observe(name: &str) -> Observation { + let connection = match zbus::blocking::Connection::session() { + Ok(connection) => connection, + Err(error) => return Observation::Unavailable(format!("session bus: {error}")), + }; + match connection.call_method( + Some("org.freedesktop.DBus"), + "/org/freedesktop/DBus", + Some("org.freedesktop.DBus"), + "GetNameOwner", + &name, + ) { + Ok(reply) => match reply.body().deserialize::() { + Ok(unique_name) => { + let pid = call::("GetConnectionUnixProcessID", &unique_name); + Observation::Owned(Owner { unique_name, pid }) + } + Err(error) => Observation::Unavailable(format!("GetNameOwner reply: {error}")), + }, + // The bus answers NameHasNoOwner as an error reply; that is the one + // failure that genuinely means "free". + Err(zbus::Error::MethodError(name, _, _)) + if name.as_str() == "org.freedesktop.DBus.Error.NameHasNoOwner" => + { + Observation::Free + } + Err(error) => Observation::Unavailable(format!("GetNameOwner: {error}")), + } +} + +fn call(method: &str, body: &(impl serde::Serialize + zbus::zvariant::DynamicType)) -> Option +where + T: for<'d> zbus::zvariant::Type + serde::de::DeserializeOwned, +{ + let connection = zbus::blocking::Connection::session().ok()?; + let reply = connection + .call_method( + Some("org.freedesktop.DBus"), + "/org/freedesktop/DBus", + Some("org.freedesktop.DBus"), + method, + body, + ) + .ok()?; + reply.body().deserialize::().ok() +} diff --git a/desktop/src-tauri/src/render_recovery/episode.rs b/desktop/src-tauri/src/render_recovery/episode.rs new file mode 100644 index 0000000000..1f48a08448 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/episode.rs @@ -0,0 +1,156 @@ +//! The pure rules of an episode: who may claim it, when a termination counts, +//! and where the ladder goes next. +//! +//! Everything here is platform-independent and side-effecting only through the +//! `Store` it is handed, which is what makes the generation races and the +//! attempt cap ordinary unit tests. + +use super::profiles::Package; +use super::state::{Claim, Phase, Record, Store, RETRY_GENERATION}; + +/// Identity of one attempt. The token names the rung's attempt; the generation +/// distinguishes the original from its sole retry. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Episode { + pub token: String, + pub generation: u32, +} + +/// Why a web process went away, reduced to the only distinction the ladder +/// makes. Mapping from WebKit's `#[non_exhaustive]` reason enum happens at the +/// signal boundary, so an unknown future reason lands in `Other` — the arm that +/// never downgrades anything. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Termination { + Crashed, + Other, +} + +/// How long after launch a `Crashed` web process still counts as a startup +/// failure, and therefore how long a launch must survive to be recorded as +/// crash-free. +/// +/// This is the ladder's own boundary. It is deliberately **not** the ~10 s +/// figure from the spike, which is a no-paint probe timeout for a track that +/// is out of scope here and was never a crash-eligibility cutoff. +/// +/// No reporter trace carries a timing, so there is nothing to derive a number +/// from; the only measurement available is the spike's 562-780 ms floor to +/// first content under llvmpipe, which is a floor and not a prediction. Five +/// seconds is roughly six times that floor — enough headroom for a cold WebKit +/// on slow hardware, while still well inside the window in which a user would +/// say the app "never came up". The error direction is chosen: too short only +/// loses a recovery the user can still trigger with `--safe-rendering`, +/// whereas too long downgrades rendering for a crash that had nothing to do +/// with startup. So it errs short. +pub(crate) const CRASH_ELIGIBILITY_WINDOW: std::time::Duration = std::time::Duration::from_secs(5); + +/// Whether this termination advances the ladder. Only a `Crashed` web process +/// inside the eligibility window does; everything else is diagnostic. +pub(crate) fn advances_ladder(termination: Termination, since_launch: std::time::Duration) -> bool { + termination == Termination::Crashed && since_launch <= CRASH_ELIGIBILITY_WINDOW +} + +/// Where the ladder goes after a tier fails. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Advance { + /// Try this tier next. + Tier(usize), + /// No tier left for this package. + Exhausted, +} + +/// Step down one tier, or run out. +/// +/// This is also case 10's per-package attempt cap. A crash always advances and +/// never repeats a tier, so the ladder can spend at most one attempt per tier +/// and the tier count *is* the cap — no separate counter can drift from it. +pub(crate) fn advance(package: Package, tier: usize) -> Advance { + match tier + 1 { + next if next <= package.terminal_tier() => Advance::Tier(next), + _ => Advance::Exhausted, + } +} + +/// What a tagged recovery child concluded about the episode it was sent to run. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Claimed { + /// This process exclusively owns `episode` at `tier`, and `started` is + /// durable. + Owner { episode: Episode, tier: usize }, + /// This process is not the owner. It writes nothing and runs on as an + /// ordinary launch; the single-instance plugin decides its fate. + Yielded(&'static str), +} + +/// Exclusively claim the prepared record this child was launched for. +/// +/// `episode` comes from the child's tag, never from the record on disk. By the +/// time a delayed child runs, a retry may have prepared a newer generation, and +/// adopting the record's generation would let the stale child write the newer +/// generation's receipt. +pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { + let Some(record) = store.read() else { + return Claimed::Yielded("no episode record"); + }; + if record.token != episode.token || record.generation != episode.generation { + return Claimed::Yielded("this episode is no longer current"); + } + if record.phase != Phase::Prepared { + return Claimed::Yielded("this episode was already claimed"); + } + + match store.claim(&episode.token, episode.generation) { + Claim::Won => {} + Claim::Lost => return Claimed::Yielded("another child claimed this episode first"), + Claim::Failed(error) => { + eprintln!("{}: claim failed: {error}", super::LOG); + return Claimed::Yielded("the claim could not be taken exclusively"); + } + } + + let mut started = record.clone(); + started.phase = Phase::Started; + started.pid = Some(std::process::id()); + if let Err(error) = store.write(&started) { + // The claim is taken and cannot be given back, so yielding here would + // strand the episode. Run on instead: the record still says `prepared`, + // and the next launch retries or advances it. + eprintln!("{}: failed to persist started: {error}", super::LOG); + return Claimed::Yielded("the started receipt could not be persisted"); + } + + Claimed::Owner { + episode: episode.clone(), + tier: record.tier, + } +} + +/// The record a parent must make durable before spawning a child for `tier`. +pub(crate) fn prepare(episode: &Episode, package: Package, tier: usize, version: &str) -> Record { + let name = package.tier(tier).map(|tier| tier.name).unwrap_or_default(); + Record::new( + Phase::Prepared, + &episode.token, + episode.generation, + name, + tier, + version, + ) +} + +/// The retry of an unclaimed attempt: same token, one generation on. +pub(crate) fn retry_of(record: &Record) -> Episode { + Episode { + token: record.token.clone(), + generation: record.generation + RETRY_GENERATION, + } +} + +/// A fresh attempt at a rung. +pub(crate) fn fresh() -> Episode { + Episode { + token: uuid::Uuid::new_v4().to_string(), + generation: 0, + } +} diff --git a/desktop/src-tauri/src/render_recovery/launcher.rs b/desktop/src-tauri/src/render_recovery/launcher.rs new file mode 100644 index 0000000000..1115da31dd --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/launcher.rs @@ -0,0 +1,173 @@ +//! The one explicit recovery launcher. +//! +//! `tauri::App::request_restart` cannot serve here: it has no child-environment +//! parameter, and its failure branch is crate-private and skips the +//! single-instance plugin's name release, so every child born from it dies as a +//! duplicate. `std::env::set_var` is equally out — it is unsound in a process +//! with live GTK and Tokio threads, and WebKit memoizes each renderer variable +//! once per process anyway, so a late assignment would be read by nothing. +//! +//! So the sequence is written out in full, in Buzz's own code: +//! +//! ```text +//! prepare the record atomically +//! → release the single-instance name (running app only) +//! → prove the name is free +//! → spawn a Command with an exact environment +//! → the child claims the prepared record +//! → the parent exits +//! ``` +//! +//! Spawn success and a zero exit code are both worthless as survival signals — +//! a child that loses the name forwards its argv and exits 0. Only the child's +//! claim receipt, correlated against the name's owner, proves a handoff. + +use super::dbus::{self, Observation}; +use super::episode::Episode; +use super::profiles::{Env, Package, Tier}; + +/// Environment tag carried by a recovery child. +pub(crate) const EPISODE: &str = "BUZZ_RENDER_EPISODE"; +pub(crate) const GENERATION: &str = "BUZZ_RENDER_GENERATION"; +pub(crate) const PROFILE: &str = "BUZZ_RENDER_PROFILE"; +pub(crate) const FORCED: &str = "BUZZ_RENDER_FORCED"; + +const TAGS: [&str; 4] = [EPISODE, GENERATION, PROFILE, FORCED]; + +/// The tag a recovery child was launched with. Its presence is what tells the +/// child that the owned variables in its environment are its parent's profile +/// rather than the user's configuration. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Tag { + /// `None` for a forced launch, which owns no episode and persists nothing. + pub episode: Option, + pub profile: String, +} + +impl Tag { + /// Read from the environment, or `None` for an ordinary launch. + /// + /// A tag missing its generation is malformed rather than generation 0: + /// defaulting would let a corrupted tag claim the original attempt. + pub(crate) fn read(env: Env<'_>) -> Option { + let profile = env(PROFILE)?; + if env(FORCED).is_some() { + return Some(Tag { + episode: None, + profile, + }); + } + let token = env(EPISODE).filter(|token| !token.is_empty())?; + let generation = env(GENERATION)?.parse().ok()?; + Some(Tag { + episode: Some(Episode { token, generation }), + profile, + }) + } +} + +/// Why a relaunch did not happen. Every arm leaves the current process running. +#[derive(Debug)] +pub(crate) enum Refusal { + /// The name has an owner, or the bus could not answer. Either way this is + /// a handoff failure, never a free name. + NameNotFree(String), + /// The binary to re-execute could not be resolved. + NoBinary(String), + /// `Command::spawn` itself failed: no child exists. + SpawnFailed(String), + /// The prepared record could not be made durable. Refusing here is the + /// point: a child that arrives to claim a record that was never written + /// would run untracked, and the ladder would lose the attempt. + StateNotDurable(String), + /// The requested rung is not in this package's ladder. + NoTier(String), +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Refusal::NameNotFree(detail) => write!(f, "single-instance name not free: {detail}"), + Refusal::NoBinary(detail) => write!(f, "cannot locate the app binary: {detail}"), + Refusal::SpawnFailed(detail) => write!(f, "spawn failed: {detail}"), + Refusal::StateNotDurable(detail) => { + write!(f, "episode record not durable: {detail}") + } + Refusal::NoTier(detail) => write!(f, "no such renderer tier: {detail}"), + } + } +} + +/// Build a child command whose owned-variable environment is exactly `tier`. +/// +/// Every owned variable is removed first, then only the tier's pairs are +/// added, so an attempt never inherits the previous attempt's profile. Package +/// scoping applies to the removals too: on AppImage `GDK_BACKEND` is +/// re-exported by the AppRun hook before the child binary starts, so removing +/// it here would be theatre. +pub(crate) fn exact_env_command( + binary: std::path::PathBuf, + args: &[std::ffi::OsString], + package: Package, + tier: &Tier, + tag: &Tag, +) -> std::process::Command { + let mut command = std::process::Command::new(binary); + command.args(args); + for key in package.owned_vars().iter().chain(TAGS.iter()) { + command.env_remove(key); + } + command.envs(tier.vars.iter().copied()); + command.env(PROFILE, &tag.profile); + match &tag.episode { + Some(episode) => { + command.env(EPISODE, &episode.token); + command.env(GENERATION, episode.generation.to_string()); + } + None => { + command.env(FORCED, "1"); + } + } + command +} + +/// The spawn edge, as a plain function pointer so tests can drive the refusal +/// and success branches without ever forking a real process. +pub(crate) type Launch = fn( + &str, + Result, + &[std::ffi::OsString], + Package, + &'static Tier, + &Tag, +) -> Result; + +/// Prove the name is free, then spawn the attempt. +/// +/// Any prepared record must already be durable: the child claims it as its +/// first action, and a child that arrives before the record does would find +/// nothing to claim. +pub(crate) fn spawn( + dbus_name: &str, + binary: Result, + args: &[std::ffi::OsString], + package: Package, + tier: &Tier, + tag: &Tag, +) -> Result { + match dbus::observe(dbus_name) { + Observation::Free => {} + Observation::Owned(owner) => { + return Err(Refusal::NameNotFree(format!( + "owned by {} (pid {:?})", + owner.unique_name, owner.pid + ))) + } + Observation::Unavailable(detail) => return Err(Refusal::NameNotFree(detail)), + } + + exact_env_command(binary.map_err(Refusal::NoBinary)?, args, package, tier, tag) + .spawn() + .map(|child| child.id()) + .map_err(|error| Refusal::SpawnFailed(error.to_string())) +} diff --git a/desktop/src-tauri/src/render_recovery/mod.rs b/desktop/src-tauri/src/render_recovery/mod.rs new file mode 100644 index 0000000000..058bd1805b --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/mod.rs @@ -0,0 +1,45 @@ +//! WebKitGTK renderer recovery ladder (Linux). +//! +//! Some Linux GPU/compositor combinations crash the WebKit web process during +//! startup, leaving Buzz with an invisible window and no way out. WebKit reads +//! each of its renderer environment variables exactly once per process, so a +//! safer configuration can only ever be applied by launching a *fresh* process +//! — the ladder is therefore built out of relaunches, not runtime toggles. +//! +//! On a `Crashed` web-process termination before the startup boundary, the app +//! prepares a durable record, releases the single-instance name, hands off to a +//! child launched with the next tier's exact environment, and exits. The child +//! claims the record, takes the name, and confirms once it has started cleanly. +//! A confirmed tier is reused on later launches — also via a relaunch, since +//! `std::env::set_var` is unsound in a process with live GTK and Tokio threads. +//! +//! Scope: crash-driven recovery, `--safe-rendering`, `--reset-rendering-mode`. +//! A window that never paints but also never crashes is **not** detected — no +//! available signal distinguishes it from a healthy window, so that track is +//! gated on validation against affected hardware. +//! +//! Layout: `profiles` (package scoping + tier table), `state` (durable record + +//! exclusive claims), `episode` (attempt identity and the pure ladder rules), +//! `dbus` (name observation), `classify` (record + observation → decision), +//! `launcher` (the explicit relaunch), `cli` (flags), `session` (the live +//! process), `wiring` (the Tauri plugin that carries it into the app). + +pub(crate) mod classify; +pub(crate) mod cli; +pub(crate) mod dbus; +pub(crate) mod episode; +pub(crate) mod launcher; +pub(crate) mod profiles; +pub(crate) mod session; +pub(crate) mod state; +pub(crate) mod wiring; + +#[cfg(test)] +mod tests; + +pub(crate) use session::{boot, Boot}; +pub(crate) use wiring::plugin; + +/// Log prefix for every line this feature emits. The ladder's whole diagnostic +/// value is in its log, so the lines are greppable under one tag. +const LOG: &str = "buzz-desktop render-recovery"; diff --git a/desktop/src-tauri/src/render_recovery/profiles.rs b/desktop/src-tauri/src/render_recovery/profiles.rs new file mode 100644 index 0000000000..550edcf1d3 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/profiles.rs @@ -0,0 +1,155 @@ +//! Package scoping, profile-owned environment variables, and the tier table. +//! +//! Every value here is derived from the WebKitGTK source analysis recorded in +//! `RESEARCH/WEBKIT_RECOVERY_SPIKE.md` (Q2/Q4). Two rules drive the shape: +//! +//! * WebKit memoizes each of these variables once per process +//! (`AcceleratedBackingStore.cpp` `std::call_once`), so a profile can only be +//! applied by launching a fresh process with an exact environment. +//! * Ownership is **package-scoped**. `GDK_BACKEND=x11` is a package invariant +//! on AppImage — linuxdeploy's GTK AppRun hook exports it unconditionally +//! before the binary starts, so its presence there is never a user signal and +//! a tier that sets it is a no-op. + +/// Reads one environment variable. Injected so ownership and package detection +/// are testable without mutating the process environment. +pub(crate) type Env<'a> = &'a dyn Fn(&str) -> Option; + +/// How this process was packaged, which decides both the owned variable set +/// and the number of ladder tiers. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Package { + AppImage, + Native, +} + +/// The four WebKit renderer variables. Owned by both packages. +const WEBKIT_VARS: [&str; 4] = [ + "WEBKIT_DMABUF_RENDERER_FORCE_SHM", + "WEBKIT_DISABLE_DMABUF_RENDERER", + "WEBKIT_SKIA_ENABLE_CPU_RENDERING", + // No tier sets this one — it is owned so that a user value still opts out. + // It stays out of the ladder pending the 3-var vs 4-var isolation on #2643. + "WEBKIT_DISABLE_COMPOSITING_MODE", +]; + +/// Owned on native Linux only. See the module docs. +const GDK_BACKEND: &str = "GDK_BACKEND"; + +const OWNED_NATIVE: [&str; 5] = [ + WEBKIT_VARS[0], + WEBKIT_VARS[1], + WEBKIT_VARS[2], + WEBKIT_VARS[3], + GDK_BACKEND, +]; + +/// One rung of the ladder: a name plus the exact variable set it applies. +/// The set is exhaustive — an attempt's environment is every owned variable +/// removed, then exactly these pairs added. +pub(crate) struct Tier { + pub name: &'static str, + pub vars: &'static [(&'static str, &'static str)], +} + +const TIERS: [Tier; 5] = [ + Tier { + name: "full-gpu", + vars: &[], + }, + Tier { + // Drops zero-copy dmabuf but keeps accelerated compositing and + // threaded scrolling: FORCE_SHM is read *after* SharedMemory is added. + name: "shm-transport", + vars: &[("WEBKIT_DMABUF_RENDERER_FORCE_SHM", "1")], + }, + Tier { + // Adds WebProcess CPU rasterization on top of the SHM transport. + name: "cpu-raster", + vars: &[ + ("WEBKIT_DMABUF_RENDERER_FORCE_SHM", "1"), + ("WEBKIT_SKIA_ENABLE_CPU_RENDERING", "1"), + ], + }, + Tier { + // DISABLE_DMABUF returns before SharedMemory is added, so this also + // loses accelerated compositing and threaded scrolling. + name: "no-accel", + vars: &[ + ("WEBKIT_DISABLE_DMABUF_RENDERER", "1"), + ("WEBKIT_SKIA_ENABLE_CPU_RENDERING", "1"), + ], + }, + Tier { + // Native Linux only — on AppImage the backend is already pinned, so + // this rung would spend a relaunch on a no-op. + name: "x11-fallback", + vars: &[ + ("WEBKIT_DISABLE_DMABUF_RENDERER", "1"), + ("WEBKIT_SKIA_ENABLE_CPU_RENDERING", "1"), + (GDK_BACKEND, "x11"), + ], + }, +]; + +impl Package { + /// Detected the same way `tauri::Env` does, so the launcher and the + /// ownership check agree with `tauri::process::current_binary`. + pub(crate) fn detect(env: Env<'_>) -> Self { + match env("APPIMAGE") { + Some(_) => Package::AppImage, + None => Package::Native, + } + } + + /// The variables this package's profiles own, exhaustively. A user + /// assignment of any of them disables recovery wholesale. + pub(crate) fn owned_vars(self) -> &'static [&'static str] { + match self { + Package::AppImage => &WEBKIT_VARS, + Package::Native => &OWNED_NATIVE, + } + } + + /// The ladder for this package: 4 rungs on AppImage, 5 on native Linux. + pub(crate) fn tiers(self) -> &'static [Tier] { + match self { + Package::AppImage => &TIERS[..4], + Package::Native => &TIERS, + } + } + + /// Index of the last rung. Reaching it and crashing again exhausts the + /// ladder, which is also case 10's per-package attempt cap: one crash + /// advances exactly one rung, so the rung count *is* the cap. + pub(crate) fn terminal_tier(self) -> usize { + self.tiers().len() - 1 + } + + pub(crate) fn tier(self, index: usize) -> Option<&'static Tier> { + self.tiers().get(index) + } + + pub(crate) fn tier_named(self, name: &str) -> Option { + self.tiers().iter().position(|tier| tier.name == name) + } + + pub(crate) fn label(self) -> &'static str { + match self { + Package::AppImage => "appimage", + Package::Native => "native", + } + } +} + +/// Owned variables the caller's environment actually carries, as `KEY=VALUE`. +/// +/// Presence is the test, not truthiness: `VAR=0` and `VAR=` are both genuine +/// user assignments and both disable recovery. +pub(crate) fn owned_present(package: Package, env: Env<'_>) -> Vec { + package + .owned_vars() + .iter() + .filter_map(|key| env(key).map(|value| format!("{key}={value}"))) + .collect() +} diff --git a/desktop/src-tauri/src/render_recovery/session.rs b/desktop/src-tauri/src/render_recovery/session.rs new file mode 100644 index 0000000000..968f693056 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/session.rs @@ -0,0 +1,551 @@ +//! Boot reconciliation and the live ladder state for the running process. +//! +//! `boot` runs before Tauri is built, while the process is still single +//! threaded and owns no D-Bus name — both things it may do (hand off to a child +//! with a different environment, or exit for `--reset-rendering-mode`) require +//! that. What it returns is either a `Session`, which the rest of the process +//! uses to react to a crash, or an instruction to exit. + +use std::ffi::OsString; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use super::classify::{self, Decision}; +use super::cli; +use super::dbus::{self, Observation}; +use super::episode::{self, Advance, Claimed, Episode, Termination}; +use super::launcher::{self, Refusal, Tag}; +use super::profiles::{self, Env, Package, Tier}; +use super::state::{Phase, Record, Store}; +use super::LOG; + +/// Why the ladder is not running this launch. In every case the app starts +/// normally with whatever environment it already has. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Disabled { + /// The user assigned an owned variable. Their configuration wins whole: no + /// tier selection, no relaunch, no ladder. + UserEnv(Vec), + /// No app data dir, so no durable record is possible. + NoStateDir(String), +} + +impl std::fmt::Display for Disabled { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Disabled::UserEnv(vars) => write!( + f, + "{} set in the environment; renderer recovery leaves user \ + configuration alone", + vars.join(", ") + ), + Disabled::NoStateDir(error) => write!(f, "{error}"), + } + } +} + +/// What `boot` concluded. +pub(crate) enum Boot { + /// Continue in this process. The environment already matches the session's + /// tier, and the session carries what is needed to react to a later crash. + Run(Arc), + /// A child was spawned to run the selected tier; this process must exit + /// without starting the app. The child is Buzz now. + HandedOff, + /// `--reset-rendering-mode` did its work. Exit without launching. + Reset, + /// Recovery is off this launch, but the app still starts. + Off(Disabled), +} + +/// Everything the running process needs to react to a web-process crash. +pub(crate) struct Session { + store: Store, + package: Package, + version: String, + dbus_name: String, + args: Vec, + /// The tier this process is actually running. + tier: usize, + /// Episode identity, present only when this process claimed a prepared + /// record. A plain launch owns no episode. + episode: Option, + /// Manual override: never advance, never persist. Set by `--safe-rendering` + /// and by any child that could not claim the episode it was sent to run. + frozen: bool, + /// Set once a ladder-eligible crash is seen, so a process that observed one + /// and failed to hand off never goes on to record a crash-free startup. + crashed: AtomicBool, + /// When this process started, for the crash-eligibility window. + launched: std::time::Instant, + /// The spawn edge. Always `launcher::spawn` in production; tests replace it + /// so a refusal or a success can be driven without forking. + launch: launcher::Launch, +} + +/// Reconcile persisted state and select this launch's renderer tier. +/// +/// `identifier` is the bundle identifier, which names both the app data dir and +/// the single-instance bus name; `version` invalidates a persisted tier across +/// an upgrade. +pub(crate) fn boot(identifier: &str, version: &str) -> Boot { + let args: Vec = std::env::args_os().skip(1).collect(); + let app_data_dir = dirs::data_dir() + .map(|dir| dir.join(identifier)) + .ok_or_else(|| "no user data directory".to_string()); + let boot = reconcile( + app_data_dir, + identifier, + version, + args, + &|key| { + // `var_os`, not `var`: presence is the test, and a non-UTF-8 value + // is still a user assignment. + std::env::var_os(key).map(|value| value.to_string_lossy().into_owned()) + }, + launcher::spawn, + ); + if let Boot::Off(reason) = &boot { + eprintln!("{LOG}: disabled — {reason}"); + } + boot +} + +/// `boot` with its environmental inputs supplied, so the decision path is +/// exercisable without touching the real process environment. +pub(super) fn reconcile( + app_data_dir: Result, + identifier: &str, + version: &str, + args: Vec, + env: Env<'_>, + launch: launcher::Launch, +) -> Boot { + let flags = cli::parse(args.iter().map(OsString::as_os_str)); + let package = Package::detect(env); + let tag = Tag::read(env); + + let app_data_dir = match app_data_dir { + Ok(dir) => dir, + Err(error) => return Boot::Off(Disabled::NoStateDir(error)), + }; + + let session = Session { + store: Store::new(&app_data_dir), + package, + version: version.to_string(), + dbus_name: dbus::single_instance_name(identifier), + args, + tier: 0, + episode: None, + frozen: false, + crashed: AtomicBool::new(false), + launched: std::time::Instant::now(), + launch, + }; + + // Reset runs ahead of the opt-out check on purpose: a user who has set an + // owned variable is exactly the user most likely to be clearing a record + // that an earlier run left behind. + if flags.reset_rendering_mode { + return session.reset(); + } + + // A recovery child never reinterprets the variables its parent injected as + // user configuration, so only an untagged launch checks for a user opt-out. + if tag.is_none() { + let present = profiles::owned_present(package, env); + if !present.is_empty() { + return Boot::Off(Disabled::UserEnv(present)); + } + } + + match tag { + Some(tag) => session.start_as_child(tag), + None => session.select_tier(flags.safe_rendering), + } +} + +impl Session { + /// `--reset-rendering-mode`: delete the persisted tier and episode state, + /// say what was cleared, and exit without launching. + fn reset(self) -> Boot { + println!( + "{}", + match self.store.clear() { + true => "Cleared the persisted renderer profile and episode state.", + false => "No persisted renderer profile or episode state to clear.", + } + ); + Boot::Reset + } + + /// Untagged launch: classify the record, then either run the selected tier + /// here or hand off to a child that can. + fn select_tier(mut self, safe_rendering: bool) -> Boot { + // `--safe-rendering` is a manual override for this launch only: the + // terminal tier, no episode, nothing persisted. + if safe_rendering { + self.tier = self.package.terminal_tier(); + self.frozen = true; + eprintln!( + "{LOG}: {} — forcing {} for this launch only", + cli::SAFE_RENDERING, + self.profile_name() + ); + return self.run_selected_tier(); + } + + let Some(record) = self.store.read() else { + return self.run_selected_tier(); + }; + let decision = classify::decide( + &record, + &dbus::observe(&self.dbus_name), + dbus::bus_id().as_deref(), + &self.version, + &pid_alive, + ); + eprintln!( + "{LOG}: found {:?} record (token={} gen={} tier={} profile={}) → {decision:?}", + record.phase, record.token, record.generation, record.tier, record.profile + ); + + match decision { + // Another instance owns the app. Run on as an ordinary duplicate + // and let the single-instance plugin forward argv and exit us. + Decision::Defer(_) => Boot::Run(Arc::new(self)), + Decision::DiscardAndBaseline { .. } => { + self.store.clear(); + self.run_selected_tier() + } + // The persisted "last crash-free startup profile" fact. + Decision::ReuseProfile { tier } => { + self.tier = tier; + self.run_selected_tier() + } + // No profile survived. Stop rather than relaunch: this process is + // the baseline environment, and the user's way to the safest tier + // is the flag, not another fork. + Decision::StopExhausted { tier } => { + self.frozen = true; + eprintln!( + "{LOG}: the ladder was already exhausted at tier {tier} on {}; \ + relaunch with {} to force the safest profile", + self.package.label(), + cli::SAFE_RENDERING + ); + Boot::Run(Arc::new(self)) + } + // An attempt that never produced a receipt: re-run the same tier + // under the next generation. + Decision::RetrySameTier { tier, .. } => { + self.tier = tier; + self.hand_off_or_run(Some(episode::retry_of(&record)), tier) + } + Decision::AdvanceOrStop { tier, .. } => self.step_down(tier), + } + } + + /// Continue here when this process already carries the selected tier's + /// environment, otherwise hand off to a child that carries it exactly. + fn run_selected_tier(self) -> Boot { + if self.tier == 0 { + // Tier 0 sets nothing, and the opt-out check above proved no owned + // variable is present — this process already *is* tier 0. + return Boot::Run(Arc::new(self)); + } + // A frozen launch is a manual override, so it runs the tier without + // owning an episode: no record, no receipt, no advance. + let episode = (!self.frozen).then(episode::fresh); + let tier = self.tier; + self.hand_off_or_run(episode, tier) + } + + /// Move one tier down for a failed attempt, or stop at the terminal tier. + fn step_down(mut self, failed: usize) -> Boot { + match episode::advance(self.package, failed) { + Advance::Tier(next) => { + self.tier = next; + self.hand_off_or_run(Some(episode::fresh()), next) + } + Advance::Exhausted => { + self.tier = failed; + self.frozen = true; + self.note_exhausted(); + Boot::Run(Arc::new(self)) + } + } + } + + /// Recovery-child path: exclusively claim the record this child was + /// launched for, and record `started` as its first action. + fn start_as_child(mut self, tag: Tag) -> Boot { + let Some(tier) = self.package.tier_named(&tag.profile) else { + // Nothing here can be trusted to describe the environment the + // parent actually applied, so this launch is not tracked at all. + eprintln!( + "{LOG}: recovery tag names an unknown profile ({}); not tracking this launch", + tag.profile + ); + self.frozen = true; + return Boot::Run(Arc::new(self)); + }; + self.tier = tier; + + // A forced child owns no episode, so it has nothing to claim — it just + // runs the environment it was handed. + let Some(episode) = tag.episode else { + self.frozen = true; + return Boot::Run(Arc::new(self)); + }; + + match episode::claim(&self.store, &episode) { + Claimed::Owner { episode, tier } => { + self.tier = tier; + self.episode = Some(episode); + } + // A child that is not the owner writes nothing and runs on as an + // ordinary launch. The single-instance plugin decides whether it is + // a duplicate; the ladder simply stops tracking it. + Claimed::Yielded(reason) => { + eprintln!("{LOG}: recovery child is not the episode owner — {reason}"); + self.frozen = true; + } + } + Boot::Run(Arc::new(self)) + } + + pub(crate) fn profile_name(&self) -> &'static str { + self.package.tier(self.tier).map(|t| t.name).unwrap_or("") + } + + /// Record that this process holds the single-instance name. + pub(crate) fn note_owned(&self) { + self.note(Phase::Owned); + } + + /// How long this process still has before it counts as a crash-free start. + pub(crate) fn until_confirmation(&self) -> std::time::Duration { + episode::CRASH_ELIGIBILITY_WINDOW.saturating_sub(self.launched.elapsed()) + } + + /// Record that this process outlived the crash-eligibility window — the + /// "last crash-free startup profile" fact. Version-scoped, and never a + /// claim that rendering is actually correct. + pub(crate) fn note_confirmed(&self) { + if self.crashed.load(Ordering::SeqCst) { + // An eligible crash was seen and the handoff did not carry us away. + // Recording this tier as crash-free would persist the opposite of + // what happened. + return; + } + self.note(Phase::Confirmed); + } + + /// Write a phase receipt for the episode this process owns. + /// + /// The receipt is bound to the bus id as well as the unique name: a unique + /// name like `:1.4` only identifies a connection on the bus that issued it, + /// so without the bus id a receipt from an earlier session's bus could + /// correlate against a stranger here. + fn note(&self, phase: Phase) { + let Some(episode) = &self.episode else { + return; + }; + let mut record = self.record(phase, episode); + record.pid = Some(std::process::id()); + match dbus::observe(&self.dbus_name) { + Observation::Owned(owner) => { + record.unique_name = Some(owner.unique_name); + record.bus_id = dbus::bus_id(); + } + // Not owning the name does not block the receipt: the phase itself + // is still true, and `classify` reads a receipt with no recorded + // identity as uncorrelatable rather than as ours. + _ => eprintln!("{LOG}: writing {phase:?} without an owner identity"), + } + self.write(&record); + } + + /// React to a web-process termination. Returns `true` when a child took + /// over and this process must exit. + pub(crate) fn on_web_process_terminated( + &self, + termination: Termination, + release_name: &dyn Fn(), + ) -> bool { + let elapsed = self.launched.elapsed(); + if !episode::advances_ladder(termination, elapsed) { + eprintln!( + "{LOG}: web process ended ({termination:?}) {elapsed:?} after launch \ + at tier {} ({}) — not ladder-eligible", + self.tier, + self.profile_name() + ); + return false; + } + self.crashed.store(true, Ordering::SeqCst); + if self.frozen { + eprintln!("{LOG}: not advancing — this launch is running a forced profile"); + return false; + } + + match episode::advance(self.package, self.tier) { + Advance::Exhausted => { + self.note_exhausted(); + false + } + Advance::Tier(next) => match self.hand_off(episode::fresh(), next, release_name) { + Ok(()) => true, + Err(refusal) => { + eprintln!("{LOG}: relaunch refused: {refusal}"); + false + } + }, + } + } + + /// Hand off to a child at `tier`, or run on here if the handoff is refused. + /// Only reached before the single-instance name exists, so there is nothing + /// to release first. + fn hand_off_or_run(mut self, episode: Option, tier: usize) -> Boot { + let handed = match episode { + Some(episode) => self.hand_off(episode, tier, &|| {}), + None => self.force(tier), + }; + match handed { + Ok(()) => Boot::HandedOff, + Err(refusal) => { + eprintln!("{LOG}: staying at the launched profile — {refusal}"); + // The handoff did not happen, so this process is still the + // baseline it was launched as and must not claim otherwise. + self.tier = 0; + self.frozen = true; + Boot::Run(Arc::new(self)) + } + } + } + + /// Prepare a record for `tier`, release the single-instance name, and spawn + /// a child carrying the tier's exact environment. + /// + /// The order is the whole mechanism. The record must be durable before the + /// child exists, because the child claims it as its first action and would + /// otherwise find nothing. The name must be released before the probe, + /// because a child that fails to take the name forwards its argv and exits + /// 0 — indistinguishable from a successful handoff. + fn hand_off( + &self, + episode: Episode, + tier: usize, + release_name: &dyn Fn(), + ) -> Result<(), Refusal> { + let rung = self.rung(tier)?; + let previous = self.store.read(); + if episode.generation == 0 { + // A new token supersedes every earlier episode, so their claims are + // spent evidence. Clearing them here — never the current token's — + // is what keeps the directory from growing one file per relaunch. + self.store.reset_claims(); + } + let record = episode::prepare(&episode, self.package, tier, &self.version); + self.store + .write(&record) + .map_err(Refusal::StateNotDurable)?; + + let tag = Tag { + episode: Some(episode), + profile: rung.name.to_string(), + }; + match self.spawn_child(rung, &tag, release_name) { + Ok(()) => Ok(()), + Err(refusal) => { + // No child exists, so the prepared record describes an attempt + // that will never be claimed. Left in place it would spend the + // rung's one retry on a failure that was never about rendering. + match previous { + Some(record) => self.write(&record), + None => { + self.store.clear(); + } + } + Err(refusal) + } + } + } + + /// Hand off to a child running `tier` with no episode: nothing is prepared, + /// nothing is claimed, and nothing is persisted. This is what a manual + /// override is — a launch the ladder runs but does not learn from. + fn force(&self, tier: usize) -> Result<(), Refusal> { + let rung = self.rung(tier)?; + let tag = Tag { + episode: None, + profile: rung.name.to_string(), + }; + self.spawn_child(rung, &tag, &|| {}) + } + + fn spawn_child( + &self, + rung: &'static Tier, + tag: &Tag, + release_name: &dyn Fn(), + ) -> Result<(), Refusal> { + // Resolved before the name is released: a failure here is a refusal + // that must leave this process still holding the name. + let binary = tauri::process::current_binary(&tauri::Env::default()) + .map_err(|error| error.to_string()); + release_name(); + let pid = (self.launch)(&self.dbus_name, binary, &self.args, self.package, rung, tag)?; + eprintln!("{LOG}: handed off to {} as pid {pid}", rung.name); + Ok(()) + } + + fn rung(&self, tier: usize) -> Result<&'static Tier, Refusal> { + self.package + .tier(tier) + .ok_or_else(|| Refusal::NoTier(format!("{} has no tier {tier}", self.package.label()))) + } + + /// Record that no profile survived, so later launches stop here instead of + /// walking the whole ladder again on every start. + fn note_exhausted(&self) { + let episode = self.episode.clone().unwrap_or_else(episode::fresh); + let mut record = self.record(Phase::Exhausted, &episode); + record.pid = Some(std::process::id()); + self.write(&record); + eprintln!( + "{LOG}: ladder exhausted on {} at tier {} ({}); \ + relaunch with {} to force the safest profile", + self.package.label(), + self.tier, + self.profile_name(), + cli::SAFE_RENDERING + ); + } + + fn record(&self, phase: Phase, episode: &Episode) -> Record { + Record::new( + phase, + &episode.token, + episode.generation, + self.profile_name(), + self.tier, + &self.version, + ) + } + + fn write(&self, record: &Record) { + if let Err(error) = self.store.write(record) { + eprintln!("{LOG}: failed to persist {:?}: {error}", record.phase); + } + } +} + +fn pid_alive(pid: u32) -> bool { + // Signal 0 runs the existence and permission checks without delivering + // anything, and unlike a /proc lookup it is not fooled by a pid namespace. + unsafe { libc::kill(pid as libc::pid_t, 0) == 0 } +} diff --git a/desktop/src-tauri/src/render_recovery/state.rs b/desktop/src-tauri/src/render_recovery/state.rs new file mode 100644 index 0000000000..5103688eeb --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/state.rs @@ -0,0 +1,217 @@ +//! Durable episode state: the `prepared → started → owned → confirmed` record +//! and the exclusive `(token, generation)` claim. +//! +//! Phases are monotonic and each post-parent phase records the identity that +//! makes the episode verifiable — a pid, then the D-Bus unique name and bus id. +//! Only the child that wins the exclusive claim ever writes a receipt. +//! +//! Claims are keyed on `(token, generation)` rather than the token alone. The +//! sole retry reuses the same token, and an exclusive claim is one-shot by +//! design, so a token-only key would make the retry lose its own claim against +//! the first attempt's file. Claim files are never deleted: a stale claim is +//! inert evidence that a generation already ran, and deleting one cannot be +//! fused with the record write into a single durable step. + +use serde::{Deserialize, Serialize}; +use std::io::Write as _; +use std::path::{Path, PathBuf}; + +/// Where the record and claim files live, relative to the app data dir. +const DIR: &str = "render-recovery"; +const RECORD: &str = "episode.json"; +const CLAIMS: &str = "claims"; + +/// The sole retry generation. An attempt whose child never claimed is retried +/// exactly once, under generation 1; there is no generation 2. +pub(crate) const RETRY_GENERATION: u32 = 1; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub(crate) enum Phase { + /// Written by the parent, atomically, before spawning. + Prepared, + /// Written by the exclusively claiming child as its first action. + Started, + /// Written by the same child once it holds the single-instance name. + Owned, + /// Written by the same child once the startup boundary passes. Doubles as + /// the persisted "last crash-free startup profile" fact. + Confirmed, + /// The ladder ran out of rungs for this package. Terminal. + Exhausted, +} + +/// The durable record. One file, rewritten atomically at each transition. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub(crate) struct Record { + pub phase: Phase, + /// Attempt identity. A new rung mints a new token; the retry reuses it. + pub token: String, + /// Retry identity within the attempt: 0, and `RETRY_GENERATION` for the + /// sole retry. + pub generation: u32, + /// Tier name, e.g. `shm-transport`. Recorded for the log; the tier index + /// is what the ladder actually reads. + pub profile: String, + /// Tier index into the package's ladder. + pub tier: usize, + /// App version, so a persisted profile does not survive an upgrade. + pub version: String, + /// The claiming child's pid. Present from `started` onward. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pid: Option, + /// The child's D-Bus unique name. Present from `owned` onward. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unique_name: Option, + /// The bus the unique name was issued on. A unique name is only meaningful + /// against the bus that minted it; a different bus can reuse `:1.4`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bus_id: Option, +} + +impl Record { + pub(crate) fn new( + phase: Phase, + token: &str, + generation: u32, + profile: &str, + tier: usize, + version: &str, + ) -> Self { + Record { + phase, + token: token.to_string(), + generation, + profile: profile.to_string(), + tier, + version: version.to_string(), + pid: None, + unique_name: None, + bus_id: None, + } + } +} + +/// Filesystem home of the episode record and its claim files. +pub(crate) struct Store { + dir: PathBuf, +} + +/// Outcome of an exclusive claim attempt. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Claim { + /// This process owns the transition and must write the receipt. + Won, + /// Another process holds this `(token, generation)`. Write nothing. + Lost, + /// The claim could not be attempted. Also write nothing — a claim that + /// cannot be proven exclusive is not a claim. + Failed(String), +} + +impl Store { + pub(crate) fn new(app_data_dir: &Path) -> Self { + Store { + dir: app_data_dir.join(DIR), + } + } + + fn record_path(&self) -> PathBuf { + self.dir.join(RECORD) + } + + fn claim_path(&self, token: &str, generation: u32) -> PathBuf { + self.dir.join(CLAIMS).join(format!("{token}-g{generation}")) + } + + /// Read the record. An unreadable or unparsable record is `None` — the + /// caller treats that as absent and starts from the baseline. + pub(crate) fn read(&self) -> Option { + let text = std::fs::read_to_string(self.record_path()).ok()?; + serde_json::from_str(&text).ok() + } + + /// Atomically replace the record. Durability matters here: a torn record + /// after a crash would be indistinguishable from a corrupt one and would + /// silently restart the ladder. + pub(crate) fn write(&self, record: &Record) -> Result<(), String> { + use atomic_write_file::AtomicWriteFile; + + std::fs::create_dir_all(&self.dir) + .map_err(|e| format!("create {}: {e}", self.dir.display()))?; + let path = self.record_path(); + let json = serde_json::to_vec_pretty(record).map_err(|e| format!("encode record: {e}"))?; + let mut file = + AtomicWriteFile::open(&path).map_err(|e| format!("open {}: {e}", path.display()))?; + file.write_all(&json) + .map_err(|e| format!("write {}: {e}", path.display()))?; + file.commit() + .map_err(|e| format!("commit {}: {e}", path.display())) + } + + /// Delete the record and every claim. Used by `--reset-rendering-mode` and + /// when a record cannot be parsed. Returns whether anything was there. + pub(crate) fn clear(&self) -> bool { + let existed = self.dir.exists(); + let _ = std::fs::remove_dir_all(&self.dir); + existed + } + + /// Drop the claim files of superseded episodes, before a fresh one is + /// prepared. + /// + /// This is not the deletion the contract forbids. That rule protects the + /// *current* token, whose sole retry must not be able to erase its own + /// first attempt's evidence — hence the `(token, generation)` key. A claim + /// belonging to an older token can strand nothing: a delayed child of a + /// superseded episode is turned away by the token check against the record + /// long before it looks at a claim file. Without this the directory grows + /// by one file per relaunch, forever. + pub(crate) fn reset_claims(&self) { + let _ = std::fs::remove_dir_all(self.dir.join(CLAIMS)); + } + + /// Take exclusive ownership of one `(token, generation)`. + /// + /// `create_new` is `O_EXCL` at the filesystem level: of N racing processes + /// exactly one gets `Ok`. This is the compare-and-transition primitive — + /// a read-then-rename would let two children both pass the read and the + /// loser write the last receipt. + pub(crate) fn claim(&self, token: &str, generation: u32) -> Claim { + let path = self.claim_path(token, generation); + if let Some(parent) = path.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + return Claim::Failed(format!("create {}: {e}", parent.display())); + } + } + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(mut file) => { + let _ = writeln!(file, "pid={}", std::process::id()); + Claim::Won + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => Claim::Lost, + Err(e) => Claim::Failed(format!("claim {}: {e}", path.display())), + } + } + + #[cfg(test)] + pub(crate) fn record_path_for_test(&self) -> PathBuf { + self.record_path() + } + + #[cfg(test)] + pub(crate) fn claim_count(&self) -> usize { + std::fs::read_dir(self.dir.join(CLAIMS)) + .map(|entries| entries.count()) + .unwrap_or(0) + } + + #[cfg(test)] + pub(crate) fn seed_claim(&self, token: &str, generation: u32) { + assert_eq!(self.claim(token, generation), Claim::Won); + } +} diff --git a/desktop/src-tauri/src/render_recovery/tests.rs b/desktop/src-tauri/src/render_recovery/tests.rs new file mode 100644 index 0000000000..574b1cda8f --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/tests.rs @@ -0,0 +1,950 @@ +//! Behavioural tests for the recovery ladder. +//! +//! Every rule the ladder depends on is exercised against real files and real +//! `Command` environments; nothing here spawns a process or touches the session +//! bus, because the parts that do are the two thin edges (`dbus::observe`, +//! `Command::spawn`) whose outcomes are injected as inputs. + +use std::ffi::{OsStr, OsString}; + +use super::classify::{decide, Decision, Ownership}; +use super::cli; +use super::dbus::{Observation, Owner}; +use super::episode::{ + advance, advances_ladder, claim, fresh, prepare, retry_of, Advance, Claimed, Episode, + Termination, CRASH_ELIGIBILITY_WINDOW, +}; +use super::launcher::{exact_env_command, Refusal, Tag, EPISODE, FORCED, GENERATION, PROFILE}; +use super::profiles::{owned_present, Package}; +use super::session::{reconcile, Boot, Disabled}; +use super::state::{Phase, Record, Store}; + +const VERSION: &str = "0.4.26"; +const IDENTIFIER: &str = "xyz.block.buzz.app"; +const TOKEN: &str = "11111111-2222-3333-4444-555555555555"; + +fn store() -> (tempfile::TempDir, Store) { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + (dir, store) +} + +fn record(phase: Phase, generation: u32, tier: usize) -> Record { + Record::new(phase, TOKEN, generation, "shm-transport", tier, VERSION) +} + +fn env_from(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let owned: Vec<(String, String)> = pairs + .iter() + .map(|(key, value)| (key.to_string(), value.to_string())) + .collect(); + move |key: &str| { + owned + .iter() + .find(|(name, _)| name == key) + .map(|(_, value)| value.clone()) + } +} + +fn owner(unique_name: &str, pid: Option) -> Observation { + Observation::Owned(Owner { + unique_name: unique_name.to_string(), + pid, + }) +} + +fn dead(_pid: u32) -> bool { + false +} + +fn alive(_pid: u32) -> bool { + true +} + +/// A spawn edge that always refuses, standing in for a bus that reports the +/// name still owned. No process is ever forked from a test. +fn refuse( + _name: &str, + _binary: Result, + _args: &[OsString], + _package: Package, + _tier: &'static super::profiles::Tier, + _tag: &Tag, +) -> Result { + Err(Refusal::NameNotFree( + "owned by :1.9 (pid Some(99))".to_string(), + )) +} + +/// A spawn edge that reports a child was launched, without launching one. +fn accept( + _name: &str, + _binary: Result, + _args: &[OsString], + _package: Package, + _tier: &'static super::profiles::Tier, + _tag: &Tag, +) -> Result { + Ok(4242) +} + +// ── Generation races ──────────────────────────────────────────────────────── +// +// The claim is keyed on `(token, generation)` because the sole retry reuses the +// token. These three cases are the reason for that key. + +#[test] +fn test_stale_generation_claim_does_not_block_the_retry() { + let (_dir, store) = store(); + // Generation 0 ran and claimed; its child then died before writing. + store.seed_claim(TOKEN, 0); + store.write(&record(Phase::Prepared, 1, 1)).expect("write"); + + let claimed = claim( + &store, + &Episode { + token: TOKEN.to_string(), + generation: 1, + }, + ); + + assert!(matches!(claimed, Claimed::Owner { tier: 1, .. })); + assert_eq!(store.read().expect("record").phase, Phase::Started); +} + +#[test] +fn test_two_children_of_one_generation_produce_exactly_one_receipt() { + let (_dir, store) = store(); + store.write(&record(Phase::Prepared, 1, 1)).expect("write"); + let episode = Episode { + token: TOKEN.to_string(), + generation: 1, + }; + + let first = claim(&store, &episode); + let second = claim(&store, &episode); + + assert!(matches!(first, Claimed::Owner { .. })); + assert!(matches!(second, Claimed::Yielded(_))); + assert_eq!(store.claim_count(), 1); + let written = store.read().expect("record"); + assert_eq!(written.phase, Phase::Started); + assert_eq!(written.pid, Some(std::process::id())); +} + +#[test] +fn test_delayed_child_of_a_superseded_generation_writes_nothing() { + let (_dir, store) = store(); + // The retry has already been prepared when generation 0's child finally + // gets scheduled. Adopting the record's generation here would let it write + // the retry's receipt. + let prepared = record(Phase::Prepared, 1, 1); + store.write(&prepared).expect("write"); + + let claimed = claim( + &store, + &Episode { + token: TOKEN.to_string(), + generation: 0, + }, + ); + + assert!(matches!(claimed, Claimed::Yielded(_))); + assert_eq!(store.read().expect("record"), prepared); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_claiming_an_already_started_episode_yields() { + let (_dir, store) = store(); + store.write(&record(Phase::Started, 0, 1)).expect("write"); + + let claimed = claim( + &store, + &Episode { + token: TOKEN.to_string(), + generation: 0, + }, + ); + + assert!(matches!(claimed, Claimed::Yielded(_))); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_claiming_another_tokens_episode_yields() { + let (_dir, store) = store(); + store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + + let claimed = claim( + &store, + &Episode { + token: "a-different-token".to_string(), + generation: 0, + }, + ); + + assert!(matches!(claimed, Claimed::Yielded(_))); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_retry_reuses_the_token_and_advances_the_generation() { + let retry = retry_of(&record(Phase::Prepared, 0, 2)); + assert_eq!(retry.token, TOKEN); + assert_eq!(retry.generation, 1); +} + +#[test] +fn test_fresh_episodes_are_distinct_at_generation_zero() { + let (first, second) = (fresh(), fresh()); + assert_ne!(first.token, second.token); + assert_eq!(first.generation, 0); +} + +// ── Injected deaths at the four commit points ─────────────────────────────── + +#[test] +fn test_death_before_the_claim_retries_the_same_tier() { + let decision = decide( + &record(Phase::Prepared, 0, 2), + &Observation::Free, + None, + VERSION, + &dead, + ); + assert!(matches!(decision, Decision::RetrySameTier { tier: 2, .. })); +} + +#[test] +fn test_death_before_the_claim_with_the_retry_spent_advances() { + let decision = decide( + &record(Phase::Prepared, 1, 2), + &Observation::Free, + None, + VERSION, + &dead, + ); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 2, .. })); +} + +#[test] +fn test_death_after_the_claim_before_ownership_advances() { + let mut started = record(Phase::Started, 0, 1); + started.pid = Some(4242); + let decision = decide(&started, &Observation::Free, None, VERSION, &dead); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 1, .. })); +} + +#[test] +fn test_death_after_ownership_before_confirm_advances() { + let mut owned = record(Phase::Owned, 0, 1); + owned.pid = Some(4242); + owned.unique_name = Some(":1.7".to_string()); + owned.bus_id = Some("bus-a".to_string()); + let decision = decide(&owned, &Observation::Free, Some("bus-a"), VERSION, &dead); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 1, .. })); +} + +#[test] +fn test_death_after_confirm_reuses_the_profile() { + let mut confirmed = record(Phase::Confirmed, 0, 2); + confirmed.unique_name = Some(":1.7".to_string()); + confirmed.bus_id = Some("bus-a".to_string()); + let decision = decide( + &confirmed, + &Observation::Free, + Some("bus-a"), + VERSION, + &dead, + ); + assert_eq!(decision, Decision::ReuseProfile { tier: 2 }); +} + +// ── Live episodes and handoff failure ─────────────────────────────────────── + +#[test] +fn test_live_child_in_the_pre_ownership_interval_defers() { + let mut started = record(Phase::Started, 0, 1); + started.pid = Some(4242); + let decision = decide(&started, &Observation::Free, None, VERSION, &alive); + assert!(matches!(decision, Decision::Defer(_))); +} + +#[test] +fn test_recorded_child_owning_the_name_defers() { + let mut started = record(Phase::Started, 0, 1); + started.pid = Some(4242); + let decision = decide(&started, &owner(":1.7", Some(4242)), None, VERSION, &dead); + assert!(matches!(decision, Decision::Defer(_))); +} + +#[test] +fn test_a_stranger_owning_the_name_is_handoff_failure() { + let mut started = record(Phase::Started, 0, 1); + started.pid = Some(4242); + let decision = decide(&started, &owner(":1.7", Some(9999)), None, VERSION, &dead); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 1, .. })); +} + +#[test] +fn test_an_unidentifiable_owner_is_handoff_failure_not_a_free_name() { + let mut started = record(Phase::Started, 0, 1); + started.pid = Some(4242); + // No owner pid: the bus refused GetConnectionUnixProcessID. + let decision = decide(&started, &owner(":1.7", None), None, VERSION, &dead); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 1, .. })); +} + +#[test] +fn test_an_unreachable_bus_is_never_read_as_a_free_name() { + let mut owned = record(Phase::Owned, 0, 1); + owned.unique_name = Some(":1.7".to_string()); + owned.bus_id = Some("bus-a".to_string()); + let observation = Observation::Unavailable("no session bus".to_string()); + assert_eq!( + super::classify::correlate(&owned, &observation, Some("bus-a")), + Ownership::Uncorrelatable + ); +} + +#[test] +fn test_a_receipt_from_another_bus_does_not_correlate() { + let mut owned = record(Phase::Owned, 0, 1); + owned.unique_name = Some(":1.7".to_string()); + owned.bus_id = Some("bus-a".to_string()); + // Same unique name, different bus — `:1.7` is only meaningful on the bus + // that issued it. + let decision = decide( + &owned, + &owner(":1.7", Some(1)), + Some("bus-b"), + VERSION, + &dead, + ); + assert!(matches!(decision, Decision::AdvanceOrStop { tier: 1, .. })); +} + +#[test] +fn test_the_recorded_owner_still_holding_the_name_defers() { + let mut owned = record(Phase::Owned, 0, 1); + owned.unique_name = Some(":1.7".to_string()); + owned.bus_id = Some("bus-a".to_string()); + let decision = decide( + &owned, + &owner(":1.7", Some(1)), + Some("bus-a"), + VERSION, + &dead, + ); + assert!(matches!(decision, Decision::Defer(_))); +} + +#[test] +fn test_a_confirmed_instance_still_running_defers() { + let mut confirmed = record(Phase::Confirmed, 0, 2); + confirmed.unique_name = Some(":1.7".to_string()); + confirmed.bus_id = Some("bus-a".to_string()); + let decision = decide( + &confirmed, + &owner(":1.7", Some(1)), + Some("bus-a"), + VERSION, + &dead, + ); + assert!(matches!(decision, Decision::Defer(_))); +} + +// ── The crash-free startup fact, and what advances the ladder ─────────────── + +#[test] +fn test_a_confirmed_profile_from_another_version_is_discarded() { + let confirmed = record(Phase::Confirmed, 0, 2); + let decision = decide(&confirmed, &Observation::Free, None, "0.4.27", &dead); + assert!(matches!(decision, Decision::DiscardAndBaseline { .. })); +} + +#[test] +fn test_only_a_crash_inside_the_startup_window_advances_the_ladder() { + let inside = CRASH_ELIGIBILITY_WINDOW - std::time::Duration::from_millis(1); + let outside = CRASH_ELIGIBILITY_WINDOW + std::time::Duration::from_millis(1); + + assert!(advances_ladder(Termination::Crashed, inside)); + assert!(advances_ladder( + Termination::Crashed, + CRASH_ELIGIBILITY_WINDOW + )); + // A crash after the window is a startup-shaped failure, logged but never + // charged against the ladder. + assert!(!advances_ladder(Termination::Crashed, outside)); + // An OOM kill or our own terminate_web_process is not a rendering failure. + assert!(!advances_ladder(Termination::Other, inside)); +} + +// ── Case 10: the per-package cap and terminal behaviour ───────────────────── + +#[test] +fn test_appimage_has_four_tiers_and_native_five() { + assert_eq!(Package::AppImage.tiers().len(), 4); + assert_eq!(Package::Native.tiers().len(), 5); + // The AppImage ladder stops before the x11 tier: linuxdeploy's AppRun hook + // already pins GDK_BACKEND=x11, so that tier would spend a relaunch on a + // no-op. + assert!(Package::AppImage.tier_named("x11-fallback").is_none()); + assert_eq!(Package::Native.tier_named("x11-fallback"), Some(4)); +} + +#[test] +fn test_no_tier_sets_the_compositing_variable() { + // Held out of the table pending the 3-var vs 4-var isolation on #2643, + // while still owned so a user value opts out. + for package in [Package::AppImage, Package::Native] { + assert!(package + .owned_vars() + .contains(&"WEBKIT_DISABLE_COMPOSITING_MODE")); + for tier in package.tiers() { + assert!(!tier + .vars + .iter() + .any(|(key, _)| *key == "WEBKIT_DISABLE_COMPOSITING_MODE")); + } + } +} + +#[test] +fn test_the_ladder_spends_one_attempt_per_tier_then_stops() { + for package in [Package::AppImage, Package::Native] { + let terminal = package.terminal_tier(); + for tier in 0..terminal { + assert_eq!(advance(package, tier), Advance::Tier(tier + 1)); + } + assert_eq!(advance(package, terminal), Advance::Exhausted); + // Nothing past the terminal tier is reachable, so the cap cannot be + // exceeded by a record claiming a tier that does not exist. + assert_eq!(advance(package, terminal + 1), Advance::Exhausted); + } +} + +#[test] +fn test_an_exhausted_record_stops_without_advancing() { + let decision = decide( + &record(Phase::Exhausted, 0, 3), + &Observation::Free, + None, + VERSION, + &dead, + ); + assert_eq!(decision, Decision::StopExhausted { tier: 3 }); +} + +// ── The environment contract ──────────────────────────────────────────────── + +#[test] +fn test_any_owned_variable_disables_recovery_whatever_its_value() { + // `0` and empty are the two values a truthiness check would wrongly ignore. + for value in ["1", "0", ""] { + let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", value)]); + let present = owned_present(Package::Native, &env); + assert_eq!( + present, + vec![format!("WEBKIT_DISABLE_DMABUF_RENDERER={value}")] + ); + + let dir = tempfile::tempdir().expect("tempdir"); + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Off(Disabled::UserEnv(vars)) => assert_eq!(vars, present), + _ => panic!("a user assignment of an owned var must disable recovery"), + } + } +} + +#[test] +fn test_appimage_does_not_treat_the_pinned_gdk_backend_as_user_input() { + // The AppRun hook exports GDK_BACKEND=x11 before the binary starts, so its + // provenance is unrecoverable and owning it would disable the ladder on + // every AppImage launch. + let env = env_from(&[("APPIMAGE", "/tmp/Buzz.AppImage"), ("GDK_BACKEND", "x11")]); + assert_eq!(Package::detect(&env), Package::AppImage); + assert!(owned_present(Package::AppImage, &env).is_empty()); + assert!(!Package::AppImage.owned_vars().contains(&"GDK_BACKEND")); + + let native = env_from(&[("GDK_BACKEND", "x11")]); + assert_eq!(Package::detect(&native), Package::Native); + assert_eq!(owned_present(Package::Native, &native).len(), 1); +} + +#[test] +fn test_a_recovery_child_does_not_read_its_injected_profile_as_user_input() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + + // Exactly what the parent hands a tier-1 child. + let env = env_from(&[ + ("WEBKIT_DMABUF_RENDERER_FORCE_SHM", "1"), + (PROFILE, "shm-transport"), + (EPISODE, TOKEN), + (GENERATION, "0"), + ]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => assert_eq!(session.profile_name(), "shm-transport"), + _ => panic!("a tagged child runs the environment it was given"), + } + assert_eq!(store.read().expect("record").phase, Phase::Started); +} + +#[test] +fn test_a_child_that_loses_its_claim_runs_on_without_writing() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + store.seed_claim(TOKEN, 0); + + let env = env_from(&[ + ("WEBKIT_DMABUF_RENDERER_FORCE_SHM", "1"), + (PROFILE, "shm-transport"), + (EPISODE, TOKEN), + (GENERATION, "0"), + ]); + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(_) => {} + _ => panic!("a claim loser still runs; the plugin decides its fate"), + } + assert_eq!(store.read().expect("record").phase, Phase::Prepared); +} + +#[test] +fn test_a_tag_without_a_generation_is_rejected_rather_than_read_as_zero() { + // Defaulting a malformed tag to generation 0 would let it claim the + // original attempt of an episode that has already been retried. + let env = env_from(&[(PROFILE, "shm-transport"), (EPISODE, TOKEN)]); + assert_eq!(Tag::read(&env), None); + + let complete = env_from(&[ + (PROFILE, "shm-transport"), + (EPISODE, TOKEN), + (GENERATION, "1"), + ]); + let tag = Tag::read(&complete).expect("tag"); + assert_eq!(tag.episode.expect("episode").generation, 1); +} + +#[test] +fn test_a_forced_child_carries_a_profile_but_no_episode() { + let env = env_from(&[(PROFILE, "no-accel"), (FORCED, "1")]); + let tag = Tag::read(&env).expect("tag"); + assert_eq!(tag.profile, "no-accel"); + assert_eq!(tag.episode, None); +} + +// ── The exact-environment child command ───────────────────────────────────── + +fn envs_of(command: &std::process::Command) -> Vec<(String, Option)> { + let mut envs: Vec<(String, Option)> = command + .get_envs() + .map(|(key, value)| { + ( + key.to_string_lossy().into_owned(), + value.map(|v| v.to_string_lossy().into_owned()), + ) + }) + .collect(); + envs.sort(); + envs +} + +#[test] +fn test_the_child_environment_is_the_tier_and_nothing_inherited() { + let package = Package::Native; + let tier = package.tier(1).expect("tier 1"); + let tag = Tag { + episode: Some(Episode { + token: TOKEN.to_string(), + generation: 1, + }), + profile: tier.name.to_string(), + }; + let command = exact_env_command("/bin/true".into(), &[], package, tier, &tag); + let envs = envs_of(&command); + + // Every owned variable is cleared, so a previous tier's profile can never + // be inherited into the next attempt. + for key in package.owned_vars() { + let cleared = envs + .iter() + .any(|(name, value)| name == key && value.is_none()); + let set = envs + .iter() + .any(|(name, value)| name == key && value.is_some()); + assert!(cleared != set, "{key} must be either cleared or set once"); + } + assert_eq!( + envs.iter() + .find(|(name, _)| name == "WEBKIT_DMABUF_RENDERER_FORCE_SHM"), + Some(&( + "WEBKIT_DMABUF_RENDERER_FORCE_SHM".to_string(), + Some("1".to_string()) + )) + ); + assert_eq!( + envs.iter().find(|(name, _)| name == GENERATION), + Some(&(GENERATION.to_string(), Some("1".to_string()))) + ); + // A prepared child is never also a forced child. + assert_eq!( + envs.iter().find(|(name, _)| name == FORCED), + Some(&(FORCED.to_string(), None)) + ); +} + +#[test] +fn test_the_appimage_child_leaves_the_pinned_backend_alone() { + let package = Package::AppImage; + let tier = package.tier(3).expect("terminal AppImage tier"); + let tag = Tag { + episode: None, + profile: tier.name.to_string(), + }; + let command = exact_env_command("/bin/true".into(), &[], package, tier, &tag); + // Removing GDK_BACKEND here would be theatre: AppRun re-exports it before + // the child binary starts. + assert!(!envs_of(&command) + .iter() + .any(|(name, _)| name == "GDK_BACKEND")); +} + +#[test] +fn test_the_child_inherits_the_parents_arguments() { + let package = Package::Native; + let tier = package.tier(1).expect("tier 1"); + let tag = Tag { + episode: None, + profile: tier.name.to_string(), + }; + let args = [OsString::from("buzz://channel/1")]; + let command = exact_env_command("/bin/true".into(), &args, package, tier, &tag); + assert_eq!( + command.get_args().collect::>(), + vec![OsStr::new("buzz://channel/1")] + ); +} + +// ── Flags and reset ───────────────────────────────────────────────────────── + +#[test] +fn test_the_renderer_flags_parse_independently_of_position() { + let args = [ + OsStr::new("buzz://channel/1"), + OsStr::new("--safe-rendering"), + ]; + let flags = cli::parse(args); + assert!(flags.safe_rendering); + assert!(!flags.reset_rendering_mode); + + let none = cli::parse([OsStr::new("--safe-renderingX")]); + assert!(!none.safe_rendering); +} + +#[test] +fn test_safe_rendering_forces_the_terminal_tier_without_persisting() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.write(&record(Phase::Confirmed, 0, 0)).expect("write"); + let env = env_from(&[]); + + // Native, so the terminal tier is x11-fallback and the handoff would need a + // real spawn — the assertion is that the record is left untouched. + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::SAFE_RENDERING)], + &env, + refuse, + ); + assert!(!matches!(boot, Boot::Reset)); + let after = store.read().expect("record"); + assert_eq!(after.phase, Phase::Confirmed); + assert_eq!(after.tier, 0); +} + +#[test] +fn test_reset_rendering_mode_clears_the_state_and_does_not_launch() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.write(&record(Phase::Confirmed, 0, 2)).expect("write"); + store.seed_claim(TOKEN, 0); + let env = env_from(&[]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::RESET_RENDERING_MODE)], + &env, + refuse, + ); + + assert!(matches!(boot, Boot::Reset)); + assert_eq!(store.read(), None); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_recovery_is_off_when_no_state_directory_exists() { + let env = env_from(&[]); + let boot = reconcile( + Err("no user data directory".to_string()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ); + assert!(matches!(boot, Boot::Off(Disabled::NoStateDir(_)))); +} + +#[test] +fn test_a_baseline_launch_runs_here_without_writing_a_record() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => assert_eq!(session.profile_name(), "full-gpu"), + _ => panic!("a first launch runs tier 0 in this process"), + } + assert_eq!(store.read(), None); +} + +// ── Durability ────────────────────────────────────────────────────────────── + +#[test] +fn test_an_unparsable_record_reads_as_absent() { + let (_dir, store) = store(); + store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + let path = store.record_path_for_test(); + std::fs::write(&path, b"{not json").expect("corrupt"); + assert_eq!(store.read(), None); +} + +#[test] +fn test_a_record_survives_a_round_trip_with_its_owner_identity() { + let (_dir, store) = store(); + let mut written = record(Phase::Owned, 1, 2); + written.pid = Some(4242); + written.unique_name = Some(":1.7".to_string()); + written.bus_id = Some("bus-a".to_string()); + store.write(&written).expect("write"); + assert_eq!(store.read(), Some(written)); +} + +#[test] +fn test_preparing_a_record_names_the_tier_it_will_run() { + let prepared = prepare( + &Episode { + token: TOKEN.to_string(), + generation: 0, + }, + Package::AppImage, + 2, + VERSION, + ); + assert_eq!(prepared.phase, Phase::Prepared); + assert_eq!(prepared.profile, "cpu-raster"); + assert_eq!(prepared.tier, 2); + assert_eq!(prepared.version, VERSION); +} + +// ── Forced launches, refused handoffs, and reaching reset ─────────────────── +// +// Three rules that only hold at the boot seam: a manual override must not teach +// the ladder anything, a handoff that never happened must not cost a retry, and +// the escape hatch must stay reachable for the user most likely to need it. + +#[test] +fn test_a_forced_child_runs_its_profile_without_writing_a_record() { + // A --safe-rendering parent hands off with BUZZ_RENDER_FORCED and no + // episode. The child must run the profile and persist nothing: a forced + // launch is not evidence about any tier. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[(PROFILE, "no-accel"), (FORCED, "1")]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => assert_eq!(session.profile_name(), "no-accel"), + _ => panic!("a forced child runs in this process"), + } + assert_eq!(store.read(), None); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_a_forced_child_that_crashes_does_not_advance_or_persist() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[(PROFILE, "cpu-raster"), (FORCED, "1")]); + + // `accept`, not `refuse`: with a spawn edge that succeeds, a forced launch + // that was wrongly tracked would really hand off and leave a record behind. + // Against `refuse` the rollback would hide the difference. + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("a forced child runs in this process"); + }; + + // Even a genuine startup crash must leave a forced launch untracked — + // otherwise the manual override would rewrite the ladder's own record. + assert!(!session.on_web_process_terminated(Termination::Crashed, &|| {})); + assert_eq!(store.read(), None); + // The confirmation receipt is the other write a forced launch must not make: + // it is the persisted "last crash-free startup profile" fact, and a manual + // override is not evidence about any tier. + session.note_owned(); + session.note_confirmed(); + assert_eq!(store.read(), None); +} + +#[test] +fn test_a_refused_handoff_does_not_spend_the_rungs_retry() { + // A confirmed tier-2 record: this launch wants to re-run tier 2, which + // needs a child. The spawn is refused, so the durable state must be exactly + // what it was — a leftover `prepared` record would make the next launch + // burn this rung's one retry on a failure that was never about rendering. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let before = record(Phase::Confirmed, 0, 2); + store.write(&before).expect("write"); + let env = env_from(&[]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => { + // The handoff did not happen, so this process is still the baseline + // it was launched as and must not claim the tier it wanted. + assert_eq!(session.profile_name(), "full-gpu"); + } + _ => panic!("a refused handoff leaves the app running here"), + } + assert_eq!(store.read(), Some(before)); +} + +#[test] +fn test_a_refused_handoff_from_no_record_leaves_no_record() { + // Same rule with nothing on disk to roll back to: a `prepared` record + // written for a child that was never spawned must be cleared, not left as + // an unclaimed attempt for the next launch to inherit. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + // A crash at tier 0 wants tier 1, so the parent prepares then spawns. + let env = env_from(&[]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) else { + panic!("a first launch runs tier 0 here"); + }; + + assert!(!session.on_web_process_terminated(Termination::Crashed, &|| {})); + assert_eq!(store.read(), None); +} + +#[test] +fn test_a_successful_handoff_leaves_the_prepared_record_for_the_child() { + // The mirror of the rollback: when the child really is launched, the + // prepared record must survive for it to claim. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("a first launch runs tier 0 here"); + }; + + assert!(session.on_web_process_terminated(Termination::Crashed, &|| {})); + let prepared = store.read().expect("the child's record"); + assert_eq!(prepared.phase, Phase::Prepared); + assert_eq!(prepared.tier, 1); + assert_eq!(prepared.generation, 0); +} + +#[test] +fn test_reset_rendering_mode_works_even_with_an_owned_var_set() { + // The user who set WEBKIT_DISABLE_DMABUF_RENDERER by hand is exactly the + // user most likely to be clearing state an earlier run left behind. If the + // opt-out returned first, the escape hatch would be unreachable for them. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.write(&record(Phase::Confirmed, 0, 2)).expect("write"); + let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "1")]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::RESET_RENDERING_MODE)], + &env, + refuse, + ); + + assert!(matches!(boot, Boot::Reset)); + assert_eq!(store.read(), None); +} diff --git a/desktop/src-tauri/src/render_recovery/wiring.rs b/desktop/src-tauri/src/render_recovery/wiring.rs new file mode 100644 index 0000000000..068cc3ac67 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/wiring.rs @@ -0,0 +1,85 @@ +//! The Tauri plugin that carries a `Session` into the running app. +//! +//! Three attachments, and nothing else lives here: +//! +//! * plugin setup — the single-instance plugin is registered before this one, +//! so by now the name is held and the `owned` receipt can name this +//! connection; +//! * a boundary thread — writes the crash-free startup receipt once the startup +//! window closes; +//! * the `main` webview's `web-process-terminated` signal — the crash edge. +//! +//! Tauri's own `on_web_content_process_terminate` hook is macOS/iOS only +//! (`tauri-2.11.5/src/webview/mod.rs`), so the crash edge is taken straight +//! from `webkit2gtk`, which is already in the dependency graph via wry. + +use std::sync::Arc; + +use tauri::plugin::TauriPlugin; +use tauri::{Manager, Runtime}; +use webkit2gtk::{WebProcessTerminationReason, WebViewExt}; + +use super::episode::Termination; +use super::session::Session; +use super::LOG; + +/// Everything the ladder needs from the live app. +pub(crate) fn plugin(session: Arc) -> TauriPlugin { + let for_webview = Arc::clone(&session); + tauri::plugin::Builder::::new("render-recovery") + .setup(move |_app, _api| { + session.note_owned(); + confirm_at_startup_boundary(Arc::clone(&session)); + Ok(()) + }) + .on_webview_ready(move |webview| { + if webview.label() != "main" { + return; + } + let session = Arc::clone(&for_webview); + let app = webview.app_handle().clone(); + if let Err(error) = webview.with_webview(move |platform| { + platform + .inner() + .connect_web_process_terminated(move |_, reason| { + let handed_off = session + .on_web_process_terminated(classify(reason), &|| { + tauri_plugin_single_instance::destroy(&app) + }); + // A child owns the app now. Exit through Tauri so the + // ordinary shutdown path still runs; two Buzz processes + // must never be live at once. + if handed_off { + app.exit(0); + } + }); + }) { + eprintln!("{LOG}: could not attach the crash hook: {error}"); + } + }) + .build() +} + +/// Write the crash-free startup receipt once the startup window closes. +/// +/// A plain thread rather than an async task: it sleeps for the whole window and +/// then does blocking file and D-Bus work, which is the opposite of what an +/// async worker is for. +fn confirm_at_startup_boundary(session: Arc) { + std::thread::spawn(move || { + std::thread::sleep(session.until_confirmation()); + session.note_confirmed(); + }); +} + +/// Reduce WebKit's termination reason to the only distinction the ladder makes. +/// +/// The enum is `#[non_exhaustive]`, and the fallback arm is deliberately the +/// one that never downgrades anything: an unknown future reason must not cost a +/// user their hardware rendering. +fn classify(reason: WebProcessTerminationReason) -> Termination { + match reason { + WebProcessTerminationReason::Crashed => Termination::Crashed, + _ => Termination::Other, + } +} From d5a6e10f36d9bcbfaedfd8866f3634f8c12f41ea Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 03:28:48 -0400 Subject: [PATCH 2/4] fix(desktop): serialize renderer recovery transitions and type the release boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two safety invariants in the recovery ladder were not enforced. A refusal raised after the single-instance name was released left the parent running without the name and unable to take it back, because the launcher's Result<(), Refusal> could not express which side of that irreversible boundary the refusal came from — and binary resolution, the one preflight most likely to fail, happened inside the launcher. Both refusal sides are now distinct outcomes, every preflight runs before the release, and a post-release refusal exits instead of lingering. The exclusive claim serialized claim-file creation, not record transitions: read-validate-write ran as three unsynchronized steps, so a delayed generation could stamp a stale receipt over a newer episode, a duplicated crash callback could spawn a second child, and a claim loser could reach the single-instance name ahead of the true owner. Every transition now runs under a crash-releasing advisory lock on a file outside the cleared directory, receipts must name the current (token, generation) and move the phase forward, the eligible-crash path is one-shot, and claim losers exit before Tauri is built. --safe-rendering against a user-set owned variable was silently dropped; it is now a fatal pre-Tauri error naming the conflicting assignments. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/lib.rs | 13 +- .../src-tauri/src/render_recovery/episode.rs | 71 +++- .../src-tauri/src/render_recovery/launcher.rs | 47 ++- .../src-tauri/src/render_recovery/session.rs | 385 ++++++++++++++---- .../src-tauri/src/render_recovery/state.rs | 136 ++++++- .../src-tauri/src/render_recovery/tests.rs | 318 ++------------- .../src/render_recovery/tests/boot.rs | 373 +++++++++++++++++ .../src/render_recovery/tests/transitions.rs | 337 +++++++++++++++ .../src-tauri/src/render_recovery/wiring.rs | 23 +- 9 files changed, 1293 insertions(+), 410 deletions(-) create mode 100644 desktop/src-tauri/src/render_recovery/tests/boot.rs create mode 100644 desktop/src-tauri/src/render_recovery/tests/transitions.rs diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 0eddc2293c..31dd1c5584 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -151,8 +151,17 @@ pub fn run() { render_recovery::Boot::Run(session) => Some(session), // Recovery is off for this launch; the app still starts normally. render_recovery::Boot::Off(_) => None, - // A child is Buzz now, or --reset-rendering-mode did its work. - render_recovery::Boot::HandedOff | render_recovery::Boot::Reset => return, + // A child is Buzz now, --reset-rendering-mode did its work, or another + // process owns this episode and this one must not compete for the name. + render_recovery::Boot::HandedOff + | render_recovery::Boot::Reset + | render_recovery::Boot::Superseded => return, + // The user asked for something that cannot be delivered. Say so and + // exit non-zero rather than starting an app that ignores the request. + render_recovery::Boot::Fatal(diagnostic) => { + eprintln!("buzz-desktop: {diagnostic}"); + std::process::exit(1); + } }; // mesh-llm's async chains (model download, node start/join) overflow diff --git a/desktop/src-tauri/src/render_recovery/episode.rs b/desktop/src-tauri/src/render_recovery/episode.rs index 1f48a08448..7141dd0287 100644 --- a/desktop/src-tauri/src/render_recovery/episode.rs +++ b/desktop/src-tauri/src/render_recovery/episode.rs @@ -78,9 +78,16 @@ pub(crate) enum Claimed { /// This process exclusively owns `episode` at `tier`, and `started` is /// durable. Owner { episode: Episode, tier: usize }, - /// This process is not the owner. It writes nothing and runs on as an - /// ordinary launch; the single-instance plugin decides its fate. - Yielded(&'static str), + /// Another process is this episode's owner, or a newer episode has + /// superseded it. This child must exit *without* competing for the + /// single-instance name: if it took the name ahead of the true owner, the + /// owner would exit as a duplicate while the durable `started` receipt + /// still named it. + Superseded(&'static str), + /// Nobody owns this episode and nobody else is going to. Exiting here would + /// cost the user their window for nothing, so this child runs on with the + /// environment it was handed and the ladder simply stops tracking it. + Untracked(&'static str), } /// Exclusively claim the prepared record this child was launched for. @@ -89,35 +96,51 @@ pub(crate) enum Claimed { /// time a delayed child runs, a retry may have prepared a newer generation, and /// adopting the record's generation would let the stale child write the newer /// generation's receipt. +/// +/// The read, the claim, and the `started` write all happen inside one +/// transaction. Split across three unsynchronized steps they were not enough: a +/// child could read its own generation as current, be descheduled while another +/// invocation durably prepared the next generation, then resume and stamp its +/// stale `started` over the newer record. pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { + let tx = match store.lock() { + Ok(tx) => tx, + Err(error) => { + eprintln!("{}: could not take the state lock: {error}", super::LOG); + return Claimed::Untracked("the state lock could not be taken"); + } + }; + let Some(record) = store.read() else { - return Claimed::Yielded("no episode record"); + return Claimed::Untracked("no episode record"); }; if record.token != episode.token || record.generation != episode.generation { - return Claimed::Yielded("this episode is no longer current"); + return Claimed::Superseded("this episode is no longer current"); } if record.phase != Phase::Prepared { - return Claimed::Yielded("this episode was already claimed"); + return Claimed::Superseded("this episode was already claimed"); } - match store.claim(&episode.token, episode.generation) { + match store.claim(&tx, &episode.token, episode.generation) { Claim::Won => {} - Claim::Lost => return Claimed::Yielded("another child claimed this episode first"), + Claim::Lost => return Claimed::Superseded("another child claimed this episode first"), Claim::Failed(error) => { + // No receipt exists to falsify, and the sibling child would hit the + // same filesystem error, so there is nothing here to defer to. eprintln!("{}: claim failed: {error}", super::LOG); - return Claimed::Yielded("the claim could not be taken exclusively"); + return Claimed::Untracked("the claim could not be taken exclusively"); } } let mut started = record.clone(); started.phase = Phase::Started; started.pid = Some(std::process::id()); - if let Err(error) = store.write(&started) { - // The claim is taken and cannot be given back, so yielding here would - // strand the episode. Run on instead: the record still says `prepared`, - // and the next launch retries or advances it. + if let Err(error) = store.write(&tx, &started) { + // The claim is taken and cannot be given back, so this process is the + // owner whether or not the receipt landed. Run on untracked: the record + // still says `prepared`, and the next launch retries or advances it. eprintln!("{}: failed to persist started: {error}", super::LOG); - return Claimed::Yielded("the started receipt could not be persisted"); + return Claimed::Untracked("the started receipt could not be persisted"); } Claimed::Owner { @@ -126,6 +149,26 @@ pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { } } +/// Whether `episode` may still write `next` over `current`. +/// +/// Two independent conditions, both necessary. The identity check rejects a +/// receipt from an episode that is no longer current — a `confirmed` write +/// racing the crash handoff that already prepared the next token would +/// otherwise resurrect the superseded attempt. The phase check rejects a +/// receipt that does not move the episode forward, so a duplicated or +/// out-of-order callback cannot walk the record backwards. +pub(crate) fn may_advance(current: Option<&Record>, episode: &Episode, next: Phase) -> bool { + let Some(current) = current else { + // Nothing on disk to advance. The record a receipt belongs to is + // written by the parent before the child exists, so its absence means + // this episode was cleared — not that a receipt should create it. + return false; + }; + current.token == episode.token + && current.generation == episode.generation + && current.phase.precedes(next) +} + /// The record a parent must make durable before spawning a child for `tier`. pub(crate) fn prepare(episode: &Episode, package: Package, tier: usize, version: &str) -> Record { let name = package.tier(tier).map(|tier| tier.name).unwrap_or_default(); diff --git a/desktop/src-tauri/src/render_recovery/launcher.rs b/desktop/src-tauri/src/render_recovery/launcher.rs index 1115da31dd..09cd0423a9 100644 --- a/desktop/src-tauri/src/render_recovery/launcher.rs +++ b/desktop/src-tauri/src/render_recovery/launcher.rs @@ -66,7 +66,7 @@ impl Tag { } } -/// Why a relaunch did not happen. Every arm leaves the current process running. +/// Why a relaunch did not happen. #[derive(Debug)] pub(crate) enum Refusal { /// The name has an owner, or the bus could not answer. Either way this is @@ -84,6 +84,37 @@ pub(crate) enum Refusal { NoTier(String), } +/// The outcome of a handoff attempt, with the release of the single-instance +/// name made visible in the type. +/// +/// A plain `Result<(), Refusal>` conflated the two refusals, and the difference +/// is the whole safety property: releasing the name is irreversible, so a +/// caller that has released it can never simply run on. Only the caller knows +/// whether its release closure actually released anything — at boot no name is +/// held yet — so this type reports *where* the refusal happened and leaves the +/// consequence to the call site. +#[derive(Debug)] +pub(crate) enum Handoff { + /// A child was spawned with the tier's exact environment. It is Buzz now. + Launched, + /// Refused while the caller still held everything it started with. Nothing + /// was released; running on is safe. + RefusedBeforeRelease(Refusal), + /// Refused after the release closure ran. For a live app that closure + /// destroyed the single-instance plugin, so there is no way back. + RefusedAfterRelease(Refusal), +} + +impl Handoff { + /// Whether no child was launched, so any state prepared for one must be + /// rolled back. Deliberately blind to *which* side of the release it was: + /// the rollback is the same either way, and only the process lifecycle + /// decision cares about the side. + pub(crate) fn refused(&self) -> bool { + !matches!(self, Handoff::Launched) + } +} + impl std::fmt::Display for Refusal { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -133,9 +164,12 @@ pub(crate) fn exact_env_command( /// The spawn edge, as a plain function pointer so tests can drive the refusal /// and success branches without ever forking a real process. +/// +/// Takes an already-resolved binary: resolving it is a preflight that must +/// happen before the name is released, so it cannot be part of this edge. pub(crate) type Launch = fn( &str, - Result, + &std::path::Path, &[std::ffi::OsString], Package, &'static Tier, @@ -144,12 +178,17 @@ pub(crate) type Launch = fn( /// Prove the name is free, then spawn the attempt. /// +/// Everything here is necessarily *after* the caller released the name: the +/// probe is only meaningful once this process has stopped holding the name +/// itself. That is why the caller must treat a refusal from this function as +/// terminal, and why every check that can be made earlier is made earlier. +/// /// Any prepared record must already be durable: the child claims it as its /// first action, and a child that arrives before the record does would find /// nothing to claim. pub(crate) fn spawn( dbus_name: &str, - binary: Result, + binary: &std::path::Path, args: &[std::ffi::OsString], package: Package, tier: &Tier, @@ -166,7 +205,7 @@ pub(crate) fn spawn( Observation::Unavailable(detail) => return Err(Refusal::NameNotFree(detail)), } - exact_env_command(binary.map_err(Refusal::NoBinary)?, args, package, tier, tag) + exact_env_command(binary.to_path_buf(), args, package, tier, tag) .spawn() .map(|child| child.id()) .map_err(|error| Refusal::SpawnFailed(error.to_string())) diff --git a/desktop/src-tauri/src/render_recovery/session.rs b/desktop/src-tauri/src/render_recovery/session.rs index 968f693056..97fc63a4ba 100644 --- a/desktop/src-tauri/src/render_recovery/session.rs +++ b/desktop/src-tauri/src/render_recovery/session.rs @@ -14,9 +14,9 @@ use super::classify::{self, Decision}; use super::cli; use super::dbus::{self, Observation}; use super::episode::{self, Advance, Claimed, Episode, Termination}; -use super::launcher::{self, Refusal, Tag}; +use super::launcher::{self, Handoff, Refusal, Tag}; use super::profiles::{self, Env, Package, Tier}; -use super::state::{Phase, Record, Store}; +use super::state::{Phase, Record, Store, Transaction}; use super::LOG; /// Why the ladder is not running this launch. In every case the app starts @@ -56,6 +56,14 @@ pub(crate) enum Boot { Reset, /// Recovery is off this launch, but the app still starts. Off(Disabled), + /// Another process owns this episode. Exit without starting the app and + /// without competing for the single-instance name. The reason is already + /// logged where it was decided, so nothing is carried here. + Superseded, + /// The user asked for something that cannot be delivered. Exit before Tauri + /// with a diagnostic and a non-zero status rather than starting an app that + /// silently ignores the request. + Fatal(String), } /// Everything the running process needs to react to a web-process crash. @@ -74,7 +82,8 @@ pub(crate) struct Session { /// and by any child that could not claim the episode it was sent to run. frozen: bool, /// Set once a ladder-eligible crash is seen, so a process that observed one - /// and failed to hand off never goes on to record a crash-free startup. + /// and failed to hand off never goes on to record a crash-free startup, and + /// so the handoff itself happens at most once per process. crashed: AtomicBool, /// When this process started, for the crash-eligibility window. launched: std::time::Instant, @@ -83,6 +92,48 @@ pub(crate) struct Session { launch: launcher::Launch, } +/// What this process must give up before a child may take the single-instance +/// name — and, decisively, whether giving it up can be undone. +/// +/// The two callers of the launcher sit on opposite sides of that question, and +/// a bare closure could not tell them apart: at boot no name is held yet, so a +/// refusal is harmless, while a live app has to destroy its single-instance +/// plugin and can never re-register it. Naming the distinction here is what +/// stops a refusal from being mistaken for a recoverable one. +enum Release<'a> { + /// Boot, before Tauri exists. Nothing is held, so nothing is released. + NothingHeld, + /// A live app holding the name. Running this destroys the single-instance + /// plugin; there is no way back. + SingleInstanceName(&'a dyn Fn()), +} + +impl Release<'_> { + /// Release, and report whether the process just crossed an irreversible + /// boundary. + fn run(&self) -> bool { + match self { + Release::NothingHeld => false, + Release::SingleInstanceName(destroy) => { + destroy(); + true + } + } + } +} + +/// What a live app must do about a web-process termination. +pub(crate) enum CrashResponse { + /// A child carrying the next tier is Buzz now. Exit. + HandedOff, + /// The relaunch was refused *after* the single-instance name was released. + /// This process no longer owns the name it needs to be the app, and cannot + /// take it back, so it must exit rather than linger as a second instance. + Stranded, + /// Nothing was released and nothing was launched. Carry on. + Continue, +} + /// Reconcile persisted state and select this launch's renderer tier. /// /// `identifier` is the bundle identifier, which names both the app data dir and @@ -156,7 +207,17 @@ pub(super) fn reconcile( if tag.is_none() { let present = profiles::owned_present(package, env); if !present.is_empty() { - return Boot::Off(Disabled::UserEnv(present)); + // `--safe-rendering` and a user-set owned variable are two + // incompatible answers to the same question, and the ladder has no + // basis for picking one: honouring the flag would overwrite + // configuration the user typed, and honouring the environment would + // silently ignore a rescue flag from a user whose app will not + // start. So neither is guessed — the request is refused, loudly, + // before Tauri. + return match flags.safe_rendering { + true => Boot::Fatal(conflict_message(&present)), + false => Boot::Off(Disabled::UserEnv(present)), + }; } } @@ -166,13 +227,45 @@ pub(super) fn reconcile( } } +/// The diagnostic for `--safe-rendering` against a user-set owned variable. +/// +/// `present` carries `KEY=value` assignments, so it both shows the user what is +/// set and names the keys to unset — the two things needed to act on this. +fn conflict_message(present: &[String]) -> String { + let keys: Vec<&str> = present + .iter() + .map(|assignment| match assignment.split_once('=') { + Some((key, _)) => key, + None => assignment.as_str(), + }) + .collect(); + format!( + "{} cannot be applied: {} already set in the environment. \ + Either unset {} and run {} again, or keep that environment and drop \ + the flag.", + cli::SAFE_RENDERING, + present.join(", "), + keys.join(", "), + cli::SAFE_RENDERING, + ) +} + impl Session { /// `--reset-rendering-mode`: delete the persisted tier and episode state, /// say what was cleared, and exit without launching. fn reset(self) -> Boot { + let cleared = match self.store.lock() { + Ok(tx) => self.store.clear(&tx), + Err(error) => { + // Another process is mid-transition. Clearing without the lock + // could delete a record a live child is about to claim, so this + // reports the refusal rather than racing it. + return Boot::Fatal(format!("could not clear renderer state: {error}")); + } + }; println!( "{}", - match self.store.clear() { + match cleared { true => "Cleared the persisted renderer profile and episode state.", false => "No persisted renderer profile or episode state to clear.", } @@ -216,7 +309,9 @@ impl Session { // and let the single-instance plugin forward argv and exit us. Decision::Defer(_) => Boot::Run(Arc::new(self)), Decision::DiscardAndBaseline { .. } => { - self.store.clear(); + if let Ok(tx) = self.store.lock() { + self.store.clear(&tx); + } self.run_selected_tier() } // The persisted "last crash-free startup profile" fact. @@ -304,16 +399,26 @@ impl Session { Claimed::Owner { episode, tier } => { self.tier = tier; self.episode = Some(episode); + Boot::Run(Arc::new(self)) + } + // Someone else owns this episode. Exiting here rather than running + // on is the point: a loser that continued into Tauri could win the + // single-instance name ahead of the true owner, and the owner would + // then exit as a duplicate while the durable `started` receipt still + // named it — a record pointing at a dead process, which the next + // launch reads as a failed handoff and charges to the ladder. + Claimed::Superseded(reason) => { + eprintln!("{LOG}: recovery child is not the episode owner — {reason}; exiting"); + Boot::Superseded } - // A child that is not the owner writes nothing and runs on as an - // ordinary launch. The single-instance plugin decides whether it is - // a duplicate; the ladder simply stops tracking it. - Claimed::Yielded(reason) => { - eprintln!("{LOG}: recovery child is not the episode owner — {reason}"); + // No owner to defer to, so exiting would cost the user their window + // for nothing. Run the environment that was handed over, untracked. + Claimed::Untracked(reason) => { + eprintln!("{LOG}: not tracking this recovery launch — {reason}"); self.frozen = true; + Boot::Run(Arc::new(self)) } } - Boot::Run(Arc::new(self)) } pub(crate) fn profile_name(&self) -> &'static str { @@ -345,6 +450,15 @@ impl Session { /// Write a phase receipt for the episode this process owns. /// + /// Two conditions gate the write, both under the transaction lock. The + /// identity check (`episode::may_advance`) rejects a receipt for an episode + /// that is no longer current: `note_confirmed` runs from a timer thread and + /// can be racing a crash handoff that has already prepared the next token, + /// and without the check it would overwrite that token with the superseded + /// episode's `confirmed`. The phase check rejects a receipt that does not + /// move the record forward, so a repeated or out-of-order callback cannot + /// walk it backwards. + /// /// The receipt is bound to the bus id as well as the unique name: a unique /// name like `:1.4` only identifies a connection on the bus that issued it, /// so without the bus id a receipt from an earlier session's bus could @@ -353,6 +467,15 @@ impl Session { let Some(episode) = &self.episode else { return; }; + let Ok(tx) = self.store.lock() else { + eprintln!("{LOG}: skipping the {phase:?} receipt — the state lock was unavailable"); + return; + }; + if !episode::may_advance(self.store.read().as_ref(), episode, phase) { + eprintln!("{LOG}: skipping the {phase:?} receipt — this episode is no longer current"); + return; + } + let mut record = self.record(phase, episode); record.pid = Some(std::process::id()); match dbus::observe(&self.dbus_name) { @@ -365,16 +488,15 @@ impl Session { // identity as uncorrelatable rather than as ours. _ => eprintln!("{LOG}: writing {phase:?} without an owner identity"), } - self.write(&record); + self.write(&tx, &record); } - /// React to a web-process termination. Returns `true` when a child took - /// over and this process must exit. + /// React to a web-process termination. pub(crate) fn on_web_process_terminated( &self, termination: Termination, - release_name: &dyn Fn(), - ) -> bool { + destroy_single_instance: &dyn Fn(), + ) -> CrashResponse { let elapsed = self.launched.elapsed(); if !episode::advances_ladder(termination, elapsed) { eprintln!( @@ -383,40 +505,64 @@ impl Session { self.tier, self.profile_name() ); - return false; + return CrashResponse::Continue; + } + // One-shot: the signal can fire again while a handoff is in flight (a + // second webview, or WebKit respawning and re-crashing), and a second + // handoff would spawn a second child for a second episode — two live + // Buzz processes racing one name. `swap` makes the first caller the only + // one that proceeds; later callers still see `crashed` set, so they also + // suppress the crash-free receipt. + if self.crashed.swap(true, Ordering::SeqCst) { + eprintln!("{LOG}: a ladder-eligible crash was already handled; not advancing again"); + return CrashResponse::Continue; } - self.crashed.store(true, Ordering::SeqCst); if self.frozen { eprintln!("{LOG}: not advancing — this launch is running a forced profile"); - return false; + return CrashResponse::Continue; } match episode::advance(self.package, self.tier) { Advance::Exhausted => { self.note_exhausted(); - false + CrashResponse::Continue } - Advance::Tier(next) => match self.hand_off(episode::fresh(), next, release_name) { - Ok(()) => true, - Err(refusal) => { - eprintln!("{LOG}: relaunch refused: {refusal}"); - false + Advance::Tier(next) => { + let release = Release::SingleInstanceName(destroy_single_instance); + match self.hand_off(episode::fresh(), next, &release) { + Handoff::Launched => CrashResponse::HandedOff, + // Nothing was released, so this process is still the app. + Handoff::RefusedBeforeRelease(refusal) => { + eprintln!("{LOG}: relaunch refused: {refusal}"); + CrashResponse::Continue + } + // The single-instance plugin is destroyed and cannot be + // re-registered. Staying would leave a Buzz that no longer + // owns the name — a later launch would start a second app + // beside it, and deep links would go to whichever won. + Handoff::RefusedAfterRelease(refusal) => { + eprintln!( + "{LOG}: relaunch refused after releasing the single-instance name \ + ({refusal}); exiting rather than running without it" + ); + CrashResponse::Stranded + } } - }, + } } } /// Hand off to a child at `tier`, or run on here if the handoff is refused. - /// Only reached before the single-instance name exists, so there is nothing - /// to release first. + /// Only reached before Tauri exists, so nothing is held and every refusal + /// leaves this process exactly as it was launched. fn hand_off_or_run(mut self, episode: Option, tier: usize) -> Boot { let handed = match episode { - Some(episode) => self.hand_off(episode, tier, &|| {}), + Some(episode) => self.hand_off(episode, tier, &Release::NothingHeld), None => self.force(tier), }; match handed { - Ok(()) => Boot::HandedOff, - Err(refusal) => { + Handoff::Launched => Boot::HandedOff, + Handoff::RefusedBeforeRelease(refusal) => { eprintln!("{LOG}: staying at the launched profile — {refusal}"); // The handoff did not happen, so this process is still the // baseline it was launched as and must not claim otherwise. @@ -424,6 +570,13 @@ impl Session { self.frozen = true; Boot::Run(Arc::new(self)) } + // Unreachable by construction: `Release::NothingHeld` releases + // nothing, so there is no post-release side to land on. Handled as a + // refusal rather than a panic — if the invariant ever breaks, the + // safe reading of "we may have released something" is to not run. + Handoff::RefusedAfterRelease(refusal) => Boot::Fatal(format!( + "renderer handoff refused after an unexpected release: {refusal}" + )), } } @@ -435,42 +588,74 @@ impl Session { /// otherwise find nothing. The name must be released before the probe, /// because a child that fails to take the name forwards its argv and exits /// 0 — indistinguishable from a successful handoff. - fn hand_off( - &self, - episode: Episode, - tier: usize, - release_name: &dyn Fn(), - ) -> Result<(), Refusal> { - let rung = self.rung(tier)?; + fn hand_off(&self, episode: Episode, tier: usize, release: &Release<'_>) -> Handoff { + let rung = match self.rung(tier) { + Ok(rung) => rung, + Err(refusal) => return Handoff::RefusedBeforeRelease(refusal), + }; + let tx = match self.store.lock() { + Ok(tx) => tx, + Err(error) => { + return Handoff::RefusedBeforeRelease(Refusal::StateNotDurable(error)); + } + }; let previous = self.store.read(); + let record = episode::prepare(&episode, self.package, tier, &self.version); + if let Err(error) = self.store.write(&tx, &record) { + return Handoff::RefusedBeforeRelease(Refusal::StateNotDurable(error)); + } if episode.generation == 0 { - // A new token supersedes every earlier episode, so their claims are - // spent evidence. Clearing them here — never the current token's — - // is what keeps the directory from growing one file per relaunch. - self.store.reset_claims(); + // Only now that the new record is durable. Pruning first would, on a + // failed record write, leave the old record current with its claim + // evidence already gone — and an old child that had passed its + // record read could then re-create its claim and write a receipt + // over the record. + self.store.prune_claims(&tx, &episode.token); } - let record = episode::prepare(&episode, self.package, tier, &self.version); - self.store - .write(&record) - .map_err(Refusal::StateNotDurable)?; + // Released before the spawn: the record is durable and every check that + // does not need the name released has already run. + drop(tx); let tag = Tag { episode: Some(episode), profile: rung.name.to_string(), }; - match self.spawn_child(rung, &tag, release_name) { - Ok(()) => Ok(()), - Err(refusal) => { - // No child exists, so the prepared record describes an attempt - // that will never be claimed. Left in place it would spend the - // rung's one retry on a failure that was never about rendering. - match previous { - Some(record) => self.write(&record), - None => { - self.store.clear(); - } - } - Err(refusal) + let handoff = self.spawn_child(rung, &tag, release); + if handoff.refused() { + // No child exists, so the prepared record describes an attempt that + // will never be claimed. Left in place it would spend the rung's one + // retry on a failure that was never about rendering. + self.roll_back(&record, previous); + } + handoff + } + + /// Restore the record a refused handoff had already overwritten. + /// + /// Identity-checked, because the lock is dropped between the prepared write + /// and the spawn: by the time a refusal comes back, a concurrent launch may + /// have prepared its own episode, and a blind restore would erase a record + /// that a live child is about to claim — turning that child's handoff into + /// an untracked launch. Only the record this call itself wrote is rolled + /// back. + fn roll_back(&self, prepared: &Record, previous: Option) { + let Ok(tx) = self.store.lock() else { + eprintln!("{LOG}: could not roll back the prepared record — the state lock was busy"); + return; + }; + match self.store.read() { + Some(current) + if current.token == prepared.token && current.generation == prepared.generation => { + } + _ => { + eprintln!("{LOG}: not rolling back — another episode is current now"); + return; + } + } + match previous { + Some(record) => self.write(&tx, &record), + None => { + self.store.clear(&tx); } } } @@ -478,29 +663,53 @@ impl Session { /// Hand off to a child running `tier` with no episode: nothing is prepared, /// nothing is claimed, and nothing is persisted. This is what a manual /// override is — a launch the ladder runs but does not learn from. - fn force(&self, tier: usize) -> Result<(), Refusal> { - let rung = self.rung(tier)?; - let tag = Tag { - episode: None, - profile: rung.name.to_string(), - }; - self.spawn_child(rung, &tag, &|| {}) + fn force(&self, tier: usize) -> Handoff { + match self.rung(tier) { + Ok(rung) => { + let tag = Tag { + episode: None, + profile: rung.name.to_string(), + }; + self.spawn_child(rung, &tag, &Release::NothingHeld) + } + Err(refusal) => Handoff::RefusedBeforeRelease(refusal), + } } - fn spawn_child( - &self, - rung: &'static Tier, - tag: &Tag, - release_name: &dyn Fn(), - ) -> Result<(), Refusal> { - // Resolved before the name is released: a failure here is a refusal - // that must leave this process still holding the name. - let binary = tauri::process::current_binary(&tauri::Env::default()) - .map_err(|error| error.to_string()); - release_name(); - let pid = (self.launch)(&self.dbus_name, binary, &self.args, self.package, rung, tag)?; - eprintln!("{LOG}: handed off to {} as pid {pid}", rung.name); - Ok(()) + /// The release boundary itself: every preflight that can fail runs *before* + /// `release.run()`, and everything after it is reported as a post-release + /// outcome. + /// + /// Resolving the binary belongs on this side of the line. Held as an + /// unresolved `Result` and passed onward it would surface only inside the + /// launcher — after the name was already gone — turning a plainly + /// recoverable "cannot find my own executable" into a stranded app. + fn spawn_child(&self, rung: &'static Tier, tag: &Tag, release: &Release<'_>) -> Handoff { + let binary = match tauri::process::current_binary(&tauri::Env::default()) { + Ok(binary) => binary, + Err(error) => { + return Handoff::RefusedBeforeRelease(Refusal::NoBinary(error.to_string())); + } + }; + + let released = release.run(); + match (self.launch)( + &self.dbus_name, + &binary, + &self.args, + self.package, + rung, + tag, + ) { + Ok(pid) => { + eprintln!("{LOG}: handed off to {} as pid {pid}", rung.name); + Handoff::Launched + } + Err(refusal) => match released { + true => Handoff::RefusedAfterRelease(refusal), + false => Handoff::RefusedBeforeRelease(refusal), + }, + } } fn rung(&self, tier: usize) -> Result<&'static Tier, Refusal> { @@ -511,11 +720,19 @@ impl Session { /// Record that no profile survived, so later launches stop here instead of /// walking the whole ladder again on every start. + /// + /// `Exhausted` outranks every other phase, so this write passes the forward + /// check from anywhere in the chain — which is what makes the terminal state + /// terminal even against a late receipt from the attempt that got here. fn note_exhausted(&self) { let episode = self.episode.clone().unwrap_or_else(episode::fresh); + let Ok(tx) = self.store.lock() else { + eprintln!("{LOG}: could not record the exhausted ladder — the state lock was busy"); + return; + }; let mut record = self.record(Phase::Exhausted, &episode); record.pid = Some(std::process::id()); - self.write(&record); + self.write(&tx, &record); eprintln!( "{LOG}: ladder exhausted on {} at tier {} ({}); \ relaunch with {} to force the safest profile", @@ -537,8 +754,8 @@ impl Session { ) } - fn write(&self, record: &Record) { - if let Err(error) = self.store.write(record) { + fn write(&self, tx: &Transaction, record: &Record) { + if let Err(error) = self.store.write(tx, record) { eprintln!("{LOG}: failed to persist {:?}: {error}", record.phase); } } diff --git a/desktop/src-tauri/src/render_recovery/state.rs b/desktop/src-tauri/src/render_recovery/state.rs index 5103688eeb..80a29312b5 100644 --- a/desktop/src-tauri/src/render_recovery/state.rs +++ b/desktop/src-tauri/src/render_recovery/state.rs @@ -11,15 +11,29 @@ //! the first attempt's file. Claim files are never deleted: a stale claim is //! inert evidence that a generation already ran, and deleting one cannot be //! fused with the record write into a single durable step. +//! +//! An exclusive claim only serializes the creation of one claim file, which is +//! not enough on its own: reading the record, validating it, and writing the +//! next phase are three operations, and a writer descheduled between them could +//! stamp a stale phase onto a newer episode. So every transition runs inside +//! [`Store::lock`], and every receipt additionally has to prove the record is +//! still the one it belongs to (`episode::may_advance`). The lock orders +//! writers; the identity check is what makes a delayed writer harmless. use serde::{Deserialize, Serialize}; use std::io::Write as _; +use std::os::fd::AsRawFd as _; use std::path::{Path, PathBuf}; /// Where the record and claim files live, relative to the app data dir. const DIR: &str = "render-recovery"; const RECORD: &str = "episode.json"; const CLAIMS: &str = "claims"; +/// The transaction lock, a **sibling** of `DIR` rather than a child: `clear()` +/// removes `DIR` wholesale, and a lock inode inside it would be unlinked from +/// under a concurrent holder — leaving two processes each holding a "lock" on a +/// different inode. +const LOCK: &str = "render-recovery.lock"; /// The sole retry generation. An attempt whose child never claimed is retried /// exactly once, under generation 1; there is no generation 2. @@ -41,6 +55,31 @@ pub(crate) enum Phase { Exhausted, } +impl Phase { + /// Position in the `prepared → started → owned → confirmed` chain, used to + /// reject a receipt that would move an episode backwards. + /// + /// `Exhausted` is deliberately above every other phase: it is terminal, and + /// a late receipt from the attempt that exhausted the ladder must not + /// reopen it. + fn rank(self) -> u8 { + match self { + Phase::Prepared => 0, + Phase::Started => 1, + Phase::Owned => 2, + Phase::Confirmed => 3, + Phase::Exhausted => 4, + } + } + + /// Whether `next` is a forward move from `self`. Equal phases are not + /// forward: a repeated receipt has nothing to add, and rejecting it keeps + /// the transition idempotent rather than merely harmless. + pub(crate) fn precedes(self, next: Phase) -> bool { + self.rank() < next.rank() + } +} + /// The durable record. One file, rewritten atomically at each transition. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub(crate) struct Record { @@ -95,6 +134,18 @@ impl Record { /// Filesystem home of the episode record and its claim files. pub(crate) struct Store { dir: PathBuf, + lock: PathBuf, +} + +/// Proof that the caller holds the cross-process transaction lock. +/// +/// Every mutation of the record takes one of these by reference, so a +/// read-validate-write sequence cannot be written without serializing it. The +/// lock is released when this value drops, and also — the property a lockfile +/// alone cannot offer — when the holding process dies, since the kernel closes +/// its descriptors. A crashing parent therefore never wedges the ladder. +pub(crate) struct Transaction { + _file: std::fs::File, } /// Outcome of an exclusive claim attempt. @@ -113,9 +164,42 @@ impl Store { pub(crate) fn new(app_data_dir: &Path) -> Self { Store { dir: app_data_dir.join(DIR), + lock: app_data_dir.join(LOCK), } } + /// Serialize every writer of this store against each other. + /// + /// `flock` rather than an `O_EXCL` lockfile because it is released by the + /// kernel on process death: a lockfile left behind by a process that + /// crashed mid-transition — exactly the failure this feature exists to + /// handle — would need a liveness heuristic to break, and a wrong guess + /// there either wedges recovery forever or defeats the mutual exclusion. + pub(crate) fn lock(&self) -> Result { + if let Some(parent) = self.lock.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| format!("create {}: {e}", parent.display()))?; + } + let file = std::fs::OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(&self.lock) + .map_err(|e| format!("open {}: {e}", self.lock.display()))?; + // Blocking: the critical sections are one small file write each, and a + // caller that gave up on the lock would have to either skip its + // transition or race it — both worse than waiting. + if unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) } != 0 { + return Err(format!( + "lock {}: {}", + self.lock.display(), + std::io::Error::last_os_error() + )); + } + Ok(Transaction { _file: file }) + } + fn record_path(&self) -> PathBuf { self.dir.join(RECORD) } @@ -134,7 +218,10 @@ impl Store { /// Atomically replace the record. Durability matters here: a torn record /// after a crash would be indistinguishable from a corrupt one and would /// silently restart the ladder. - pub(crate) fn write(&self, record: &Record) -> Result<(), String> { + /// + /// Requires a `Transaction`, so the read this write is based on and the + /// write itself are always inside one critical section. + pub(crate) fn write(&self, _tx: &Transaction, record: &Record) -> Result<(), String> { use atomic_write_file::AtomicWriteFile; std::fs::create_dir_all(&self.dir) @@ -151,24 +238,38 @@ impl Store { /// Delete the record and every claim. Used by `--reset-rendering-mode` and /// when a record cannot be parsed. Returns whether anything was there. - pub(crate) fn clear(&self) -> bool { + pub(crate) fn clear(&self, _tx: &Transaction) -> bool { let existed = self.dir.exists(); let _ = std::fs::remove_dir_all(&self.dir); existed } - /// Drop the claim files of superseded episodes, before a fresh one is - /// prepared. + /// Drop the claim files of superseded episodes, keeping every claim that + /// belongs to `keep_token`. /// /// This is not the deletion the contract forbids. That rule protects the /// *current* token, whose sole retry must not be able to erase its own - /// first attempt's evidence — hence the `(token, generation)` key. A claim - /// belonging to an older token can strand nothing: a delayed child of a - /// superseded episode is turned away by the token check against the record - /// long before it looks at a claim file. Without this the directory grows - /// by one file per relaunch, forever. - pub(crate) fn reset_claims(&self) { - let _ = std::fs::remove_dir_all(self.dir.join(CLAIMS)); + /// first attempt's evidence — hence the `(token, generation)` key and the + /// filter here. A claim belonging to an older token can strand nothing: a + /// delayed child of a superseded episode is turned away by the token check + /// against the record long before it looks at a claim file. Without this + /// the directory grows by one file per relaunch, forever. + /// + /// Called only *after* the new record is durable. Pruning first would, on a + /// failed record write, leave the old record current with its claim + /// evidence already gone — and an old child that had passed its record read + /// could then re-create its claim and write a receipt over the record. + pub(crate) fn prune_claims(&self, _tx: &Transaction, keep_token: &str) { + let claims = self.dir.join(CLAIMS); + let Ok(entries) = std::fs::read_dir(&claims) else { + return; + }; + let keep = format!("{keep_token}-g"); + for entry in entries.flatten() { + if !entry.file_name().to_string_lossy().starts_with(&keep) { + let _ = std::fs::remove_file(entry.path()); + } + } } /// Take exclusive ownership of one `(token, generation)`. @@ -177,7 +278,7 @@ impl Store { /// exactly one gets `Ok`. This is the compare-and-transition primitive — /// a read-then-rename would let two children both pass the read and the /// loser write the last receipt. - pub(crate) fn claim(&self, token: &str, generation: u32) -> Claim { + pub(crate) fn claim(&self, _tx: &Transaction, token: &str, generation: u32) -> Claim { let path = self.claim_path(token, generation); if let Some(parent) = path.parent() { if let Err(e) = std::fs::create_dir_all(parent) { @@ -212,6 +313,15 @@ impl Store { #[cfg(test)] pub(crate) fn seed_claim(&self, token: &str, generation: u32) { - assert_eq!(self.claim(token, generation), Claim::Won); + let tx = self.lock().expect("lock"); + assert_eq!(self.claim(&tx, token, generation), Claim::Won); + } + + /// Write a record outside a transition, for tests that need a starting + /// state. Takes and drops its own lock. + #[cfg(test)] + pub(crate) fn seed_record(&self, record: &Record) { + let tx = self.lock().expect("lock"); + self.write(&tx, record).expect("write"); } } diff --git a/desktop/src-tauri/src/render_recovery/tests.rs b/desktop/src-tauri/src/render_recovery/tests.rs index 574b1cda8f..19b692c607 100644 --- a/desktop/src-tauri/src/render_recovery/tests.rs +++ b/desktop/src-tauri/src/render_recovery/tests.rs @@ -4,6 +4,14 @@ //! `Command` environments; nothing here spawns a process or touches the session //! bus, because the parts that do are the two thin edges (`dbus::observe`, //! `Command::spawn`) whose outcomes are injected as inputs. +//! +//! This module holds the ladder's own rules — generations, injected deaths, the +//! tier table, and the environment contract — and the fixtures the two +//! submodules share. `boot` covers the pre-Tauri seam (flags, forced launches, +//! refused handoffs); `transitions` covers durable writes under contention. + +mod boot; +mod transitions; use std::ffi::{OsStr, OsString}; @@ -11,12 +19,12 @@ use super::classify::{decide, Decision, Ownership}; use super::cli; use super::dbus::{Observation, Owner}; use super::episode::{ - advance, advances_ladder, claim, fresh, prepare, retry_of, Advance, Claimed, Episode, - Termination, CRASH_ELIGIBILITY_WINDOW, + advance, advances_ladder, claim, fresh, may_advance, prepare, retry_of, Advance, Claimed, + Episode, Termination, CRASH_ELIGIBILITY_WINDOW, }; use super::launcher::{exact_env_command, Refusal, Tag, EPISODE, FORCED, GENERATION, PROFILE}; -use super::profiles::{owned_present, Package}; -use super::session::{reconcile, Boot, Disabled}; +use super::profiles::{owned_present, Package, Tier}; +use super::session::{reconcile, Boot, CrashResponse, Disabled}; use super::state::{Phase, Record, Store}; const VERSION: &str = "0.4.26"; @@ -65,10 +73,10 @@ fn alive(_pid: u32) -> bool { /// name still owned. No process is ever forked from a test. fn refuse( _name: &str, - _binary: Result, + _binary: &std::path::Path, _args: &[OsString], _package: Package, - _tier: &'static super::profiles::Tier, + _tier: &'static Tier, _tag: &Tag, ) -> Result { Err(Refusal::NameNotFree( @@ -79,10 +87,10 @@ fn refuse( /// A spawn edge that reports a child was launched, without launching one. fn accept( _name: &str, - _binary: Result, + _binary: &std::path::Path, _args: &[OsString], _package: Package, - _tier: &'static super::profiles::Tier, + _tier: &'static Tier, _tag: &Tag, ) -> Result { Ok(4242) @@ -98,7 +106,7 @@ fn test_stale_generation_claim_does_not_block_the_retry() { let (_dir, store) = store(); // Generation 0 ran and claimed; its child then died before writing. store.seed_claim(TOKEN, 0); - store.write(&record(Phase::Prepared, 1, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 1, 1)); let claimed = claim( &store, @@ -115,7 +123,7 @@ fn test_stale_generation_claim_does_not_block_the_retry() { #[test] fn test_two_children_of_one_generation_produce_exactly_one_receipt() { let (_dir, store) = store(); - store.write(&record(Phase::Prepared, 1, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 1, 1)); let episode = Episode { token: TOKEN.to_string(), generation: 1, @@ -125,7 +133,7 @@ fn test_two_children_of_one_generation_produce_exactly_one_receipt() { let second = claim(&store, &episode); assert!(matches!(first, Claimed::Owner { .. })); - assert!(matches!(second, Claimed::Yielded(_))); + assert!(matches!(second, Claimed::Superseded(_))); assert_eq!(store.claim_count(), 1); let written = store.read().expect("record"); assert_eq!(written.phase, Phase::Started); @@ -139,7 +147,7 @@ fn test_delayed_child_of_a_superseded_generation_writes_nothing() { // gets scheduled. Adopting the record's generation here would let it write // the retry's receipt. let prepared = record(Phase::Prepared, 1, 1); - store.write(&prepared).expect("write"); + store.seed_record(&prepared); let claimed = claim( &store, @@ -149,7 +157,7 @@ fn test_delayed_child_of_a_superseded_generation_writes_nothing() { }, ); - assert!(matches!(claimed, Claimed::Yielded(_))); + assert!(matches!(claimed, Claimed::Superseded(_))); assert_eq!(store.read().expect("record"), prepared); assert_eq!(store.claim_count(), 0); } @@ -157,7 +165,7 @@ fn test_delayed_child_of_a_superseded_generation_writes_nothing() { #[test] fn test_claiming_an_already_started_episode_yields() { let (_dir, store) = store(); - store.write(&record(Phase::Started, 0, 1)).expect("write"); + store.seed_record(&record(Phase::Started, 0, 1)); let claimed = claim( &store, @@ -167,14 +175,14 @@ fn test_claiming_an_already_started_episode_yields() { }, ); - assert!(matches!(claimed, Claimed::Yielded(_))); + assert!(matches!(claimed, Claimed::Superseded(_))); assert_eq!(store.claim_count(), 0); } #[test] fn test_claiming_another_tokens_episode_yields() { let (_dir, store) = store(); - store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 0, 1)); let claimed = claim( &store, @@ -184,7 +192,7 @@ fn test_claiming_another_tokens_episode_yields() { }, ); - assert!(matches!(claimed, Claimed::Yielded(_))); + assert!(matches!(claimed, Claimed::Superseded(_))); assert_eq!(store.claim_count(), 0); } @@ -484,7 +492,7 @@ fn test_appimage_does_not_treat_the_pinned_gdk_backend_as_user_input() { fn test_a_recovery_child_does_not_read_its_injected_profile_as_user_input() { let dir = tempfile::tempdir().expect("tempdir"); let store = Store::new(dir.path()); - store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 0, 1)); // Exactly what the parent hands a tier-1 child. let env = env_from(&[ @@ -509,10 +517,10 @@ fn test_a_recovery_child_does_not_read_its_injected_profile_as_user_input() { } #[test] -fn test_a_child_that_loses_its_claim_runs_on_without_writing() { +fn test_a_child_that_loses_its_claim_exits_instead_of_racing_the_owner() { let dir = tempfile::tempdir().expect("tempdir"); let store = Store::new(dir.path()); - store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 0, 1)); store.seed_claim(TOKEN, 0); let env = env_from(&[ @@ -521,6 +529,9 @@ fn test_a_child_that_loses_its_claim_runs_on_without_writing() { (EPISODE, TOKEN), (GENERATION, "0"), ]); + // Exiting, not running on: a loser that reached Tauri could take the + // single-instance name ahead of the owner named in the durable `started` + // receipt, and the owner would then exit as a duplicate. match reconcile( Ok(dir.path().to_path_buf()), IDENTIFIER, @@ -529,8 +540,8 @@ fn test_a_child_that_loses_its_claim_runs_on_without_writing() { &env, refuse, ) { - Boot::Run(_) => {} - _ => panic!("a claim loser still runs; the plugin decides its fate"), + Boot::Superseded => {} + _ => panic!("a claim loser must exit before competing for the name"), } assert_eq!(store.read().expect("record").phase, Phase::Prepared); } @@ -651,107 +662,12 @@ fn test_the_child_inherits_the_parents_arguments() { ); } -// ── Flags and reset ───────────────────────────────────────────────────────── - -#[test] -fn test_the_renderer_flags_parse_independently_of_position() { - let args = [ - OsStr::new("buzz://channel/1"), - OsStr::new("--safe-rendering"), - ]; - let flags = cli::parse(args); - assert!(flags.safe_rendering); - assert!(!flags.reset_rendering_mode); - - let none = cli::parse([OsStr::new("--safe-renderingX")]); - assert!(!none.safe_rendering); -} - -#[test] -fn test_safe_rendering_forces_the_terminal_tier_without_persisting() { - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - store.write(&record(Phase::Confirmed, 0, 0)).expect("write"); - let env = env_from(&[]); - - // Native, so the terminal tier is x11-fallback and the handoff would need a - // real spawn — the assertion is that the record is left untouched. - let boot = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - vec![OsString::from(cli::SAFE_RENDERING)], - &env, - refuse, - ); - assert!(!matches!(boot, Boot::Reset)); - let after = store.read().expect("record"); - assert_eq!(after.phase, Phase::Confirmed); - assert_eq!(after.tier, 0); -} - -#[test] -fn test_reset_rendering_mode_clears_the_state_and_does_not_launch() { - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - store.write(&record(Phase::Confirmed, 0, 2)).expect("write"); - store.seed_claim(TOKEN, 0); - let env = env_from(&[]); - - let boot = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - vec![OsString::from(cli::RESET_RENDERING_MODE)], - &env, - refuse, - ); - - assert!(matches!(boot, Boot::Reset)); - assert_eq!(store.read(), None); - assert_eq!(store.claim_count(), 0); -} - -#[test] -fn test_recovery_is_off_when_no_state_directory_exists() { - let env = env_from(&[]); - let boot = reconcile( - Err("no user data directory".to_string()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - refuse, - ); - assert!(matches!(boot, Boot::Off(Disabled::NoStateDir(_)))); -} - -#[test] -fn test_a_baseline_launch_runs_here_without_writing_a_record() { - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - let env = env_from(&[]); - - match reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - refuse, - ) { - Boot::Run(session) => assert_eq!(session.profile_name(), "full-gpu"), - _ => panic!("a first launch runs tier 0 in this process"), - } - assert_eq!(store.read(), None); -} - // ── Durability ────────────────────────────────────────────────────────────── #[test] fn test_an_unparsable_record_reads_as_absent() { let (_dir, store) = store(); - store.write(&record(Phase::Prepared, 0, 1)).expect("write"); + store.seed_record(&record(Phase::Prepared, 0, 1)); let path = store.record_path_for_test(); std::fs::write(&path, b"{not json").expect("corrupt"); assert_eq!(store.read(), None); @@ -764,7 +680,7 @@ fn test_a_record_survives_a_round_trip_with_its_owner_identity() { written.pid = Some(4242); written.unique_name = Some(":1.7".to_string()); written.bus_id = Some("bus-a".to_string()); - store.write(&written).expect("write"); + store.seed_record(&written); assert_eq!(store.read(), Some(written)); } @@ -784,167 +700,3 @@ fn test_preparing_a_record_names_the_tier_it_will_run() { assert_eq!(prepared.tier, 2); assert_eq!(prepared.version, VERSION); } - -// ── Forced launches, refused handoffs, and reaching reset ─────────────────── -// -// Three rules that only hold at the boot seam: a manual override must not teach -// the ladder anything, a handoff that never happened must not cost a retry, and -// the escape hatch must stay reachable for the user most likely to need it. - -#[test] -fn test_a_forced_child_runs_its_profile_without_writing_a_record() { - // A --safe-rendering parent hands off with BUZZ_RENDER_FORCED and no - // episode. The child must run the profile and persist nothing: a forced - // launch is not evidence about any tier. - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - let env = env_from(&[(PROFILE, "no-accel"), (FORCED, "1")]); - - match reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - refuse, - ) { - Boot::Run(session) => assert_eq!(session.profile_name(), "no-accel"), - _ => panic!("a forced child runs in this process"), - } - assert_eq!(store.read(), None); - assert_eq!(store.claim_count(), 0); -} - -#[test] -fn test_a_forced_child_that_crashes_does_not_advance_or_persist() { - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - let env = env_from(&[(PROFILE, "cpu-raster"), (FORCED, "1")]); - - // `accept`, not `refuse`: with a spawn edge that succeeds, a forced launch - // that was wrongly tracked would really hand off and leave a record behind. - // Against `refuse` the rollback would hide the difference. - let Boot::Run(session) = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - accept, - ) else { - panic!("a forced child runs in this process"); - }; - - // Even a genuine startup crash must leave a forced launch untracked — - // otherwise the manual override would rewrite the ladder's own record. - assert!(!session.on_web_process_terminated(Termination::Crashed, &|| {})); - assert_eq!(store.read(), None); - // The confirmation receipt is the other write a forced launch must not make: - // it is the persisted "last crash-free startup profile" fact, and a manual - // override is not evidence about any tier. - session.note_owned(); - session.note_confirmed(); - assert_eq!(store.read(), None); -} - -#[test] -fn test_a_refused_handoff_does_not_spend_the_rungs_retry() { - // A confirmed tier-2 record: this launch wants to re-run tier 2, which - // needs a child. The spawn is refused, so the durable state must be exactly - // what it was — a leftover `prepared` record would make the next launch - // burn this rung's one retry on a failure that was never about rendering. - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - let before = record(Phase::Confirmed, 0, 2); - store.write(&before).expect("write"); - let env = env_from(&[]); - - match reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - refuse, - ) { - Boot::Run(session) => { - // The handoff did not happen, so this process is still the baseline - // it was launched as and must not claim the tier it wanted. - assert_eq!(session.profile_name(), "full-gpu"); - } - _ => panic!("a refused handoff leaves the app running here"), - } - assert_eq!(store.read(), Some(before)); -} - -#[test] -fn test_a_refused_handoff_from_no_record_leaves_no_record() { - // Same rule with nothing on disk to roll back to: a `prepared` record - // written for a child that was never spawned must be cleared, not left as - // an unclaimed attempt for the next launch to inherit. - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - // A crash at tier 0 wants tier 1, so the parent prepares then spawns. - let env = env_from(&[]); - let Boot::Run(session) = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - refuse, - ) else { - panic!("a first launch runs tier 0 here"); - }; - - assert!(!session.on_web_process_terminated(Termination::Crashed, &|| {})); - assert_eq!(store.read(), None); -} - -#[test] -fn test_a_successful_handoff_leaves_the_prepared_record_for_the_child() { - // The mirror of the rollback: when the child really is launched, the - // prepared record must survive for it to claim. - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - let env = env_from(&[]); - let Boot::Run(session) = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - Vec::new(), - &env, - accept, - ) else { - panic!("a first launch runs tier 0 here"); - }; - - assert!(session.on_web_process_terminated(Termination::Crashed, &|| {})); - let prepared = store.read().expect("the child's record"); - assert_eq!(prepared.phase, Phase::Prepared); - assert_eq!(prepared.tier, 1); - assert_eq!(prepared.generation, 0); -} - -#[test] -fn test_reset_rendering_mode_works_even_with_an_owned_var_set() { - // The user who set WEBKIT_DISABLE_DMABUF_RENDERER by hand is exactly the - // user most likely to be clearing state an earlier run left behind. If the - // opt-out returned first, the escape hatch would be unreachable for them. - let dir = tempfile::tempdir().expect("tempdir"); - let store = Store::new(dir.path()); - store.write(&record(Phase::Confirmed, 0, 2)).expect("write"); - let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "1")]); - - let boot = reconcile( - Ok(dir.path().to_path_buf()), - IDENTIFIER, - VERSION, - vec![OsString::from(cli::RESET_RENDERING_MODE)], - &env, - refuse, - ); - - assert!(matches!(boot, Boot::Reset)); - assert_eq!(store.read(), None); -} diff --git a/desktop/src-tauri/src/render_recovery/tests/boot.rs b/desktop/src-tauri/src/render_recovery/tests/boot.rs new file mode 100644 index 0000000000..8aab473cef --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/tests/boot.rs @@ -0,0 +1,373 @@ +//! Boot-seam behaviour: the two flags, forced launches, and refused handoffs. +//! +//! Everything here is decided by `reconcile` before Tauri exists, which is the +//! only place a launch can still change its own environment or decline to +//! start at all. + +use super::*; + +// ── Flags and reset ───────────────────────────────────────────────────────── + +#[test] +fn test_the_renderer_flags_parse_independently_of_position() { + let args = [ + OsStr::new("buzz://channel/1"), + OsStr::new("--safe-rendering"), + ]; + let flags = cli::parse(args); + assert!(flags.safe_rendering); + assert!(!flags.reset_rendering_mode); + + let none = cli::parse([OsStr::new("--safe-renderingX")]); + assert!(!none.safe_rendering); +} + +#[test] +fn test_safe_rendering_forces_the_terminal_tier_without_persisting() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Confirmed, 0, 0)); + let env = env_from(&[]); + + // Native, so the terminal tier is x11-fallback and the handoff would need a + // real spawn — the assertion is that the record is left untouched. + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::SAFE_RENDERING)], + &env, + refuse, + ); + assert!(!matches!(boot, Boot::Reset)); + let after = store.read().expect("record"); + assert_eq!(after.phase, Phase::Confirmed); + assert_eq!(after.tier, 0); +} + +#[test] +fn test_reset_rendering_mode_clears_the_state_and_does_not_launch() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Confirmed, 0, 2)); + store.seed_claim(TOKEN, 0); + let env = env_from(&[]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::RESET_RENDERING_MODE)], + &env, + refuse, + ); + + assert!(matches!(boot, Boot::Reset)); + assert_eq!(store.read(), None); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_recovery_is_off_when_no_state_directory_exists() { + let env = env_from(&[]); + let boot = reconcile( + Err("no user data directory".to_string()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ); + assert!(matches!(boot, Boot::Off(Disabled::NoStateDir(_)))); +} + +#[test] +fn test_a_baseline_launch_runs_here_without_writing_a_record() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => assert_eq!(session.profile_name(), "full-gpu"), + _ => panic!("a first launch runs tier 0 in this process"), + } + assert_eq!(store.read(), None); +} + +// ── Forced launches, refused handoffs, and reaching reset ─────────────────── +// +// Three rules that only hold at the boot seam: a manual override must not teach +// the ladder anything, a handoff that never happened must not cost a retry, and +// the escape hatch must stay reachable for the user most likely to need it. + +#[test] +fn test_a_forced_child_runs_its_profile_without_writing_a_record() { + // A --safe-rendering parent hands off with BUZZ_RENDER_FORCED and no + // episode. The child must run the profile and persist nothing: a forced + // launch is not evidence about any tier. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[(PROFILE, "no-accel"), (FORCED, "1")]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => assert_eq!(session.profile_name(), "no-accel"), + _ => panic!("a forced child runs in this process"), + } + assert_eq!(store.read(), None); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_a_forced_child_that_crashes_does_not_advance_or_persist() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[(PROFILE, "cpu-raster"), (FORCED, "1")]); + + // `accept`, not `refuse`: with a spawn edge that succeeds, a forced launch + // that was wrongly tracked would really hand off and leave a record behind. + // Against `refuse` the rollback would hide the difference. + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("a forced child runs in this process"); + }; + + // Even a genuine startup crash must leave a forced launch untracked — + // otherwise the manual override would rewrite the ladder's own record. + assert!(matches!( + session.on_web_process_terminated(Termination::Crashed, &|| {}), + CrashResponse::Continue + )); + assert_eq!(store.read(), None); + // The confirmation receipt is the other write a forced launch must not make: + // it is the persisted "last crash-free startup profile" fact, and a manual + // override is not evidence about any tier. + session.note_owned(); + session.note_confirmed(); + assert_eq!(store.read(), None); +} + +#[test] +fn test_a_refused_handoff_does_not_spend_the_rungs_retry() { + // A confirmed tier-2 record: this launch wants to re-run tier 2, which + // needs a child. The spawn is refused, so the durable state must be exactly + // what it was — a leftover `prepared` record would make the next launch + // burn this rung's one retry on a failure that was never about rendering. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let before = record(Phase::Confirmed, 0, 2); + store.seed_record(&before); + let env = env_from(&[]); + + match reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) { + Boot::Run(session) => { + // The handoff did not happen, so this process is still the baseline + // it was launched as and must not claim the tier it wanted. + assert_eq!(session.profile_name(), "full-gpu"); + } + _ => panic!("a refused handoff leaves the app running here"), + } + assert_eq!(store.read(), Some(before)); +} + +#[test] +fn test_a_post_release_refusal_strands_the_parent_and_leaves_no_record() { + // Same rollback rule with nothing on disk, plus the lifecycle invariant: + // the live crash path releases the single-instance name before probing it, + // so a refusal from the launch edge arrives *after* the plugin is destroyed. + // This process can never take the name back, so it must not run on. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ) else { + panic!("a first launch runs tier 0 here"); + }; + + // The closure records the crossing, which is the only way to see it: a + // no-op closure cannot distinguish a refusal before the release from one + // after it. + let released = std::cell::Cell::new(false); + let response = session.on_web_process_terminated(Termination::Crashed, &|| released.set(true)); + + assert!( + released.get(), + "the live crash path releases the name before probing it" + ); + assert!( + matches!(response, CrashResponse::Stranded), + "a refusal after the release must exit, never return to normal operation" + ); + // The prepared record is still rolled back: no child exists to claim it. + assert_eq!(store.read(), None); +} + +#[test] +fn test_a_pre_release_refusal_leaves_the_app_running_with_its_name() { + // The mirror of the stranding rule: a refusal raised *before* the release + // is recoverable by construction, because nothing was given up. An app data + // path that cannot hold a directory makes the record write fail, which is + // the earliest refusal the live crash path can reach. + let dir = tempfile::tempdir().expect("tempdir"); + let blocker = dir.path().join("not-a-directory"); + std::fs::write(&blocker, b"").expect("blocker"); + let app_data_dir = blocker.join("state"); + let env = env_from(&[]); + + // Tier 0 needs no durable state, so this launch still comes up normally. + let Boot::Run(session) = reconcile( + Ok(app_data_dir), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("a first launch runs tier 0 here even with no usable state dir"); + }; + + let released = std::cell::Cell::new(false); + let response = session.on_web_process_terminated(Termination::Crashed, &|| released.set(true)); + + assert!( + !released.get(), + "a refusal that precedes the release must not have released the name" + ); + assert!( + matches!(response, CrashResponse::Continue), + "the app still owns its name, so it carries on" + ); +} + +#[test] +fn test_a_successful_handoff_leaves_the_prepared_record_for_the_child() { + // The mirror of the rollback: when the child really is launched, the + // prepared record must survive for it to claim. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("a first launch runs tier 0 here"); + }; + + assert!(matches!( + session.on_web_process_terminated(Termination::Crashed, &|| {}), + CrashResponse::HandedOff + )); + let prepared = store.read().expect("the child's record"); + assert_eq!(prepared.phase, Phase::Prepared); + assert_eq!(prepared.tier, 1); + assert_eq!(prepared.generation, 0); +} + +// ── The --safe-rendering conflict ─────────────────────────────────────────── + +#[test] +fn test_safe_rendering_against_a_user_set_owned_var_is_fatal_not_ignored() { + // Two incompatible answers to one question. Starting anyway would ignore a + // rescue flag from a user whose app does not come up; overriding would + // discard configuration they typed. Neither is guessed. + let dir = tempfile::tempdir().expect("tempdir"); + let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "0")]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::SAFE_RENDERING)], + &env, + refuse, + ); + + let Boot::Fatal(diagnostic) = boot else { + panic!("--safe-rendering under a user-set owned var must not start the app"); + }; + // The diagnostic has to name what is set and what to unset, or the user + // cannot act on it. `0` is the value a truthiness check would drop. + assert!(diagnostic.contains(cli::SAFE_RENDERING), "{diagnostic}"); + assert!( + diagnostic.contains("WEBKIT_DISABLE_DMABUF_RENDERER=0"), + "{diagnostic}" + ); +} + +#[test] +fn test_an_owned_var_without_the_flag_still_only_disables_recovery() { + // The conflict is specific to the flag. On its own a user assignment is a + // preference, not a contradiction, so the app starts under it. + let dir = tempfile::tempdir().expect("tempdir"); + let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "0")]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse, + ); + + assert!(matches!(boot, Boot::Off(Disabled::UserEnv(_)))); +} + +#[test] +fn test_reset_rendering_mode_works_even_with_an_owned_var_set() { + // The user who set WEBKIT_DISABLE_DMABUF_RENDERER by hand is exactly the + // user most likely to be clearing state an earlier run left behind. If the + // opt-out returned first, the escape hatch would be unreachable for them. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Confirmed, 0, 2)); + let env = env_from(&[("WEBKIT_DISABLE_DMABUF_RENDERER", "1")]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + vec![OsString::from(cli::RESET_RENDERING_MODE)], + &env, + refuse, + ); + + assert!(matches!(boot, Boot::Reset)); + assert_eq!(store.read(), None); +} diff --git a/desktop/src-tauri/src/render_recovery/tests/transitions.rs b/desktop/src-tauri/src/render_recovery/tests/transitions.rs new file mode 100644 index 0000000000..3f1b09c8b7 --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/tests/transitions.rs @@ -0,0 +1,337 @@ +//! Durable transitions under real contention, and the rules that make a late +//! writer harmless. +//! +//! The exclusive claim only serializes the creation of one claim file, so these +//! drive the transitions themselves from two threads through a barrier with the +//! state lock held across the window each race needs. + +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Barrier}; + +use super::*; + +// ── Serialized transitions under real contention ──────────────────────────── +// +// The exclusive claim only serializes the creation of one claim file. These +// tests drive the transitions themselves from two threads through a barrier, +// with the state lock held across the window the race needs, so the ordering is +// forced rather than hoped for. +// +// `flock` is per-open-file-description, so two `Store` values in one process +// contend exactly as two processes would — verified by the blocking assertions +// below, which only hold if the second writer really waits. + +/// Give a thread parked on the state lock time to get there. Only the +/// *contention* evidence depends on this interval; every invariant assertion is +/// ordering-based and holds however the sleep lands. +const CONTENTION_WINDOW: std::time::Duration = std::time::Duration::from_millis(150); + +#[test] +fn test_a_stale_generation_cannot_overwrite_a_record_prepared_while_it_waited() { + // The interleaving Thufir named: a delayed generation-0 child is mid-claim + // when another invocation durably prepares generation 1. Split into three + // unsynchronized operations the child would read gen 0 as current, pause, + // and then stamp its stale `started` over the newer record. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Prepared, 0, 1)); + + // Held for the whole window in which the stale child is trying to claim. + let tx = store.lock().expect("lock"); + + let path = dir.path().to_path_buf(); + let arrived = Arc::new(Barrier::new(2)); + let finished = Arc::new(AtomicBool::new(false)); + let stale = { + let (arrived, finished) = (Arc::clone(&arrived), Arc::clone(&finished)); + std::thread::spawn(move || { + let store = Store::new(&path); + arrived.wait(); + let claimed = claim( + &store, + &Episode { + token: TOKEN.to_string(), + generation: 0, + }, + ); + finished.store(true, Ordering::SeqCst); + claimed + }) + }; + + arrived.wait(); + std::thread::sleep(CONTENTION_WINDOW); + assert!( + !finished.load(Ordering::SeqCst), + "the claim must block on the state lock, not proceed beside the holder" + ); + + // The retry becomes durable while the stale child is parked. + let prepared = record(Phase::Prepared, 1, 1); + store.write(&tx, &prepared).expect("prepare generation 1"); + drop(tx); + + let claimed = stale.join().expect("stale child"); + // Only reachable if the child read the record *after* the write above, which + // is the serialization this test exists to prove. + assert!( + matches!(claimed, Claimed::Superseded(_)), + "a generation-0 child must be turned away once generation 1 is current" + ); + assert_eq!(store.read().as_ref(), Some(&prepared)); + assert_eq!(store.claim_count(), 0); +} + +#[test] +fn test_a_confirmation_racing_a_handoff_does_not_resurrect_its_episode() { + // `note_confirmed` runs from the startup-boundary thread, so it can be in + // flight when a crash handoff has already prepared the next token. Writing + // the old episode's `confirmed` over that token would make the ladder reuse + // a profile the crash just disproved. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Prepared, 0, 1)); + let env = env_from(&[ + ("WEBKIT_DMABUF_RENDERER_FORCE_SHM", "1"), + (PROFILE, "shm-transport"), + (EPISODE, TOKEN), + (GENERATION, "0"), + ]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + accept, + ) else { + panic!("the tagged child claims its episode"); + }; + session.note_owned(); + assert_eq!(store.read().expect("record").phase, Phase::Owned); + + let tx = store.lock().expect("lock"); + + let arrived = Arc::new(Barrier::new(2)); + let finished = Arc::new(AtomicBool::new(false)); + let confirming = { + let (session, arrived, finished) = ( + Arc::clone(&session), + Arc::clone(&arrived), + Arc::clone(&finished), + ); + std::thread::spawn(move || { + arrived.wait(); + session.note_confirmed(); + finished.store(true, Ordering::SeqCst); + }) + }; + + arrived.wait(); + std::thread::sleep(CONTENTION_WINDOW); + assert!( + !finished.load(Ordering::SeqCst), + "the confirmation must block on the state lock" + ); + + // What the crash handoff writes: a new token at the next rung. + let handed_off = Record::new( + Phase::Prepared, + "a-newer-token", + 0, + "cpu-raster", + 2, + VERSION, + ); + store + .write(&tx, &handed_off) + .expect("prepare the next rung"); + drop(tx); + + confirming.join().expect("confirming thread"); + assert_eq!( + store.read().as_ref(), + Some(&handed_off), + "the superseded episode's confirmation must be rejected, not written" + ); +} + +/// Spawn counter for the two-callback race below. A `static` because the launch +/// edge is a plain fn pointer with no captured state; only that one test reads +/// it, so no other test can perturb the count. +static SPAWNS: AtomicUsize = AtomicUsize::new(0); + +fn count_spawns( + _name: &str, + _binary: &std::path::Path, + _args: &[OsString], + _package: Package, + _tier: &'static Tier, + _tag: &Tag, +) -> Result { + Ok(SPAWNS.fetch_add(1, Ordering::SeqCst) as u32 + 1) +} + +#[test] +fn test_two_termination_callbacks_hand_off_exactly_once() { + // WebKit can report a second termination while the first handoff is in + // flight. Two handoffs would spawn two children for two episodes — two live + // Buzz processes racing one name, with the record naming only one of them. + SPAWNS.store(0, Ordering::SeqCst); + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let env = env_from(&[]); + let Boot::Run(session) = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + count_spawns, + ) else { + panic!("a first launch runs tier 0 here"); + }; + + let arrived = Arc::new(Barrier::new(2)); + let callbacks: Vec<_> = (0..2) + .map(|_| { + let (session, arrived) = (Arc::clone(&session), Arc::clone(&arrived)); + std::thread::spawn(move || { + arrived.wait(); + matches!( + session.on_web_process_terminated(Termination::Crashed, &|| {}), + CrashResponse::HandedOff + ) + }) + }) + .collect(); + + let handed_off = callbacks + .into_iter() + .map(|thread| thread.join().expect("callback")) + .filter(|handed| *handed) + .count(); + + assert_eq!(handed_off, 1, "exactly one callback may hand off"); + assert_eq!( + SPAWNS.load(Ordering::SeqCst), + 1, + "exactly one child may be spawned" + ); + // One survivor, and the durable record names the episode it was spawned for. + let prepared = store.read().expect("the survivor's record"); + assert_eq!(prepared.phase, Phase::Prepared); + assert_eq!(prepared.tier, 1); + assert_eq!(prepared.generation, 0); + assert_eq!(store.claim_count(), 0); +} + +/// App-data path for the rollback race below, so the refusing launch edge can +/// write a competing record from inside the window the parent left open. A +/// `static` for the same reason as `SPAWNS`: the launch edge captures nothing. +static RACING_STORE: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// The competing record: a different episode, prepared by "another launch" +/// during the spawn window, then refused so the parent reaches its rollback. +fn refuse_after_a_competing_prepare( + _name: &str, + _binary: &std::path::Path, + _args: &[OsString], + _package: Package, + _tier: &'static Tier, + _tag: &Tag, +) -> Result { + let path = RACING_STORE.lock().expect("path").clone().expect("path"); + Store::new(&path).seed_record(&Record::new( + Phase::Prepared, + "a-newer-token", + 0, + "cpu-raster", + 2, + VERSION, + )); + Err(Refusal::SpawnFailed("no child".to_string())) +} + +#[test] +fn test_a_rollback_does_not_erase_an_episode_prepared_after_it() { + // The state lock is dropped between the prepared write and the spawn, which + // is deliberate — the spawn must not run under it. So a refusal can come + // back after another launch has prepared its own episode, and an unchecked + // rollback would delete a record that launch's child is about to claim, + // turning a real handoff into an untracked launch. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Confirmed, 0, 2)); + *RACING_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); + let env = env_from(&[]); + + // Wants tier 2, so it prepares a record, spawns, and is refused. + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + refuse_after_a_competing_prepare, + ); + + assert!(matches!(boot, Boot::Run(_)), "a refusal keeps this process"); + let current = store.read().expect("the competing episode's record"); + assert_eq!( + current.token, "a-newer-token", + "the rollback must leave the newer episode alone" + ); + assert_eq!(current.phase, Phase::Prepared); +} + +// ── Receipt currency and forward-only phases ──────────────────────────────── + +#[test] +fn test_a_receipt_requires_a_current_episode_and_a_forward_phase() { + let current = record(Phase::Owned, 0, 1); + let episode = Episode { + token: TOKEN.to_string(), + generation: 0, + }; + + assert!(may_advance(Some(¤t), &episode, Phase::Confirmed)); + // Backwards and sideways are both rejected: a repeated callback has nothing + // to add, and an out-of-order one must not undo a later phase. + assert!(!may_advance(Some(¤t), &episode, Phase::Owned)); + assert!(!may_advance(Some(¤t), &episode, Phase::Started)); + // Another token's or generation's receipt is never current here. + assert!(!may_advance( + Some(¤t), + &Episode { + token: "another-token".to_string(), + generation: 0 + }, + Phase::Confirmed + )); + assert!(!may_advance( + Some(¤t), + &Episode { + token: TOKEN.to_string(), + generation: 1 + }, + Phase::Confirmed + )); + // A cleared record is not something a receipt may recreate: the record a + // receipt belongs to is written by the parent before the child exists. + assert!(!may_advance(None, &episode, Phase::Confirmed)); +} + +#[test] +fn test_the_exhausted_phase_is_terminal_against_a_late_receipt() { + // `Exhausted` outranks every other phase, so the attempt that exhausted the + // ladder cannot reopen it with a receipt that arrives afterwards. + let exhausted = record(Phase::Exhausted, 0, 3); + let episode = Episode { + token: TOKEN.to_string(), + generation: 0, + }; + for phase in [Phase::Started, Phase::Owned, Phase::Confirmed] { + assert!(!may_advance(Some(&exhausted), &episode, phase)); + } +} diff --git a/desktop/src-tauri/src/render_recovery/wiring.rs b/desktop/src-tauri/src/render_recovery/wiring.rs index 068cc3ac67..b1e1356f44 100644 --- a/desktop/src-tauri/src/render_recovery/wiring.rs +++ b/desktop/src-tauri/src/render_recovery/wiring.rs @@ -20,7 +20,7 @@ use tauri::{Manager, Runtime}; use webkit2gtk::{WebProcessTerminationReason, WebViewExt}; use super::episode::Termination; -use super::session::Session; +use super::session::{CrashResponse, Session}; use super::LOG; /// Everything the ladder needs from the live app. @@ -42,15 +42,18 @@ pub(crate) fn plugin(session: Arc) -> TauriPlugin { platform .inner() .connect_web_process_terminated(move |_, reason| { - let handed_off = session - .on_web_process_terminated(classify(reason), &|| { - tauri_plugin_single_instance::destroy(&app) - }); - // A child owns the app now. Exit through Tauri so the - // ordinary shutdown path still runs; two Buzz processes - // must never be live at once. - if handed_off { - app.exit(0); + let response = session.on_web_process_terminated(classify(reason), &|| { + tauri_plugin_single_instance::destroy(&app) + }); + // Exit through Tauri in both terminal cases so the + // ordinary shutdown path still runs. `HandedOff`: a + // child owns the app now, and two Buzz processes must + // never be live at once. `Stranded`: the single-instance + // name was released and cannot be taken back, so this + // process is no longer able to be the app. + match response { + CrashResponse::HandedOff | CrashResponse::Stranded => app.exit(0), + CrashResponse::Continue => {} } }); }) { From 8dbea79cb11e0905f4c211ef5c7942c553b1f53d Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 12:21:29 -0400 Subject: [PATCH 3/4] fix(desktop): make renderer state transitions compare-and-transition Boot decided from a record read outside the transaction lock and then mutated under it without re-checking that record was still current, so a decision could outlive the state it rested on: a concurrent --reset-rendering-mode could be undone by a stale retry recreating a prepared episode, and a delayed version-mismatch discard could delete a newly prepared current-version episode. Store::transition is now the only way anything durable changes. It re-reads the record under the caller's lock and writes only if the record still matches the state the caller decided from; a refusal makes boot re-decide from what is actually there rather than proceed. Because the expected source names a phase, the receipt chain is exact for free -- owned requires started, confirmed requires owned -- replacing a rank-based ordering that accepted skips. The prepare/release/spawn sequence moves to session::handoff, which keeps both files well clear of the size gate. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src-tauri/src/render_recovery/episode.rs | 44 +- desktop/src-tauri/src/render_recovery/mod.rs | 2 +- .../src-tauri/src/render_recovery/session.rs | 463 ++++++++++-------- .../src/render_recovery/session/handoff.rs | 202 ++++++++ .../src-tauri/src/render_recovery/state.rs | 213 ++++++-- .../src-tauri/src/render_recovery/tests.rs | 109 ++++- .../src/render_recovery/tests/transitions.rs | 257 ++++++++-- 7 files changed, 964 insertions(+), 326 deletions(-) create mode 100644 desktop/src-tauri/src/render_recovery/session/handoff.rs diff --git a/desktop/src-tauri/src/render_recovery/episode.rs b/desktop/src-tauri/src/render_recovery/episode.rs index 7141dd0287..235314aef7 100644 --- a/desktop/src-tauri/src/render_recovery/episode.rs +++ b/desktop/src-tauri/src/render_recovery/episode.rs @@ -6,7 +6,7 @@ //! attempt cap ordinary unit tests. use super::profiles::Package; -use super::state::{Claim, Phase, Record, Store, RETRY_GENERATION}; +use super::state::{Claim, Expected, Next, Phase, Record, Store, RETRY_GENERATION}; /// Identity of one attempt. The token names the rung's attempt; the generation /// distinguishes the original from its sole retry. @@ -98,10 +98,11 @@ pub(crate) enum Claimed { /// generation's receipt. /// /// The read, the claim, and the `started` write all happen inside one -/// transaction. Split across three unsynchronized steps they were not enough: a -/// child could read its own generation as current, be descheduled while another -/// invocation durably prepared the next generation, then resume and stamp its -/// stale `started` over the newer record. +/// transaction, and the write names the exact record the read validated. Split +/// across three unsynchronized steps they were not enough: a child could read +/// its own generation as current, be descheduled while another invocation +/// durably prepared the next generation, then resume and stamp its stale +/// `started` over the newer record. pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { let tx = match store.lock() { Ok(tx) => tx, @@ -135,7 +136,7 @@ pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { let mut started = record.clone(); started.phase = Phase::Started; started.pid = Some(std::process::id()); - if let Err(error) = store.write(&tx, &started) { + if let Err(error) = store.transition(&tx, Some(Expected::of(&record)), Next::Record(&started)) { // The claim is taken and cannot be given back, so this process is the // owner whether or not the receipt landed. Run on untracked: the record // still says `prepared`, and the next launch retries or advances it. @@ -149,24 +150,21 @@ pub(crate) fn claim(store: &Store, episode: &Episode) -> Claimed { } } -/// Whether `episode` may still write `next` over `current`. +/// The phase a receipt for `next` requires the record to already be at. /// -/// Two independent conditions, both necessary. The identity check rejects a -/// receipt from an episode that is no longer current — a `confirmed` write -/// racing the crash handoff that already prepared the next token would -/// otherwise resurrect the superseded attempt. The phase check rejects a -/// receipt that does not move the episode forward, so a duplicated or -/// out-of-order callback cannot walk the record backwards. -pub(crate) fn may_advance(current: Option<&Record>, episode: &Episode, next: Phase) -> bool { - let Some(current) = current else { - // Nothing on disk to advance. The record a receipt belongs to is - // written by the parent before the child exists, so its absence means - // this episode was cleared — not that a receipt should create it. - return false; - }; - current.token == episode.token - && current.generation == episode.generation - && current.phase.precedes(next) +/// The chain is exact rather than ordered: a receipt names the one state it +/// follows, so a skip is a mismatch. `Prepared` has no predecessor — it is +/// written by a parent about an attempt that does not exist yet — and +/// `Exhausted` is validated as a terminal write against the whole attempt +/// rather than one position, because the process recording it may or may not +/// have landed its own `confirmed` receipt first. +pub(crate) fn predecessor_of(next: Phase) -> Option { + match next { + Phase::Started => Some(Phase::Prepared), + Phase::Owned => Some(Phase::Started), + Phase::Confirmed => Some(Phase::Owned), + Phase::Prepared | Phase::Exhausted => None, + } } /// The record a parent must make durable before spawning a child for `tier`. diff --git a/desktop/src-tauri/src/render_recovery/mod.rs b/desktop/src-tauri/src/render_recovery/mod.rs index 058bd1805b..701b7ae0b6 100644 --- a/desktop/src-tauri/src/render_recovery/mod.rs +++ b/desktop/src-tauri/src/render_recovery/mod.rs @@ -22,7 +22,7 @@ //! exclusive claims), `episode` (attempt identity and the pure ladder rules), //! `dbus` (name observation), `classify` (record + observation → decision), //! `launcher` (the explicit relaunch), `cli` (flags), `session` (the live -//! process), `wiring` (the Tauri plugin that carries it into the app). +//! process, with `session::handoff` for prepare/release/spawn), `wiring` (the Tauri plugin that carries it into the app). pub(crate) mod classify; pub(crate) mod cli; diff --git a/desktop/src-tauri/src/render_recovery/session.rs b/desktop/src-tauri/src/render_recovery/session.rs index 97fc63a4ba..d5a87cf985 100644 --- a/desktop/src-tauri/src/render_recovery/session.rs +++ b/desktop/src-tauri/src/render_recovery/session.rs @@ -6,6 +6,8 @@ //! that. What it returns is either a `Session`, which the rest of the process //! uses to react to a crash, or an instruction to exit. +mod handoff; + use std::ffi::OsString; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -16,7 +18,7 @@ use super::dbus::{self, Observation}; use super::episode::{self, Advance, Claimed, Episode, Termination}; use super::launcher::{self, Handoff, Refusal, Tag}; use super::profiles::{self, Env, Package, Tier}; -use super::state::{Phase, Record, Store, Transaction}; +use super::state::{Expected, Next, Phase, Record, Refused, Store}; use super::LOG; /// Why the ladder is not running this launch. In every case the app starts @@ -122,6 +124,64 @@ impl Release<'_> { } } +/// A durable write was refused because the record no longer matches the state +/// its caller decided from — another process reset it, or a newer episode +/// superseded it. +/// +/// Nothing was released and nothing was spawned when this comes back: the +/// prepared write is the first thing a handoff does, before the release and the +/// spawn. Boot answers it by deciding again from the new state; the live crash +/// path answers it by standing down, since a process whose episode has been +/// superseded is not the one that should be laddering. +/// +/// Carries the description rather than the record it found, because describing +/// what changed is all any caller does with it. +struct Stale(String); + +impl std::fmt::Display for Stale { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +/// Which state a durable write is allowed to replace. +/// +/// The two callers differ in kind, not merely in value. Boot reconciliation acts +/// on a record it read *earlier* and must prove that exact record is still +/// current. The live crash path acts on a crash that just happened, and its +/// source is its own episode wherever the chain has reached — it holds no +/// earlier snapshot to be stale about. Naming the distinction is what stops a +/// classified snapshot from silently standing in for a fresh read. +#[derive(Clone, Copy)] +enum Source<'a> { + /// Exactly this record, or the write is refused. `None` expects no record. + Classified(Option<&'a Record>), + /// This process's own episode, at the phase a `prepared` write must follow. + OwnEpisode, +} + +/// What one reconciliation round settled on, once any durable write it needed +/// has landed. +/// +/// Carries no `Session`: the loop holds that by `&mut`, so a round that has to +/// be retaken does not have to move an owned value out and back. +enum Settled { + /// Run in this process, with whatever tier and freeze the round selected. + RunHere, + /// A child carrying the selected tier is Buzz now; this process exits. + HandedOff, + /// Exit before Tauri with this diagnostic. + Fatal(String), +} + +/// How many times boot reconciliation re-decides before standing down. +/// +/// This is not a spin: a refusal only happens when another process durably +/// changed the record, so every round is somebody else's progress. Standing +/// down runs the launched profile untracked — the one outcome that is always +/// safe, because it writes nothing and still starts the app. +const RECONCILE_ATTEMPTS: usize = 3; + /// What a live app must do about a web-process termination. pub(crate) enum CrashResponse { /// A child carrying the next tier is Buzz now. Exit. @@ -254,8 +314,8 @@ impl Session { /// `--reset-rendering-mode`: delete the persisted tier and episode state, /// say what was cleared, and exit without launching. fn reset(self) -> Boot { - let cleared = match self.store.lock() { - Ok(tx) => self.store.clear(&tx), + let tx = match self.store.lock() { + Ok(tx) => tx, Err(error) => { // Another process is mid-transition. Clearing without the lock // could delete a record a live child is about to claim, so this @@ -263,6 +323,17 @@ impl Session { return Boot::Fatal(format!("could not clear renderer state: {error}")); } }; + // Reading the source inside the transaction that clears it is what makes + // reset an ordering boundary: it cannot be stale, and any decision taken + // from the state it deletes is refused when that decision tries to write. + let cleared = self.store.exists(&tx); + let source = self.store.read(); + if let Err(error) = + self.store + .transition(&tx, source.as_ref().map(Expected::of), Next::Cleared) + { + return Boot::Fatal(format!("could not clear renderer state: {error}")); + } println!( "{}", match cleared { @@ -273,11 +344,19 @@ impl Session { Boot::Reset } - /// Untagged launch: classify the record, then either run the selected tier - /// here or hand off to a child that can. + /// Untagged launch: decide from the durable record, then either run the + /// selected tier here or hand off to a child that can. + /// + /// Deciding and acting are two steps with a D-Bus round trip between them, + /// so the record can move in between — another launch's reset, or a newer + /// episode. Every write therefore names the record it was decided from and + /// is refused if that record is gone, and a refusal re-decides from what is + /// there now rather than proceeding on a decision about a state that no + /// longer exists. fn select_tier(mut self, safe_rendering: bool) -> Boot { // `--safe-rendering` is a manual override for this launch only: the - // terminal tier, no episode, nothing persisted. + // terminal tier, no episode, nothing persisted. It reads no record, so + // there is nothing for it to be stale about. if safe_rendering { self.tier = self.package.terminal_tier(); self.frozen = true; @@ -286,11 +365,54 @@ impl Session { cli::SAFE_RENDERING, self.profile_name() ); - return self.run_selected_tier(); + return match self.hand_off_or_run(None, self.tier, None) { + Ok(Settled::RunHere) => Boot::Run(Arc::new(self)), + Ok(Settled::HandedOff) => Boot::HandedOff, + Ok(Settled::Fatal(diagnostic)) => Boot::Fatal(diagnostic), + // A forced launch prepares no record, so it has nothing to be + // stale about; `hand_off_or_run` cannot refuse it this way. + Err(stale) => Boot::Fatal(format!("renderer state changed unexpectedly: {stale}")), + }; + } + + for attempt in 1..=RECONCILE_ATTEMPTS { + match self.reconcile_once() { + Ok(Settled::RunHere) => return Boot::Run(Arc::new(self)), + Ok(Settled::HandedOff) => return Boot::HandedOff, + Ok(Settled::Fatal(diagnostic)) => return Boot::Fatal(diagnostic), + Err(stale) => { + eprintln!( + "{LOG}: renderer state changed under this launch ({stale}); \ + re-reading it (attempt {attempt} of {RECONCILE_ATTEMPTS})" + ); + // Nothing was released or spawned — a refused write is the + // first thing a handoff does — so the next round starts from + // the tier this process was actually launched as. + self.tier = 0; + self.frozen = false; + } + } } + // Another process has out-raced this one repeatedly. Running the + // launched profile untracked is the safe stand-down: it writes nothing, + // so it cannot corrupt whatever that process is doing, and the app still + // starts. + eprintln!( + "{LOG}: renderer state kept changing; running the launched profile \ + without tracking it" + ); + self.tier = 0; + self.frozen = true; + Boot::Run(Arc::new(self)) + } + /// One decide-then-act round. `Err` means the record moved before the write + /// this decision called for could land, so the decision must be retaken. + fn reconcile_once(&mut self) -> Result { let Some(record) = self.store.read() else { - return self.run_selected_tier(); + // No record to be superseded, and a handoff from here prepares only a + // freshly minted episode, which expects exactly this absence. + return self.run_here_or_hand_off(None); }; let decision = classify::decide( &record, @@ -303,25 +425,27 @@ impl Session { "{LOG}: found {:?} record (token={} gen={} tier={} profile={}) → {decision:?}", record.phase, record.token, record.generation, record.tier, record.profile ); + let source = Some(&record); match decision { // Another instance owns the app. Run on as an ordinary duplicate // and let the single-instance plugin forward argv and exit us. - Decision::Defer(_) => Boot::Run(Arc::new(self)), + // Writes nothing, so nothing can be stale. + Decision::Defer(_) => Ok(Settled::RunHere), Decision::DiscardAndBaseline { .. } => { - if let Ok(tx) = self.store.lock() { - self.store.clear(&tx); - } - self.run_selected_tier() + self.clear_discarded(&record)?; + // The record this decision was about is gone and the baseline is + // what is left, so a fresh episode expects no record at all. + self.run_here_or_hand_off(None) } // The persisted "last crash-free startup profile" fact. Decision::ReuseProfile { tier } => { self.tier = tier; - self.run_selected_tier() + self.run_here_or_hand_off(source) } // No profile survived. Stop rather than relaunch: this process is // the baseline environment, and the user's way to the safest tier - // is the flag, not another fork. + // is the flag, not another fork. Writes nothing. Decision::StopExhausted { tier } => { self.frozen = true; eprintln!( @@ -330,45 +454,70 @@ impl Session { self.package.label(), cli::SAFE_RENDERING ); - Boot::Run(Arc::new(self)) + Ok(Settled::RunHere) } // An attempt that never produced a receipt: re-run the same tier // under the next generation. Decision::RetrySameTier { tier, .. } => { self.tier = tier; - self.hand_off_or_run(Some(episode::retry_of(&record)), tier) + self.hand_off_or_run(Some(episode::retry_of(&record)), tier, source) + } + Decision::AdvanceOrStop { tier, .. } => self.step_down(tier, &record), + } + } + + /// Discard a record this launch cannot use — a different app version's. + /// + /// Named source, so a delayed version-mismatch decision cannot delete an + /// episode that a newer launch prepared while this one was classifying. + fn clear_discarded(&self, source: &Record) -> Result<(), Stale> { + let tx = self + .store + .lock() + .map_err(|error| Stale(format!("the state lock was unavailable: {error}")))?; + match self + .store + .transition(&tx, Some(Expected::of(source)), Next::Cleared) + { + Ok(()) => Ok(()), + Err(error @ Refused::Stale(_)) => Err(Stale(error.to_string())), + Err(error) => { + eprintln!("{LOG}: could not discard the stale record — {error}"); + Ok(()) } - Decision::AdvanceOrStop { tier, .. } => self.step_down(tier), } } /// Continue here when this process already carries the selected tier's /// environment, otherwise hand off to a child that carries it exactly. - fn run_selected_tier(self) -> Boot { + fn run_here_or_hand_off(&mut self, source: Option<&Record>) -> Result { if self.tier == 0 { // Tier 0 sets nothing, and the opt-out check above proved no owned // variable is present — this process already *is* tier 0. - return Boot::Run(Arc::new(self)); + return Ok(Settled::RunHere); } // A frozen launch is a manual override, so it runs the tier without // owning an episode: no record, no receipt, no advance. let episode = (!self.frozen).then(episode::fresh); let tier = self.tier; - self.hand_off_or_run(episode, tier) + self.hand_off_or_run(episode, tier, source) } /// Move one tier down for a failed attempt, or stop at the terminal tier. - fn step_down(mut self, failed: usize) -> Boot { + fn step_down(&mut self, failed: usize, source: &Record) -> Result { match episode::advance(self.package, failed) { Advance::Tier(next) => { self.tier = next; - self.hand_off_or_run(Some(episode::fresh()), next) + self.hand_off_or_run(Some(episode::fresh()), next, Some(source)) } Advance::Exhausted => { self.tier = failed; self.frozen = true; - self.note_exhausted(); - Boot::Run(Arc::new(self)) + // Named source: an `exhausted` write is terminal, so writing it + // over a record that has since been reset or superseded would + // strand the ladder on a decision about state that is gone. + self.note_exhausted(Source::Classified(Some(source)))?; + Ok(Settled::RunHere) } } } @@ -450,14 +599,14 @@ impl Session { /// Write a phase receipt for the episode this process owns. /// - /// Two conditions gate the write, both under the transaction lock. The - /// identity check (`episode::may_advance`) rejects a receipt for an episode - /// that is no longer current: `note_confirmed` runs from a timer thread and - /// can be racing a crash handoff that has already prepared the next token, - /// and without the check it would overwrite that token with the superseded - /// episode's `confirmed`. The phase check rejects a receipt that does not - /// move the record forward, so a repeated or out-of-order callback cannot - /// walk it backwards. + /// The transition names both the episode and the phase the receipt follows, + /// which is two rejections in one. Identity: `note_confirmed` runs from a + /// timer thread and can be racing a crash handoff that already prepared the + /// next token, and without the check it would overwrite that token with the + /// superseded episode's `confirmed`. Order: the chain is exact, so a receipt + /// that skips its predecessor — a `confirmed` after a failed `owned` write, + /// or an out-of-order callback — is refused rather than accepted as + /// "forward". /// /// The receipt is bound to the bus id as well as the unique name: a unique /// name like `:1.4` only identifies a connection on the bus that issued it, @@ -467,14 +616,15 @@ impl Session { let Some(episode) = &self.episode else { return; }; + let Some(follows) = episode::predecessor_of(phase) else { + // Only `prepared` and `exhausted` have no predecessor, and neither is + // written through here. + return; + }; let Ok(tx) = self.store.lock() else { eprintln!("{LOG}: skipping the {phase:?} receipt — the state lock was unavailable"); return; }; - if !episode::may_advance(self.store.read().as_ref(), episode, phase) { - eprintln!("{LOG}: skipping the {phase:?} receipt — this episode is no longer current"); - return; - } let mut record = self.record(phase, episode); record.pid = Some(std::process::id()); @@ -488,7 +638,13 @@ impl Session { // identity as uncorrelatable rather than as ours. _ => eprintln!("{LOG}: writing {phase:?} without an owner identity"), } - self.write(&tx, &record); + let expected = Expected::after(&episode.token, episode.generation, follows); + if let Err(error) = self + .store + .transition(&tx, Some(expected), Next::Record(&record)) + { + eprintln!("{LOG}: skipping the {phase:?} receipt — {error}"); + } } /// React to a web-process termination. @@ -524,12 +680,31 @@ impl Session { match episode::advance(self.package, self.tier) { Advance::Exhausted => { - self.note_exhausted(); + // A stale terminal write means this process's episode was + // superseded while it ran. Nothing to do: the process that + // superseded it owns the ladder now. + if let Err(stale) = self.note_exhausted(Source::OwnEpisode) { + eprintln!("{LOG}: not recording the exhausted ladder — {stale}"); + } CrashResponse::Continue } Advance::Tier(next) => { let release = Release::SingleInstanceName(destroy_single_instance); - match self.hand_off(episode::fresh(), next, &release) { + let handoff = + match self.hand_off(episode::fresh(), next, Source::OwnEpisode, &release) { + Ok(handoff) => handoff, + // Refused before anything was released or spawned, so this + // process still owns its name. Another process superseded + // this episode, which means it is already driving the + // ladder — advancing here too would fork it. + Err(stale) => { + eprintln!( + "{LOG}: not advancing the ladder — {stale}; another launch owns it now" + ); + return CrashResponse::Continue; + } + }; + match handoff { Handoff::Launched => CrashResponse::HandedOff, // Nothing was released, so this process is still the app. Handoff::RefusedBeforeRelease(refusal) => { @@ -552,187 +727,58 @@ impl Session { } } - /// Hand off to a child at `tier`, or run on here if the handoff is refused. - /// Only reached before Tauri exists, so nothing is held and every refusal - /// leaves this process exactly as it was launched. - fn hand_off_or_run(mut self, episode: Option, tier: usize) -> Boot { - let handed = match episode { - Some(episode) => self.hand_off(episode, tier, &Release::NothingHeld), - None => self.force(tier), - }; - match handed { - Handoff::Launched => Boot::HandedOff, - Handoff::RefusedBeforeRelease(refusal) => { - eprintln!("{LOG}: staying at the launched profile — {refusal}"); - // The handoff did not happen, so this process is still the - // baseline it was launched as and must not claim otherwise. - self.tier = 0; - self.frozen = true; - Boot::Run(Arc::new(self)) - } - // Unreachable by construction: `Release::NothingHeld` releases - // nothing, so there is no post-release side to land on. Handled as a - // refusal rather than a panic — if the invariant ever breaks, the - // safe reading of "we may have released something" is to not run. - Handoff::RefusedAfterRelease(refusal) => Boot::Fatal(format!( - "renderer handoff refused after an unexpected release: {refusal}" - )), - } - } - - /// Prepare a record for `tier`, release the single-instance name, and spawn - /// a child carrying the tier's exact environment. - /// - /// The order is the whole mechanism. The record must be durable before the - /// child exists, because the child claims it as its first action and would - /// otherwise find nothing. The name must be released before the probe, - /// because a child that fails to take the name forwards its argv and exits - /// 0 — indistinguishable from a successful handoff. - fn hand_off(&self, episode: Episode, tier: usize, release: &Release<'_>) -> Handoff { - let rung = match self.rung(tier) { - Ok(rung) => rung, - Err(refusal) => return Handoff::RefusedBeforeRelease(refusal), - }; - let tx = match self.store.lock() { - Ok(tx) => tx, - Err(error) => { - return Handoff::RefusedBeforeRelease(Refusal::StateNotDurable(error)); - } - }; - let previous = self.store.read(); - let record = episode::prepare(&episode, self.package, tier, &self.version); - if let Err(error) = self.store.write(&tx, &record) { - return Handoff::RefusedBeforeRelease(Refusal::StateNotDurable(error)); - } - if episode.generation == 0 { - // Only now that the new record is durable. Pruning first would, on a - // failed record write, leave the old record current with its claim - // evidence already gone — and an old child that had passed its - // record read could then re-create its claim and write a receipt - // over the record. - self.store.prune_claims(&tx, &episode.token); - } - // Released before the spawn: the record is durable and every check that - // does not need the name released has already run. - drop(tx); - - let tag = Tag { - episode: Some(episode), - profile: rung.name.to_string(), - }; - let handoff = self.spawn_child(rung, &tag, release); - if handoff.refused() { - // No child exists, so the prepared record describes an attempt that - // will never be claimed. Left in place it would spend the rung's one - // retry on a failure that was never about rendering. - self.roll_back(&record, previous); - } - handoff - } - - /// Restore the record a refused handoff had already overwritten. - /// - /// Identity-checked, because the lock is dropped between the prepared write - /// and the spawn: by the time a refusal comes back, a concurrent launch may - /// have prepared its own episode, and a blind restore would erase a record - /// that a live child is about to claim — turning that child's handoff into - /// an untracked launch. Only the record this call itself wrote is rolled - /// back. - fn roll_back(&self, prepared: &Record, previous: Option) { - let Ok(tx) = self.store.lock() else { - eprintln!("{LOG}: could not roll back the prepared record — the state lock was busy"); - return; - }; - match self.store.read() { - Some(current) - if current.token == prepared.token && current.generation == prepared.generation => { - } - _ => { - eprintln!("{LOG}: not rolling back — another episode is current now"); - return; - } - } - match previous { - Some(record) => self.write(&tx, &record), - None => { - self.store.clear(&tx); - } - } - } - - /// Hand off to a child running `tier` with no episode: nothing is prepared, - /// nothing is claimed, and nothing is persisted. This is what a manual - /// override is — a launch the ladder runs but does not learn from. - fn force(&self, tier: usize) -> Handoff { - match self.rung(tier) { - Ok(rung) => { - let tag = Tag { - episode: None, - profile: rung.name.to_string(), - }; - self.spawn_child(rung, &tag, &Release::NothingHeld) - } - Err(refusal) => Handoff::RefusedBeforeRelease(refusal), - } + fn rung(&self, tier: usize) -> Result<&'static Tier, Refusal> { + self.package + .tier(tier) + .ok_or_else(|| Refusal::NoTier(format!("{} has no tier {tier}", self.package.label()))) } - /// The release boundary itself: every preflight that can fail runs *before* - /// `release.run()`, and everything after it is reported as a post-release - /// outcome. + /// The state a `prepared` write may replace. /// - /// Resolving the binary belongs on this side of the line. Held as an - /// unresolved `Result` and passed onward it would surface only inside the - /// launcher — after the name was already gone — turning a plainly - /// recoverable "cannot find my own executable" into a stranded app. - fn spawn_child(&self, rung: &'static Tier, tag: &Tag, release: &Release<'_>) -> Handoff { - let binary = match tauri::process::current_binary(&tauri::Env::default()) { - Ok(binary) => binary, - Err(error) => { - return Handoff::RefusedBeforeRelease(Refusal::NoBinary(error.to_string())); - } - }; - - let released = release.run(); - match (self.launch)( - &self.dbus_name, - &binary, - &self.args, - self.package, - rung, - tag, - ) { - Ok(pid) => { - eprintln!("{LOG}: handed off to {} as pid {pid}", rung.name); - Handoff::Launched - } - Err(refusal) => match released { - true => Handoff::RefusedAfterRelease(refusal), - false => Handoff::RefusedBeforeRelease(refusal), + /// A classified source must still be exactly what was decided from. A live + /// crash path holds no earlier snapshot: it names its own episode, or — when + /// this process owns none, which is every tier-0 launch — whatever the + /// transaction just read, since there is no earlier decision to be stale + /// about. + fn expected<'a>( + &'a self, + source: Source<'a>, + current: Option<&'a Record>, + ) -> Option> { + match source { + Source::Classified(source) => source.map(Expected::of), + Source::OwnEpisode => match &self.episode { + Some(episode) => Some(Expected::attempt(&episode.token, episode.generation)), + None => current.map(Expected::of), }, } } - fn rung(&self, tier: usize) -> Result<&'static Tier, Refusal> { - self.package - .tier(tier) - .ok_or_else(|| Refusal::NoTier(format!("{} has no tier {tier}", self.package.label()))) - } - /// Record that no profile survived, so later launches stop here instead of /// walking the whole ladder again on every start. /// - /// `Exhausted` outranks every other phase, so this write passes the forward - /// check from anywhere in the chain — which is what makes the terminal state - /// terminal even against a late receipt from the attempt that got here. - fn note_exhausted(&self) { + /// `Exhausted` is terminal, so it is the one write that most needs a named + /// source: stamping it over a record that has since been reset or superseded + /// would stop the ladder on the strength of a decision about state that no + /// longer exists. + fn note_exhausted(&self, source: Source<'_>) -> Result<(), Stale> { let episode = self.episode.clone().unwrap_or_else(episode::fresh); let Ok(tx) = self.store.lock() else { eprintln!("{LOG}: could not record the exhausted ladder — the state lock was busy"); - return; + return Ok(()); }; + let current = self.store.read(); let mut record = self.record(Phase::Exhausted, &episode); record.pid = Some(std::process::id()); - self.write(&tx, &record); + match self.store.transition( + &tx, + self.expected(source, current.as_ref()), + Next::Record(&record), + ) { + Ok(()) => {} + Err(error @ Refused::Stale(_)) => return Err(Stale(error.to_string())), + Err(error) => eprintln!("{LOG}: could not record the exhausted ladder — {error}"), + } eprintln!( "{LOG}: ladder exhausted on {} at tier {} ({}); \ relaunch with {} to force the safest profile", @@ -741,6 +787,7 @@ impl Session { self.profile_name(), cli::SAFE_RENDERING ); + Ok(()) } fn record(&self, phase: Phase, episode: &Episode) -> Record { @@ -753,12 +800,6 @@ impl Session { &self.version, ) } - - fn write(&self, tx: &Transaction, record: &Record) { - if let Err(error) = self.store.write(tx, record) { - eprintln!("{LOG}: failed to persist {:?}: {error}", record.phase); - } - } } fn pid_alive(pid: u32) -> bool { diff --git a/desktop/src-tauri/src/render_recovery/session/handoff.rs b/desktop/src-tauri/src/render_recovery/session/handoff.rs new file mode 100644 index 0000000000..26f9087a8d --- /dev/null +++ b/desktop/src-tauri/src/render_recovery/session/handoff.rs @@ -0,0 +1,202 @@ +//! The handoff itself: preparing a record for a child, crossing the +//! single-instance release boundary, and undoing a preparation whose child was +//! never launched. +//! +//! Split out of `session` because it is one sequence with one invariant — the +//! record is durable before the child exists, and nothing that can fail runs +//! after the name is released — while `session` is about deciding *whether* to +//! run it. + +use super::*; + +impl Session { + /// Hand off to a child at `tier`, or run on here if the handoff is refused. + /// Only reached before Tauri exists, so nothing is held and every refusal + /// leaves this process exactly as it was launched. + pub(super) fn hand_off_or_run( + &mut self, + episode: Option, + tier: usize, + source: Option<&Record>, + ) -> Result { + let handed = match episode { + Some(episode) => self.hand_off( + episode, + tier, + Source::Classified(source), + &Release::NothingHeld, + )?, + // A forced launch prepares nothing, so it has no source to be stale + // about. + None => self.force(tier), + }; + match handed { + Handoff::Launched => Ok(Settled::HandedOff), + Handoff::RefusedBeforeRelease(refusal) => { + eprintln!("{LOG}: staying at the launched profile — {refusal}"); + // The handoff did not happen, so this process is still the + // baseline it was launched as and must not claim otherwise. + self.tier = 0; + self.frozen = true; + Ok(Settled::RunHere) + } + // Unreachable by construction: `Release::NothingHeld` releases + // nothing, so there is no post-release side to land on. Handled as a + // refusal rather than a panic — if the invariant ever breaks, the + // safe reading of "we may have released something" is to not run. + Handoff::RefusedAfterRelease(refusal) => Ok(Settled::Fatal(format!( + "renderer handoff refused after an unexpected release: {refusal}" + ))), + } + } + + /// Prepare a record for `tier`, release the single-instance name, and spawn + /// a child carrying the tier's exact environment. + /// + /// The order is the whole mechanism. The record must be durable before the + /// child exists, because the child claims it as its first action and would + /// otherwise find nothing. The name must be released before the probe, + /// because a child that fails to take the name forwards its argv and exits + /// 0 — indistinguishable from a successful handoff. + /// + /// The prepared write names `source`, so a handoff decided from a record + /// that has since been reset or superseded is refused *before* anything is + /// released or spawned — `Err(Stale)` therefore always means this process is + /// untouched. + pub(super) fn hand_off( + &self, + episode: Episode, + tier: usize, + source: Source<'_>, + release: &Release<'_>, + ) -> Result { + let rung = match self.rung(tier) { + Ok(rung) => rung, + Err(refusal) => return Ok(Handoff::RefusedBeforeRelease(refusal)), + }; + let tx = match self.store.lock() { + Ok(tx) => tx, + Err(error) => { + return Ok(Handoff::RefusedBeforeRelease(Refusal::StateNotDurable( + error, + ))); + } + }; + let previous = self.store.read(); + let record = episode::prepare(&episode, self.package, tier, &self.version); + match self.store.transition( + &tx, + self.expected(source, previous.as_ref()), + Next::Record(&record), + ) { + Ok(()) => {} + Err(error @ Refused::Stale(_)) => return Err(Stale(error.to_string())), + Err(Refused::NotDurable(detail)) => { + return Ok(Handoff::RefusedBeforeRelease(Refusal::StateNotDurable( + detail, + ))); + } + } + if episode.generation == 0 { + // Only now that the new record is durable. Pruning first would, on a + // failed record write, leave the old record current with its claim + // evidence already gone — and an old child that had passed its + // record read could then re-create its claim and write a receipt + // over the record. + self.store.prune_claims(&tx, &episode.token); + } + // Released before the spawn: the record is durable and every check that + // does not need the name released has already run. + drop(tx); + + let tag = Tag { + episode: Some(episode), + profile: rung.name.to_string(), + }; + let handoff = self.spawn_child(rung, &tag, release); + if handoff.refused() { + // No child exists, so the prepared record describes an attempt that + // will never be claimed. Left in place it would spend the rung's one + // retry on a failure that was never about rendering. + self.roll_back(&record, previous); + } + Ok(handoff) + } + + /// Restore the record a refused handoff had already overwritten. + /// + /// Identity-checked, because the lock is dropped between the prepared write + /// and the spawn: by the time a refusal comes back, a concurrent launch may + /// have prepared its own episode, and a blind restore would erase a record + /// that a live child is about to claim — turning that child's handoff into + /// an untracked launch. Only the record this call itself wrote is rolled + /// back, which is exactly what naming it as the expected source says. + fn roll_back(&self, prepared: &Record, previous: Option) { + let Ok(tx) = self.store.lock() else { + eprintln!("{LOG}: could not roll back the prepared record — the state lock was busy"); + return; + }; + let next = match &previous { + Some(record) => Next::Record(record), + None => Next::Cleared, + }; + if let Err(error) = self + .store + .transition(&tx, Some(Expected::of(prepared)), next) + { + eprintln!("{LOG}: not rolling back — {error}"); + } + } + + /// Hand off to a child running `tier` with no episode: nothing is prepared, + /// nothing is claimed, and nothing is persisted. This is what a manual + /// override is — a launch the ladder runs but does not learn from. + fn force(&self, tier: usize) -> Handoff { + match self.rung(tier) { + Ok(rung) => { + let tag = Tag { + episode: None, + profile: rung.name.to_string(), + }; + self.spawn_child(rung, &tag, &Release::NothingHeld) + } + Err(refusal) => Handoff::RefusedBeforeRelease(refusal), + } + } + + /// The release boundary itself: every preflight that can fail runs *before* + /// `release.run()`, and everything after it is reported as a post-release + /// outcome. + /// + /// Resolving the binary belongs on this side of the line. Held as an + /// unresolved `Result` and passed onward it would surface only inside the + /// launcher — after the name was already gone — turning a plainly + /// recoverable "cannot find my own executable" into a stranded app. + fn spawn_child(&self, rung: &'static Tier, tag: &Tag, release: &Release<'_>) -> Handoff { + let binary = match tauri::process::current_binary(&tauri::Env::default()) { + Ok(binary) => binary, + Err(error) => { + return Handoff::RefusedBeforeRelease(Refusal::NoBinary(error.to_string())); + } + }; + + let released = release.run(); + match (self.launch)( + &self.dbus_name, + &binary, + &self.args, + self.package, + rung, + tag, + ) { + Ok(pid) => { + eprintln!("{LOG}: handed off to {} as pid {pid}", rung.name); + Handoff::Launched + } + Err(refusal) => match released { + true => Handoff::RefusedAfterRelease(refusal), + false => Handoff::RefusedBeforeRelease(refusal), + }, + } + } +} diff --git a/desktop/src-tauri/src/render_recovery/state.rs b/desktop/src-tauri/src/render_recovery/state.rs index 80a29312b5..feef8ef9b2 100644 --- a/desktop/src-tauri/src/render_recovery/state.rs +++ b/desktop/src-tauri/src/render_recovery/state.rs @@ -13,12 +13,19 @@ //! fused with the record write into a single durable step. //! //! An exclusive claim only serializes the creation of one claim file, which is -//! not enough on its own: reading the record, validating it, and writing the +//! not enough on its own: reading the record, deciding from it, and writing the //! next phase are three operations, and a writer descheduled between them could -//! stamp a stale phase onto a newer episode. So every transition runs inside -//! [`Store::lock`], and every receipt additionally has to prove the record is -//! still the one it belongs to (`episode::may_advance`). The lock orders -//! writers; the identity check is what makes a delayed writer harmless. +//! stamp a stale phase onto a newer episode. So [`Store::transition`] is the +//! only way to make anything durable, and it is compare-and-transition: it +//! re-reads the record under the caller's [`Transaction`] and applies the write +//! only if the record still exactly matches the one the caller decided from. +//! The lock orders writers; the comparison is what makes a delayed writer +//! harmless. +//! +//! Because the expected source names a *phase* and not merely an identity, the +//! chain is exact for free: a caller writing `owned` declares that it expects +//! `started`, so `prepared → owned` is rejected as a mismatch with no separate +//! ordering rule to keep in step with the phase list. use serde::{Deserialize, Serialize}; use std::io::Write as _; @@ -55,31 +62,6 @@ pub(crate) enum Phase { Exhausted, } -impl Phase { - /// Position in the `prepared → started → owned → confirmed` chain, used to - /// reject a receipt that would move an episode backwards. - /// - /// `Exhausted` is deliberately above every other phase: it is terminal, and - /// a late receipt from the attempt that exhausted the ladder must not - /// reopen it. - fn rank(self) -> u8 { - match self { - Phase::Prepared => 0, - Phase::Started => 1, - Phase::Owned => 2, - Phase::Confirmed => 3, - Phase::Exhausted => 4, - } - } - - /// Whether `next` is a forward move from `self`. Equal phases are not - /// forward: a repeated receipt has nothing to add, and rejecting it keeps - /// the transition idempotent rather than merely harmless. - pub(crate) fn precedes(self, next: Phase) -> bool { - self.rank() < next.rank() - } -} - /// The durable record. One file, rewritten atomically at each transition. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub(crate) struct Record { @@ -131,6 +113,116 @@ impl Record { } } +/// The record state a transition requires *before* it writes anything. +/// +/// Every mutation names the state its caller decided from and is refused if the +/// record has moved since. The comparison is on `(token, generation)` plus +/// optionally the phase; pid, owner identity, and tier are payload a transition +/// records rather than depends on. +/// +/// `phase: Some(_)` is what makes the receipt chain exact. A receipt is written +/// with the phase it must follow — `owned` expects `started` — so a skip is an +/// ordinary mismatch, and there is no separate ordering rule that could drift +/// out of step with the phase list. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct Expected<'a> { + token: &'a str, + generation: u32, + /// `None` matches this attempt at any phase. + phase: Option, +} + +impl<'a> Expected<'a> { + /// Exactly the record the caller has in hand, phase included. What a + /// decision taken from an earlier read must prove is still current. + pub(crate) fn of(record: &'a Record) -> Self { + Expected { + token: &record.token, + generation: record.generation, + phase: Some(record.phase), + } + } + + /// This attempt at exactly `phase`. Used by receipts, which name the phase + /// they follow rather than holding the record that carries it. + pub(crate) fn after(token: &'a str, generation: u32, phase: Phase) -> Self { + Expected { + token, + generation, + phase: Some(phase), + } + } + + /// This attempt wherever the chain has reached. + /// + /// For a live process writing about its own episode: it is the only writer + /// of that episode's receipts, so any phase of it is a legitimate source, + /// and what must still be caught — a reset, or a newer episode — changes the + /// token or clears the record either way. + pub(crate) fn attempt(token: &'a str, generation: u32) -> Self { + Expected { + token, + generation, + phase: None, + } + } + + fn holds(&self, current: Option<&Record>) -> bool { + current.is_some_and(|current| { + current.token == self.token + && current.generation == self.generation + && self.phase.is_none_or(|phase| current.phase == phase) + }) + } +} + +/// What a transition makes durable. +#[derive(Clone, Copy, Debug)] +pub(crate) enum Next<'a> { + /// Replace the record with this one. + Record(&'a Record), + /// Delete the record and every claim. `--reset-rendering-mode`, and the + /// discard of a record written by another app version. + Cleared, +} + +/// Why a transition did not happen. +/// +/// Both variants carry a description rather than the record involved: every +/// caller either reports the refusal or re-decides from a fresh read, and none +/// inspects the record that was found. +#[derive(Debug)] +pub(crate) enum Refused { + /// The record is no longer the one the caller decided from, so whatever the + /// caller intended rests on state that no longer exists. The caller must + /// reconcile again rather than proceed. + Stale(String), + /// The record could not be made durable. + NotDurable(String), +} + +impl Refused { + /// Describe what was found where the caller expected its own source. + fn stale(found: Option<&Record>) -> Self { + Refused::Stale(match found { + Some(found) => format!( + "the episode record is now {:?} (token={} gen={})", + found.phase, found.token, found.generation + ), + None => "the episode record was cleared".to_string(), + }) + } +} + +impl std::fmt::Display for Refused { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Refused::Stale(detail) => f.write_str(detail), + Refused::NotDurable(error) => write!(f, "the record is not durable: {error}"), + } + } +} + /// Filesystem home of the episode record and its claim files. pub(crate) struct Store { dir: PathBuf, @@ -215,13 +307,42 @@ impl Store { serde_json::from_str(&text).ok() } - /// Atomically replace the record. Durability matters here: a torn record - /// after a crash would be indistinguishable from a corrupt one and would - /// silently restart the ladder. + /// Atomically replace the record, or delete it, if the record still matches + /// `expected` — `None` meaning it must still be absent. + /// + /// This is the only way anything durable changes. The re-read happens under + /// the caller's `Transaction`, so between the comparison and the write no + /// other process can intervene — which is exactly what a caller that decided + /// from an earlier read needs, since its decision may have been overtaken by + /// a reset or a newer episode in the meantime. /// - /// Requires a `Transaction`, so the read this write is based on and the - /// write itself are always inside one critical section. - pub(crate) fn write(&self, _tx: &Transaction, record: &Record) -> Result<(), String> { + /// Durability matters for the write: a torn record after a crash would be + /// indistinguishable from a corrupt one and would silently restart the + /// ladder. + pub(crate) fn transition( + &self, + _tx: &Transaction, + expected: Option>, + next: Next<'_>, + ) -> Result<(), Refused> { + let current = self.read(); + let holds = match expected { + Some(expected) => expected.holds(current.as_ref()), + None => current.is_none(), + }; + if !holds { + return Err(Refused::stale(current.as_ref())); + } + match next { + Next::Cleared => { + let _ = std::fs::remove_dir_all(&self.dir); + Ok(()) + } + Next::Record(record) => self.write(record).map_err(Refused::NotDurable), + } + } + + fn write(&self, record: &Record) -> Result<(), String> { use atomic_write_file::AtomicWriteFile; std::fs::create_dir_all(&self.dir) @@ -236,12 +357,10 @@ impl Store { .map_err(|e| format!("commit {}: {e}", path.display())) } - /// Delete the record and every claim. Used by `--reset-rendering-mode` and - /// when a record cannot be parsed. Returns whether anything was there. - pub(crate) fn clear(&self, _tx: &Transaction) -> bool { - let existed = self.dir.exists(); - let _ = std::fs::remove_dir_all(&self.dir); - existed + /// Whether any renderer state exists, for `--reset-rendering-mode`'s report + /// of what it cleared. Called under the same transaction as the clear. + pub(crate) fn exists(&self, _tx: &Transaction) -> bool { + self.dir.exists() } /// Drop the claim files of superseded episodes, keeping every claim that @@ -318,10 +437,16 @@ impl Store { } /// Write a record outside a transition, for tests that need a starting - /// state. Takes and drops its own lock. + /// state. Takes and drops its own lock, and expects whatever is there. #[cfg(test)] pub(crate) fn seed_record(&self, record: &Record) { let tx = self.lock().expect("lock"); - self.write(&tx, record).expect("write"); + let current = self.read(); + self.transition( + &tx, + current.as_ref().map(Expected::of), + Next::Record(record), + ) + .expect("seed"); } } diff --git a/desktop/src-tauri/src/render_recovery/tests.rs b/desktop/src-tauri/src/render_recovery/tests.rs index 19b692c607..04b0160e9f 100644 --- a/desktop/src-tauri/src/render_recovery/tests.rs +++ b/desktop/src-tauri/src/render_recovery/tests.rs @@ -19,13 +19,13 @@ use super::classify::{decide, Decision, Ownership}; use super::cli; use super::dbus::{Observation, Owner}; use super::episode::{ - advance, advances_ladder, claim, fresh, may_advance, prepare, retry_of, Advance, Claimed, + advance, advances_ladder, claim, fresh, predecessor_of, prepare, retry_of, Advance, Claimed, Episode, Termination, CRASH_ELIGIBILITY_WINDOW, }; use super::launcher::{exact_env_command, Refusal, Tag, EPISODE, FORCED, GENERATION, PROFILE}; use super::profiles::{owned_present, Package, Tier}; use super::session::{reconcile, Boot, CrashResponse, Disabled}; -use super::state::{Phase, Record, Store}; +use super::state::{Expected, Next, Phase, Record, Store}; const VERSION: &str = "0.4.26"; const IDENTIFIER: &str = "xyz.block.buzz.app"; @@ -120,24 +120,109 @@ fn test_stale_generation_claim_does_not_block_the_retry() { assert_eq!(store.read().expect("record").phase, Phase::Started); } +/// Environment variable that turns `claimer` from a no-op into a real claim +/// attempt against the state dir it names. Set only by the test below, on the +/// children it spawns. +const CLAIMER_STATE_DIR: &str = "BUZZ_TEST_CLAIMER_STATE_DIR"; + +/// Exit codes a claimer child reports its outcome with, since a claim result +/// cannot be returned across a process boundary. +const EXIT_OWNER: i32 = 10; +const EXIT_SUPERSEDED: i32 = 11; + +/// One of the two racing children of `test_two_children_of_one_generation...`. +/// +/// A separate `#[test]` because that is what makes it re-executable: the test +/// binary can be asked for exactly this name, which yields a *real* process with +/// its own pid — the only way to prove the receipt names the winner rather than +/// merely that one receipt exists. Inert in an ordinary run, where the variable +/// is unset. +#[test] +fn claimer() { + let Some(dir) = std::env::var_os(CLAIMER_STATE_DIR) else { + return; + }; + let dir = std::path::PathBuf::from(dir); + + // Rendezvous: each child announces itself, then waits for the other, so both + // are inside `claim` at once and the exclusive claim and the state lock are + // contended for real rather than in sequence. + let barrier = dir.join("barrier"); + std::fs::create_dir_all(&barrier).expect("barrier dir"); + std::fs::write(barrier.join(std::process::id().to_string()), b"").expect("arrive"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while std::fs::read_dir(&barrier).map(|e| e.count()).unwrap_or(0) < 2 { + assert!( + std::time::Instant::now() < deadline, + "the sibling claimer never arrived" + ); + std::thread::sleep(std::time::Duration::from_millis(2)); + } + + let claimed = claim( + &Store::new(&dir), + &Episode { + token: TOKEN.to_string(), + generation: 1, + }, + ); + // `exit`, not a panic: the outcome has to survive as a status the parent can + // read, and the harness would turn a panic into a plain failure. + std::process::exit(match claimed { + Claimed::Owner { .. } => EXIT_OWNER, + Claimed::Superseded(_) => EXIT_SUPERSEDED, + Claimed::Untracked(reason) => panic!("neither child should be untracked: {reason}"), + }); +} + #[test] fn test_two_children_of_one_generation_produce_exactly_one_receipt() { - let (_dir, store) = store(); + let (dir, store) = store(); store.seed_record(&record(Phase::Prepared, 1, 1)); - let episode = Episode { - token: TOKEN.to_string(), - generation: 1, - }; - let first = claim(&store, &episode); - let second = claim(&store, &episode); + // Two real processes, so the two claimers have different pids and the + // receipt can be checked against the identity of the one that won. + let children: Vec = (0..2) + .map(|_| { + std::process::Command::new(std::env::current_exe().expect("test binary")) + .args(["--exact", "render_recovery::tests::claimer", "--nocapture"]) + .env(CLAIMER_STATE_DIR, dir.path()) + .spawn() + .expect("spawn a claimer") + }) + .collect(); + + let outcomes: Vec<(u32, Option)> = children + .into_iter() + .map(|mut child| { + let pid = child.id(); + (pid, child.wait().expect("claimer exit").code()) + }) + .collect(); + + let winners: Vec = outcomes + .iter() + .filter(|(_, code)| *code == Some(EXIT_OWNER)) + .map(|(pid, _)| *pid) + .collect(); + let losers = outcomes + .iter() + .filter(|(_, code)| *code == Some(EXIT_SUPERSEDED)) + .count(); - assert!(matches!(first, Claimed::Owner { .. })); - assert!(matches!(second, Claimed::Superseded(_))); + assert_eq!( + winners.len(), + 1, + "exactly one child may own the episode: {outcomes:?}" + ); + assert_eq!(losers, 1, "the other must stand down: {outcomes:?}"); + // One claim file, and the durable receipt names the process that took it. + // A loser that had written the receipt would leave the record pointing at a + // process that is not the app. assert_eq!(store.claim_count(), 1); let written = store.read().expect("record"); assert_eq!(written.phase, Phase::Started); - assert_eq!(written.pid, Some(std::process::id())); + assert_eq!(written.pid, Some(winners[0])); } #[test] diff --git a/desktop/src-tauri/src/render_recovery/tests/transitions.rs b/desktop/src-tauri/src/render_recovery/tests/transitions.rs index 3f1b09c8b7..36924caac4 100644 --- a/desktop/src-tauri/src/render_recovery/tests/transitions.rs +++ b/desktop/src-tauri/src/render_recovery/tests/transitions.rs @@ -68,7 +68,10 @@ fn test_a_stale_generation_cannot_overwrite_a_record_prepared_while_it_waited() // The retry becomes durable while the stale child is parked. let prepared = record(Phase::Prepared, 1, 1); - store.write(&tx, &prepared).expect("prepare generation 1"); + let current = store.read().expect("the generation-0 record"); + store + .transition(&tx, Some(Expected::of(¤t)), Next::Record(&prepared)) + .expect("prepare generation 1"); drop(tx); let claimed = stale.join().expect("stale child"); @@ -143,8 +146,9 @@ fn test_a_confirmation_racing_a_handoff_does_not_resurrect_its_episode() { 2, VERSION, ); + let current = store.read().expect("the owned record"); store - .write(&tx, &handed_off) + .transition(&tx, Some(Expected::of(¤t)), Next::Record(&handed_off)) .expect("prepare the next rung"); drop(tx); @@ -285,53 +289,236 @@ fn test_a_rollback_does_not_erase_an_episode_prepared_after_it() { assert_eq!(current.phase, Phase::Prepared); } -// ── Receipt currency and forward-only phases ──────────────────────────────── +// ── Stale boot decisions ──────────────────────────────────────────────────── +// +// Deciding and writing are two steps with a D-Bus round trip between them, so +// the record can move in between. These prove the write is refused rather than +// applied, and that the launch then re-decides from what is actually there. + +/// A refusing launch edge that clears the state dir first, standing in for +/// another process running `--reset-rendering-mode` inside the window between +/// this launch's classification and its prepared write. +/// +/// Refusing after the clear is what keeps the test honest: the parent reaches +/// its rollback, so a rollback that ignored the reset would restore the record +/// just as surely as a stale prepare would. +fn reset_then_refuse( + _name: &str, + _binary: &std::path::Path, + _args: &[OsString], + _package: Package, + _tier: &'static Tier, + _tag: &Tag, +) -> Result { + let path = RACING_STORE.lock().expect("path").clone().expect("path"); + let store = Store::new(&path); + let tx = store.lock().expect("lock"); + let current = store.read(); + store + .transition(&tx, current.as_ref().map(Expected::of), Next::Cleared) + .expect("reset"); + Err(Refusal::SpawnFailed("no child".to_string())) +} #[test] -fn test_a_receipt_requires_a_current_episode_and_a_forward_phase() { - let current = record(Phase::Owned, 0, 1); - let episode = Episode { +fn test_a_reset_is_not_undone_by_a_launch_that_decided_before_it() { + // The race Thufir named: A reads a failed tier and decides to advance, B + // resets and reports success, then A writes its pre-reset decision. If A's + // write lands, the user who just cleared their renderer state finds a + // prepared episode waiting for them anyway. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + store.seed_record(&record(Phase::Confirmed, 0, 2)); + *RACING_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); + let env = env_from(&[]); + + let boot = reconcile( + Ok(dir.path().to_path_buf()), + IDENTIFIER, + VERSION, + Vec::new(), + &env, + reset_then_refuse, + ); + + // The app still starts — a reset is not a reason to refuse to launch — but + // at the baseline it was launched as, with nothing persisted. + let Boot::Run(session) = boot else { + panic!("a cleared record still starts the app"); + }; + assert_eq!(session.profile_name(), "full-gpu"); + assert_eq!( + store.read(), + None, + "the reset must survive the decision taken before it" + ); +} + +#[test] +fn test_a_version_mismatch_discard_does_not_delete_a_newer_episode() { + // The mirror race: this launch classifies a record from another app version + // and decides to discard it, but by the time it writes, a current-version + // launch has prepared an episode whose child is about to claim it. A blind + // clear would turn that handoff into an untracked launch. + let dir = tempfile::tempdir().expect("tempdir"); + let store = Store::new(dir.path()); + let stale_version = Record::new(Phase::Confirmed, TOKEN, 0, "cpu-raster", 2, "0.4.25"); + store.seed_record(&stale_version); + + // Classify the old record exactly as `reconcile` does, then let the newer + // episode land before the discard is attempted. + let classified = store.read().expect("the other version's record"); + let newer = Record::new( + Phase::Prepared, + "a-newer-token", + 0, + "shm-transport", + 1, + VERSION, + ); + store.seed_record(&newer); + + let tx = store.lock().expect("lock"); + let refused = store + .transition(&tx, Some(Expected::of(&classified)), Next::Cleared) + .is_err(); + drop(tx); + + assert!(refused, "the discard must be refused, not applied"); + assert_eq!( + store.read().as_ref(), + Some(&newer), + "the newer episode must survive a delayed discard" + ); +} + +// ── The exact receipt chain ───────────────────────────────────────────────── +// +// A receipt names the phase it follows, so the chain is checked by the same +// comparison that rejects a superseded episode. These drive the real store +// rather than a predicate, which is what makes them evidence that nothing lands +// on disk. + +/// Attempt a receipt exactly as `Session::note` does — expecting this episode at +/// the phase `next` must follow — and report whether it landed. +fn receipt(store: &Store, episode: &Episode, next: Phase) -> bool { + let follows = predecessor_of(next).expect("a receipt phase has a predecessor"); + let mut written = record(next, episode.generation, 1); + written.pid = Some(std::process::id()); + let tx = store.lock().expect("lock"); + store + .transition( + &tx, + Some(Expected::after(&episode.token, episode.generation, follows)), + Next::Record(&written), + ) + .is_ok() +} + +fn episode_zero() -> Episode { + Episode { token: TOKEN.to_string(), generation: 0, - }; + } +} + +#[test] +fn test_the_receipt_chain_admits_only_the_exact_next_phase() { + let (_dir, store) = store(); + store.seed_record(&record(Phase::Started, 0, 1)); + let episode = episode_zero(); + + // Backwards and sideways: a repeated callback has nothing to add, and an + // out-of-order one must not undo a later phase. + assert!(!receipt(&store, &episode, Phase::Started)); + // The one legal move from `started`. + assert!(receipt(&store, &episode, Phase::Owned)); + assert_eq!(store.read().expect("record").phase, Phase::Owned); + assert!(receipt(&store, &episode, Phase::Confirmed)); + assert_eq!(store.read().expect("record").phase, Phase::Confirmed); +} - assert!(may_advance(Some(¤t), &episode, Phase::Confirmed)); - // Backwards and sideways are both rejected: a repeated callback has nothing - // to add, and an out-of-order one must not undo a later phase. - assert!(!may_advance(Some(¤t), &episode, Phase::Owned)); - assert!(!may_advance(Some(¤t), &episode, Phase::Started)); - // Another token's or generation's receipt is never current here. - assert!(!may_advance( - Some(¤t), - &Episode { +#[test] +fn test_a_receipt_that_skips_its_predecessor_is_rejected() { + // Rank-based "forward" accepted these. The chain is exact: a `confirmed` + // that skips `owned` would persist a crash-free startup fact for a tier that + // never recorded holding the name, and the next launch would reuse it. + for (from, skipped) in [ + (Phase::Prepared, Phase::Owned), + (Phase::Prepared, Phase::Confirmed), + (Phase::Started, Phase::Confirmed), + ] { + let (_dir, store) = store(); + let before = record(from, 0, 1); + store.seed_record(&before); + + assert!( + !receipt(&store, &episode_zero(), skipped), + "{skipped:?} must not be writable over {from:?}" + ); + assert_eq!( + store.read().as_ref(), + Some(&before), + "a rejected receipt must leave the record untouched" + ); + } +} + +#[test] +fn test_a_receipt_for_a_superseded_episode_is_rejected() { + let (_dir, store) = store(); + store.seed_record(&record(Phase::Started, 0, 1)); + + // Another token, and another generation of this token: neither is current. + for episode in [ + Episode { token: "another-token".to_string(), - generation: 0 + generation: 0, }, - Phase::Confirmed - )); - assert!(!may_advance( - Some(¤t), - &Episode { + Episode { token: TOKEN.to_string(), - generation: 1 + generation: 1, }, - Phase::Confirmed - )); - // A cleared record is not something a receipt may recreate: the record a - // receipt belongs to is written by the parent before the child exists. - assert!(!may_advance(None, &episode, Phase::Confirmed)); + ] { + assert!(!receipt(&store, &episode, Phase::Owned)); + } + assert_eq!(store.read().expect("record").phase, Phase::Started); +} + +#[test] +fn test_a_receipt_does_not_recreate_a_cleared_record() { + // The record a receipt belongs to is written by the parent before the child + // exists, so its absence means the episode was cleared — not that a receipt + // should bring it back. + let (_dir, store) = store(); + assert!(!receipt(&store, &episode_zero(), Phase::Owned)); + assert_eq!(store.read(), None); } #[test] fn test_the_exhausted_phase_is_terminal_against_a_late_receipt() { - // `Exhausted` outranks every other phase, so the attempt that exhausted the - // ladder cannot reopen it with a receipt that arrives afterwards. + // `exhausted` is written from any phase of its own attempt, so nothing in + // the chain leads out of it: every receipt names a predecessor, and + // `exhausted` is not one. + let (_dir, store) = store(); let exhausted = record(Phase::Exhausted, 0, 3); - let episode = Episode { - token: TOKEN.to_string(), - generation: 0, - }; + store.seed_record(&exhausted); + let episode = episode_zero(); + for phase in [Phase::Started, Phase::Owned, Phase::Confirmed] { - assert!(!may_advance(Some(&exhausted), &episode, phase)); + assert!(!receipt(&store, &episode, phase)); } + assert_eq!(store.read().as_ref(), Some(&exhausted)); +} + +#[test] +fn test_only_prepared_and_exhausted_have_no_predecessor() { + // The two phases no receipt writes: `prepared` is written by a parent about + // an attempt that does not exist yet, and `exhausted` is validated against + // the whole attempt instead of one position. + assert_eq!(predecessor_of(Phase::Started), Some(Phase::Prepared)); + assert_eq!(predecessor_of(Phase::Owned), Some(Phase::Started)); + assert_eq!(predecessor_of(Phase::Confirmed), Some(Phase::Owned)); + assert_eq!(predecessor_of(Phase::Prepared), None); + assert_eq!(predecessor_of(Phase::Exhausted), None); } From 24a0a96c396044616c77cf0ade803d5f5c305373 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 12:56:49 -0400 Subject: [PATCH 4/4] test(desktop): give each racing launch edge its own store slot The two tests that inject a competing write from inside the spawn window shared one static path slot. The launch edge is a bare fn pointer and captures nothing, so the path has to reach it through a static -- but with one slot the tests overwrite each other's path whenever the harness runs them in parallel, which fails the assertions in CI while passing under a luckier local ordering. Verified by reintroducing the shared slot: 6/6 runs fail. With a slot per edge, 8/8 pass. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src/render_recovery/tests/transitions.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/desktop/src-tauri/src/render_recovery/tests/transitions.rs b/desktop/src-tauri/src/render_recovery/tests/transitions.rs index 36924caac4..db63b2ec13 100644 --- a/desktop/src-tauri/src/render_recovery/tests/transitions.rs +++ b/desktop/src-tauri/src/render_recovery/tests/transitions.rs @@ -230,10 +230,15 @@ fn test_two_termination_callbacks_hand_off_exactly_once() { assert_eq!(store.claim_count(), 0); } -/// App-data path for the rollback race below, so the refusing launch edge can -/// write a competing record from inside the window the parent left open. A -/// `static` for the same reason as `SPAWNS`: the launch edge captures nothing. -static RACING_STORE: std::sync::Mutex> = std::sync::Mutex::new(None); +/// Where a racing launch edge finds the store to act on. +/// +/// The launch edge is a bare `fn` pointer and captures nothing, so the path has +/// to reach it through a `static`. Each racing edge therefore gets its OWN +/// `static` rather than sharing one: two tests using the same slot overwrite +/// each other's path when the harness runs them in parallel, which is a race in +/// the test scaffolding rather than in the code under test. +static ROLLBACK_STORE: std::sync::Mutex> = std::sync::Mutex::new(None); +static RESET_STORE: std::sync::Mutex> = std::sync::Mutex::new(None); /// The competing record: a different episode, prepared by "another launch" /// during the spawn window, then refused so the parent reaches its rollback. @@ -245,7 +250,7 @@ fn refuse_after_a_competing_prepare( _tier: &'static Tier, _tag: &Tag, ) -> Result { - let path = RACING_STORE.lock().expect("path").clone().expect("path"); + let path = ROLLBACK_STORE.lock().expect("path").clone().expect("path"); Store::new(&path).seed_record(&Record::new( Phase::Prepared, "a-newer-token", @@ -267,7 +272,7 @@ fn test_a_rollback_does_not_erase_an_episode_prepared_after_it() { let dir = tempfile::tempdir().expect("tempdir"); let store = Store::new(dir.path()); store.seed_record(&record(Phase::Confirmed, 0, 2)); - *RACING_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); + *ROLLBACK_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); let env = env_from(&[]); // Wants tier 2, so it prepares a record, spawns, and is refused. @@ -310,7 +315,7 @@ fn reset_then_refuse( _tier: &'static Tier, _tag: &Tag, ) -> Result { - let path = RACING_STORE.lock().expect("path").clone().expect("path"); + let path = RESET_STORE.lock().expect("path").clone().expect("path"); let store = Store::new(&path); let tx = store.lock().expect("lock"); let current = store.read(); @@ -329,7 +334,7 @@ fn test_a_reset_is_not_undone_by_a_launch_that_decided_before_it() { let dir = tempfile::tempdir().expect("tempdir"); let store = Store::new(dir.path()); store.seed_record(&record(Phase::Confirmed, 0, 2)); - *RACING_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); + *RESET_STORE.lock().expect("path") = Some(dir.path().to_path_buf()); let env = env_from(&[]); let boot = reconcile(