From 1b22005b4005bd7b15d886776e9e82eaabded7b7 Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Thu, 12 Nov 2026 11:34:38 +0800 Subject: [PATCH 1/6] fix(runtime): force-stop active machines and preserve parallel Store writes Port the verified force-stop and native concurrency changes onto current layerfs_v2_0_0. Stop providers without a live worker, retain saved data and runtime registration custody, and advertise machine.stop force. Restore macOS writer and Store lifecycle admission, and treat disappearing stale leases as already released. Preserve upstream Sandbox recovery slots, pool/capacity fixes and API retirements; omit obsolete image override handling. Ported-from: 7b5f5a56305f6aeb10fb8d96cf15d7ee71c6c316 Validation: PG CLI build and warning-denying all-targets Clippy; API dead-worker force; live mounted/running native machine and Sandbox force using a matching private daemon; upstream bridge admission/setup and pool/capacity tests. Before the unrelated upstream recovery-slot merge, the unchanged persistence, lease, provider and CLI scopes passed 20 native, one lease, 24 provider and seven CLI discovery/framing tests; boundary and 182 tooling tests also passed. The old test daemon's private-config refusal was diagnosed as upstream's LFSD08-to-LFSD09 startup protocol change; its matching-daemon rerun passed. No mutation retries were added. Production LOC: 158658 -> 159105 (delta +447) Scope: all first-party core, engine, control and API product sources including runtime SQL, Python and JavaScript; excludes tests, docs, tooling and generated artifacts. Method: tools/production_loc.py --root --json on parent dbd633facd4959debef999e986e80fafd868e51b and staged tree 9b47c8dd6f3fdb0b1d5edb31ec0fe97cf6b3bdb8. Counter SHA256: d3148db88f91b09eb74eb659b2f700209776a24d022f09774dc7bb38ea1eecf6 Reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Assisted-by: Cindy (https://cindy.app) --- api/crates/layerfs-api/src/machine/stop.rs | 12 +- api/crates/layerfs-api/tests/machine_force.rs | 196 +++++++++++++ .../layerfs-api/tests/machine_force_native.rs | 204 ++++++++++++++ api/crates/layerfs-cli/tests/capabilities.rs | 33 +++ .../layerfs-cli/tests/common/stream_worker.rs | 2 +- api/crates/layerfs-cli/tests/stream_framed.rs | 16 +- control/crates/layerfs-control/src/lib.rs | 2 + control/crates/layerfs-control/src/machine.rs | 5 + .../layerfs-control/src/machine_force.rs | 260 ++++++++++++++++++ .../crates/layerfs-machine/src/endpoint.rs | 1 + control/crates/layerfs-machine/src/lib.rs | 7 + control/crates/layerfs-machine/src/power.rs | 14 + .../crates/layerfs-platform/src/adapter.rs | 2 +- control/crates/layerfs-platform/src/lib.rs | 2 +- .../src/platforms/apple_container/adapter.rs | 4 +- .../src/platforms/apple_container/power.rs | 5 +- .../src/platforms/docker/adapter.rs | 4 +- .../src/platforms/docker/power.rs | 15 +- .../src/platforms/docker/windows.rs | 2 +- .../src/platforms/lima/adapter.rs | 4 +- .../src/platforms/lima/power.rs | 12 +- .../src/platforms/native/adapter.rs | 2 +- .../layerfs-platform/src/setup/calls.rs | 9 +- .../crates/layerfs-platform/src/setup/mod.rs | 2 +- .../layerfs-platform/tests/machine_force.rs | 126 +++++++++ .../src/backend/host_process/sandbox.rs | 25 ++ .../tests/local_stop_mount.rs | 26 ++ control/crates/layerfs-sandbox/src/direct.rs | 3 + control/crates/layerfs-sandbox/src/get.rs | 1 + .../layerfs-sandbox/src/record/sandbox.rs | 43 +++ control/crates/layerfs-sandbox/src/reopen.rs | 23 ++ control/crates/layerfs-sandbox/src/route.rs | 3 + .../layerfs-sandbox/src/workspace/direct.rs | 3 + control/crates/layerfs-store/src/lease.rs | 23 +- control/crates/layerfs-store/src/place.rs | 3 + .../crates/layerfs-store/tests/lease_race.rs | 56 ++++ docs/architecture/api/sdk/machine/README.md | 2 +- docs/architecture/api/sdk/machine/stop.md | 39 ++- docs/architecture/engine/store/backends.md | 34 ++- docs/architecture/glossary.md | 2 +- .../src/backend/sqlite/lane/lane.rs | 2 +- .../backend/sqlite/lane/writer_admission.rs | 25 +- .../src/backend/sqlite/lifecycle/seal.rs | 20 +- .../src/backend/sqlite/session/background.rs | 3 + .../src/backend/sqlite/session/connection.rs | 14 +- .../src/backend/sqlite/session/lifecycle.rs | 61 ++++ .../src/backend/sqlite/session/mod.rs | 1 + .../tests/sqlite_lock_custody.rs | 2 +- .../tests/writer_admission.rs | 77 +++++- 49 files changed, 1349 insertions(+), 83 deletions(-) create mode 100644 api/crates/layerfs-api/tests/machine_force.rs create mode 100644 api/crates/layerfs-api/tests/machine_force_native.rs create mode 100644 control/crates/layerfs-control/src/machine_force.rs create mode 100644 control/crates/layerfs-platform/tests/machine_force.rs create mode 100644 control/crates/layerfs-store/tests/lease_race.rs create mode 100644 engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs diff --git a/api/crates/layerfs-api/src/machine/stop.rs b/api/crates/layerfs-api/src/machine/stop.rs index 90a11099df..0c0dedc807 100644 --- a/api/crates/layerfs-api/src/machine/stop.rs +++ b/api/crates/layerfs-api/src/machine/stop.rs @@ -1,6 +1,6 @@ //! `machine.stop`: stop a machine's worker and VM; its disks stay. -use crate::calls::table::Kind::Name; -use crate::calls::table::{a, call, Call}; +use crate::calls::table::Kind::{Bool, Name}; +use crate::calls::table::{a, call, opt, Call}; use crate::calls::Context; use layerfs_bridge::control::SdkValue; #[cfg(feature = "machine")] @@ -12,7 +12,7 @@ pub(crate) const CALL: Call = call( "machine", "stop", &[a("name", Name)], - &[], + &[opt("force", Bool)], "stop a machine's worker and VM; its disks stay", run, ); @@ -21,7 +21,11 @@ pub(crate) const CALL: Call = call( #[cfg(feature = "machine")] fn run(context: &mut Context<'_>, args: &SdkValue) -> Outcome { let name = MachineName::parse(args.get_text("name").unwrap_or(""))?; - layerfs_control::machine::stop(&context.home, &name) + if args.get_bool("force") { + layerfs_control::machine::stop_force(&context.home, &name) + } else { + layerfs_control::machine::stop(&context.home, &name) + } } /// Without the `machine` feature (the guest worker's build) it is diff --git a/api/crates/layerfs-api/tests/machine_force.rs b/api/crates/layerfs-api/tests/machine_force.rs new file mode 100644 index 0000000000..e12b6d5799 --- /dev/null +++ b/api/crates/layerfs-api/tests/machine_force.rs @@ -0,0 +1,196 @@ +//! Provider-direct force stop preserves storage and avoids a dead worker entirely. +#![cfg(feature = "machine")] +use layerfs_api::home::{self, paths, read_record, write_record}; +use layerfs_bridge::control::SdkValue; +use std::{net::TcpListener, os::unix::fs::PermissionsExt, path::Path}; +fn t(value: &str) -> SdkValue { + SdkValue::text(value) +} +fn call( + home: &Path, + name: &str, + args: &[(&str, SdkValue)], +) -> Result { + layerfs_api::call( + Some(home), + name, + SdkValue::Map( + args.iter() + .map(|(name, value)| ((*name).to_owned(), value.clone())) + .collect(), + ), + &mut |_| {}, + ) +} +#[test] +fn force_stop_bypasses_in_use_and_dead_worker_preserving_disk_store_and_records() { + let root = std::env::temp_dir().join(format!("lfs-force-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let home = root.join("home"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::set_permissions(&home, std::fs::Permissions::from_mode(0o700)).unwrap(); + home::open(&home).unwrap(); + let tool = root.join("limactl"); + std::fs::write( + &tool, + r#"#!/bin/sh +d=${0%/*} +echo "$*" >> "$d/calls" +case "$1" in + list) printf 'Running\t/nonexistent\n';; + stop) test "$2" = --force || exit 19; test ! -f "$d/refuse" || exit 18;; + *) exit 17;; +esac +"#, + ) + .unwrap(); + std::fs::set_permissions(&tool, std::fs::Permissions::from_mode(0o700)).unwrap(); + std::env::set_var("PATH", &root); + write_record( + &paths::machine_record(&home, "vm"), + &SdkValue::map([ + ("name", t("vm")), + ("platform", t("lima")), + ("state", t("running")), + ]), + ) + .unwrap(); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let worker = SdkValue::map([ + ("address", t(&listener.local_addr().unwrap().to_string())), + ("secret", t("aa")), + ]); + for machine in ["vm"] { + write_record(&paths::worker_record(&home, machine), &worker).unwrap(); + } + for (sandbox, machine) in [("one", "vm"), ("two", "vm")] { + write_record( + &paths::sandbox_record(&home, sandbox), + &SdkValue::map([ + ("name", t(sandbox)), + ("id", t(sandbox)), + ("machine", t(machine)), + ("project", t("shop/app")), + ]), + ) + .unwrap(); + write_record( + &paths::workspace_dir(&home, sandbox).join("work.json"), + &SdkValue::map([("name", t("work")), ("path", t("/workspaces/one"))]), + ) + .unwrap(); + } + let (store, disk) = (paths::store_file(&home, "shop"), root.join("disk.img")); + write_record( + &paths::store_record(&home, "shop"), + &SdkValue::map([ + ("name", t("shop")), + ("kind", t("sqlite")), + ("machine", t("vm")), + ("profile", t("disposable")), + ]), + ) + .unwrap(); + let store_record = std::fs::read(paths::store_record(&home, "shop")).unwrap(); + std::fs::write(&store, b"saved-store").unwrap(); + std::fs::write(&disk, b"saved-disk").unwrap(); + let denied = call(&home, "machine.stop", &[("name", t("vm"))]).unwrap_err(); + assert_eq!(denied.code, "InUse"); + let stopped = call( + &home, + "machine.stop", + &[("name", t("vm")), ("force", SdkValue::Bool(true))], + ) + .unwrap(); + assert_eq!(stopped.get_text("state"), Some("stopped")); + assert!(stopped.get_bool("forced")); + assert_eq!( + stopped.get("stopped_sandboxes"), + Some(&SdkValue::List(vec![t("one"), t("two")])) + ); + assert_eq!( + stopped.get("unknown"), + Some(&SdkValue::List(vec![t("one:work"), t("two:work")])) + ); + assert!( + matches!(listener.accept(),Err(error) if error.kind()==std::io::ErrorKind::WouldBlock), + "worker must never be contacted" + ); + for machine in ["vm"] { + assert!(!paths::worker_record(&home, machine).exists()); + assert!(paths::machine_record(&home, machine).exists()); + } + let sandbox = call(&home, "sandbox.get", &[("name", t("one"))]).unwrap(); + assert_eq!(sandbox.get_text("state"), Some("gone")); + let logs = std::fs::read_to_string(root.join("calls")).unwrap(); + assert_eq!( + logs.lines() + .filter(|line| line.starts_with("stop --force")) + .count(), + 1 + ); + assert!(!logs.contains("start") && !logs.contains("delete")); + assert_eq!(std::fs::read(&store).unwrap(), b"saved-store"); + assert_eq!( + std::fs::read(paths::store_record(&home, "shop")).unwrap(), + store_record + ); + assert_eq!(std::fs::read(&disk).unwrap(), b"saved-disk"); + assert!(read_record(&paths::machine_record(&home, "vm")) + .unwrap() + .unwrap() + .get("op") + .is_none()); + let again = call( + &home, + "machine.stop", + &[("name", t("vm")), ("force", SdkValue::Bool(true))], + ) + .unwrap(); + assert_eq!(again.get_text("state"), Some("stopped")); + // A failed later provider operation retains worker/state and explicit unresolved custody. + write_record(&paths::worker_record(&home, "vm"), &worker).unwrap(); + std::fs::write(root.join("refuse"), b"fail once").unwrap(); + let before = std::fs::read(paths::worker_record(&home, "vm")).unwrap(); + let failed = call( + &home, + "machine.stop", + &[("name", t("vm")), ("force", SdkValue::Bool(true))], + ) + .unwrap_err(); + assert_eq!(failed.code, "Failed"); + assert_eq!( + std::fs::read(paths::worker_record(&home, "vm")).unwrap(), + before + ); + let uncertain = call(&home, "sandbox.list", &[]).unwrap(); + assert!(format!("{uncertain:?}").contains("unresolved")); + let machine = read_record(&paths::machine_record(&home, "vm")) + .unwrap() + .unwrap(); + assert!(machine.get("op").unwrap().get_bool("uncertain")); + // Only a fresh explicit request reconciles the uncertain stop; no internal retry occurred. + std::fs::remove_file(root.join("refuse")).unwrap(); + let recovered = call( + &home, + "machine.stop", + &[("name", t("vm")), ("force", SdkValue::Bool(true))], + ) + .unwrap(); + assert_eq!(recovered.get_text("state"), Some("stopped")); + assert_eq!( + call(&home, "sandbox.get", &[("name", t("one"))]) + .unwrap() + .get_text("state"), + Some("gone") + ); + let native = call( + &home, + "machine.stop", + &[("name", t("native")), ("force", SdkValue::Bool(true))], + ) + .unwrap(); + assert_eq!(native.get_text("state"), Some("ready")); + std::fs::remove_dir_all(root).unwrap(); +} diff --git a/api/crates/layerfs-api/tests/machine_force_native.rs b/api/crates/layerfs-api/tests/machine_force_native.rs new file mode 100644 index 0000000000..16a274ffed --- /dev/null +++ b/api/crates/layerfs-api/tests/machine_force_native.rs @@ -0,0 +1,204 @@ +//! Live native machine force stop and Sandbox force close with mounted, executing Workspaces. +#![cfg(all(target_os = "macos", feature = "machine"))] +use layerfs_api::home::{self, paths, read_record}; +use layerfs_bridge::control::SdkValue; +use layerfs_bridge::process::LocalProcess; +use std::{ + path::Path, + process::{Child, Command}, + time::{Duration, Instant}, +}; +fn t(value: &str) -> SdkValue { + SdkValue::text(value) +} +fn call(home: &Path, name: &str, args: &[(&str, SdkValue)]) -> SdkValue { + layerfs_api::call( + Some(home), + name, + SdkValue::Map( + args.iter() + .map(|(name, value)| ((*name).to_owned(), value.clone())) + .collect(), + ), + &mut |_| {}, + ) + .unwrap_or_else(|error| panic!("{name}: {error:?}")) +} +struct Helper(Child); +impl Drop for Helper { + fn drop(&mut self) { + if self.0.try_wait().ok().flatten().is_none() { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } +} +#[test] +fn exec_helper() { + let Ok(home) = std::env::var("LAYERFS_FORCE_NATIVE_HELPER") else { + return; + }; + let home = Path::new(&home); + let name = std::env::var("LAYERFS_FORCE_NATIVE_NAME").unwrap(); + let _ = layerfs_api::call( + Some(home), + "workspace.exec", + SdkValue::map([ + ("ws", t(&format!("{name}:work"))), + ( + "command", + t(&format!( + "printf dirty > pending.txt; echo $$ > '{}'; sleep 60", + home.join(format!("{name}.pid")).display() + )), + ), + ]), + &mut |_| {}, + ); +} +fn live_case(home: &Path, name: &str, machine: bool) { + call( + home, + "sandbox.create", + &[ + ("project", t("shop/app")), + ("name", t(name)), + ("machine", t("native")), + ], + ); + let mounted = call( + home, + "workspace.mount", + &[ + ("sandbox", t(name)), + ("branch", t("work")), + ("name", t("work")), + ], + ); + let mount = mounted.get_text("path").unwrap().to_owned(); + let record = read_record(&paths::sandbox_record(home, name)) + .unwrap() + .unwrap(); + let daemon = record.get("daemon").unwrap(); + let process = LocalProcess::observe(daemon.get_int("pid").unwrap() as u32).unwrap(); + let mut helper = Helper( + Command::new(std::env::current_exe().unwrap()) + .args(["exec_helper", "--exact", "--nocapture"]) + .env("LAYERFS_FORCE_NATIVE_HELPER", home) + .env("LAYERFS_FORCE_NATIVE_NAME", name) + .spawn() + .unwrap(), + ); + let marker = home.join(format!("{name}.pid")); + let until = Instant::now() + Duration::from_secs(10); + while !marker.exists() { + assert!(Instant::now() < until, "exec never became ready"); + std::thread::sleep(Duration::from_millis(20)); + } + let command = LocalProcess::observe( + std::fs::read_to_string(marker) + .unwrap() + .trim() + .parse() + .unwrap(), + ) + .unwrap(); + assert!(process.alive() && command.alive()); + if machine { + let stopped = call( + home, + "machine.stop", + &[("name", t("native")), ("force", SdkValue::Bool(true))], + ); + assert_eq!(stopped.get_text("state"), Some("ready")); + assert_eq!( + call(home, "sandbox.get", &[("name", t(name))]).get_text("state"), + Some("gone") + ); + // An explicit second stop of retained terminal records is an idempotent no-op. + let again = call( + home, + "machine.stop", + &[("name", t("native")), ("force", SdkValue::Bool(true))], + ); + assert_eq!(again.get_text("state"), Some("ready")); + } else { + let closed = call( + home, + "sandbox.close", + &[("name", t(name)), ("force", SdkValue::Bool(true))], + ); + assert!(!paths::sandbox_record(home, name).exists()); + assert!( + closed.get("unknown").is_some(), + "active Workspace frontier is explicitly unknown" + ); + } + assert!( + !process.alive() && !command.alive(), + "exact daemon and command processes ended" + ); + let until = Instant::now() + Duration::from_secs(5); + while helper.0.try_wait().unwrap().is_none() { + assert!(Instant::now() < until, "exec helper remained live"); + std::thread::sleep(Duration::from_millis(20)); + } + let mounts = Command::new("/sbin/mount").output().unwrap(); + assert!( + !String::from_utf8_lossy(&mounts.stdout).contains(&format!(" on {mount} ")), + "native mount remains attached" + ); + assert!(paths::store_file(home, "shop").exists()); + assert!(paths::machine_record(home, "native").exists()); + let saved = call( + home, + "file.read", + &[ + ("project", t("shop/app")), + ("branch", t("work")), + ("path", t("/README.md")), + ], + ); + assert_eq!(saved.get_text("text"), Some("saved\n")); +} +#[test] +fn native_force_stops_live_daemon_and_command_and_detaches_mount_without_opening_store() { + let Ok(daemon) = std::env::var("LAYERFS_DAEMON") else { + eprintln!("NOT_RUN: explicit LAYERFS_DAEMON required for live native force proof"); + return; + }; + let home = std::env::temp_dir().join(format!("lfs-native-force-{}", std::process::id())); + home::open(&home).unwrap(); + let source = home.join("source"); + std::fs::create_dir(&source).unwrap(); + std::fs::write(source.join("README.md"), b"saved\n").unwrap(); + call(&home, "store.create", &[("name", t("shop"))]); + assert_eq!( + read_record(&paths::store_record(&home, "shop")) + .unwrap() + .unwrap() + .get_text("profile"), + Some("disposable") + ); + call( + &home, + "project.init", + &[ + ("project", t("shop/app")), + ("folder", t(&source.display().to_string())), + ], + ); + call( + &home, + "branch.fork", + &[ + ("project", t("shop/app")), + ("name", t("work")), + ("from_branch", t("main")), + ], + ); + home::import_env(&home, &[("LAYERFS_DAEMON".to_owned(), daemon)]).unwrap(); + live_case(&home, "live", true); + live_case(&home, "close", false); + std::fs::remove_dir_all(&home).unwrap(); +} diff --git a/api/crates/layerfs-cli/tests/capabilities.rs b/api/crates/layerfs-cli/tests/capabilities.rs index 40e91bc4b6..b1a598cce8 100644 --- a/api/crates/layerfs-cli/tests/capabilities.rs +++ b/api/crates/layerfs-cli/tests/capabilities.rs @@ -98,3 +98,36 @@ fn invalid_discovery_options_are_usage_errors() { } assert!(!home.exists()); } + +#[test] +fn explicit_force_options_are_discoverable_alongside_existing_integration_calls() { + let home = tempdir("force-capabilities").join("missing"); + let out = run(&mut layerfs(&home, &["help", "--json"])); + assert!(out.status.success(), "{}", text(&out.stderr)); + let value = json::parse(&text(&out.stdout)).unwrap(); + let Some(SdkValue::List(calls)) = value.get("calls") else { + panic!("calls list"); + }; + for (family, name) in [("machine", "stop"), ("sandbox", "close")] { + let call = calls + .iter() + .find(|call| { + call.get_text("family") == Some(family) && call.get_text("call") == Some(name) + }) + .unwrap(); + let Some(SdkValue::List(options)) = call.get("options") else { + panic!("call options"); + }; + let force = options + .iter() + .find(|option| option.get_text("name") == Some("force")) + .unwrap(); + assert_eq!(force.get_text("kind"), Some("bool")); + assert_eq!(force.get("optional"), Some(&SdkValue::Bool(true))); + } + assert!(calls + .iter() + .any(|call| call.get_text("family") == Some("project") + && call.get_text("call") == Some("export"))); + assert!(!home.exists()); +} diff --git a/api/crates/layerfs-cli/tests/common/stream_worker.rs b/api/crates/layerfs-cli/tests/common/stream_worker.rs index 4dd7614360..6552d3ad9e 100644 --- a/api/crates/layerfs-cli/tests/common/stream_worker.rs +++ b/api/crates/layerfs-cli/tests/common/stream_worker.rs @@ -73,7 +73,7 @@ fn create(more: &SdkValue) -> SdkValue { SdkValue::map([ ("sandbox", more.get("sandbox").cloned().unwrap_or_default()), ("name", t("lfs-x-t1")), - ("image", t("local")), + ("image", t(&format!("sha256:{}", "c".repeat(64)))), ("platform", t("docker")), ("runtime", t("namespaces")), ("route", t("fuse")), diff --git a/api/crates/layerfs-cli/tests/stream_framed.rs b/api/crates/layerfs-cli/tests/stream_framed.rs index e83396886a..fcc92851fb 100644 --- a/api/crates/layerfs-cli/tests/stream_framed.rs +++ b/api/crates/layerfs-cli/tests/stream_framed.rs @@ -24,7 +24,13 @@ fn result_is_final_even_for_command_exit_2_125_or_cancellation() { &home, &["workspace", "exec", "d1:one", command, "--events"], )); - assert_eq!(out.status.code(), Some(0), "{}", text(&out.stderr)); + assert_eq!( + out.status.code(), + Some(0), + "stdout={} stderr={}", + text(&out.stdout), + text(&out.stderr) + ); assert!(out.stderr.is_empty()); let frames = frames(&out.stdout); assert!(frames[0].get("progress").is_some()); @@ -52,7 +58,13 @@ fn output_over_one_mib_and_binary_bytes_are_preserved_in_order() { &home, &["workspace", "exec", "d1:one", "framed-big", "--events"], )); - assert_eq!(out.status.code(), Some(0), "{}", text(&out.stderr)); + assert_eq!( + out.status.code(), + Some(0), + "stdout={} stderr={}", + text(&out.stdout), + text(&out.stderr) + ); let frames = frames(&out.stdout); let bytes: Vec<_> = frames.iter().filter_map(|f| f.get("bytes")).collect(); assert_eq!(bytes.len(), 3); diff --git a/control/crates/layerfs-control/src/lib.rs b/control/crates/layerfs-control/src/lib.rs index 0e2377af70..a2cf1d05d7 100644 --- a/control/crates/layerfs-control/src/lib.rs +++ b/control/crates/layerfs-control/src/lib.rs @@ -12,6 +12,8 @@ pub mod doctor; pub mod inventory; #[cfg(feature = "machine")] pub mod machine; +#[cfg(feature = "machine")] +mod machine_force; pub mod migrate; pub mod project; pub mod store; diff --git a/control/crates/layerfs-control/src/machine.rs b/control/crates/layerfs-control/src/machine.rs index 63262fda42..2e0472ca1c 100644 --- a/control/crates/layerfs-control/src/machine.rs +++ b/control/crates/layerfs-control/src/machine.rs @@ -62,3 +62,8 @@ pub fn remove(home: &Path, name: &MachineName) -> Outcome { pub fn update(home: &Path, request: &MachineUpdate) -> Outcome { layerfs_machine::update(home, request, RELATED) } + +/// Explicit crash stop of a machine and this home's Sandboxes, preserving data. +pub fn stop_force(home: &Path, name: &MachineName) -> Outcome { + crate::machine_force::stop(home, name) +} diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs new file mode 100644 index 0000000000..78a0c32aa3 --- /dev/null +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -0,0 +1,260 @@ +//! Explicit provider crash stop without a worker request, VM start or data deletion. +use crate::inventory; +use layerfs_bridge::control::{SdkRefusal, SdkValue}; +use layerfs_home::home::{self, lock, paths}; +use layerfs_home::names::MachineName; +use layerfs_home::op::Op; +use layerfs_home::outcome::{failed, refused, Outcome}; +use layerfs_machine::platform::{self as adapters, Platform}; +use layerfs_machine::record::MachineRecord; +use layerfs_machine::{machine_refusal as refusal, machine_spec}; +use layerfs_sandbox::host::{Deploy, Host, Message}; +use layerfs_sandbox::record::SandboxRecord; +use layerfs_sandbox::{direct, files, reopen}; +use std::path::Path; +type Result = std::result::Result; + +pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { + let home = home.to_path_buf(); + let name = name.as_str(); + force_record(&home, name)?; + let _start = layerfs_machine::force_stop_lock(&home, name)?; + let intent = Op::begin( + "machine.stop", + "force", + SdkValue::map([("force", SdkValue::Bool(true))]), + )?; + let (spec, hosted, sandboxes) = { + let _home = lock::take(&home)?; + let machine = force_record(&home, name)?; + let hosted = inventory::hosted_on(&home, name)?; + let sandboxes = inventory::sandboxes(&home)? + .into_iter() + .filter(|sandbox| { + sandbox.machine() == name || hosted.iter().any(|child| sandbox.machine() == child) + }) + .collect::>(); + for sandbox in &sandboxes { + if sandbox + .op() + .is_some_and(|op| op.name() == "sandbox.create" && op.owner_alive()) + { + return Err(refused( + "Busy", + format!( + "Sandbox {} is still being created; no machine stop was attempted", + sandbox.name() + ), + )); + } + } + let spec = machine_spec(&home, &home_id(&home)?, &machine)?; + for child in &hosted { + force_record(&home, child)?; + } + for machine in std::iter::once(name).chain(hosted.iter().map(String::as_str)) { + update(&home, machine, |record| record.with_op(&intent))?; + } + (spec, hosted, sandboxes) + }; + let mut unknown = Vec::new(); + let mut stopped = Vec::new(); + let mut native_attempt = None; + let attempted = (|| -> Result<()> { + if spec.kind == Platform::Native { + for sandbox in &sandboxes { + native_attempt = Some(sandbox); + native_stop(&home, sandbox)?; + mark(&home, sandbox, true)?; + stopped.push(sandbox.name().to_owned()); + unknown.extend(workspaces(&home, sandbox)?); + } + } else { + adapters::stop_force(&spec).map_err(refusal)?; + for sandbox in &sandboxes { + mark(&home, sandbox, true)?; + stopped.push(sandbox.name().to_owned()); + let observed = workspaces(&home, sandbox)?; + if observed.is_empty() { + unknown.push(format!("{}:*", sandbox.name())); + } else { + unknown.extend(observed); + } + } + } + Ok(()) + })(); + if let Err(mut error) = attempted { + let uncertain = error.code == "Unknown" + || !stopped.is_empty() + || native_attempt.is_some() + || (spec.kind != Platform::Native && error.code != "Unsupported"); + { + let _home = lock::take(&home)?; + for machine in std::iter::once(name).chain(hosted.iter().map(String::as_str)) { + update(&home, machine, |record| { + if uncertain { + record.with_op(&intent.clone().as_uncertain()) + } else { + record.without_op() + } + })?; + } + } + // A multi-container provider stop may have completed only part of its work. + // Keep that uncertainty distinct from both a running or a confirmed gone runtime. + if spec.kind != Platform::Native && error.code != "Unsupported" { + for sandbox in &sandboxes { + if !stopped.iter().any(|name| name == sandbox.name()) { + mark(&home, sandbox, false)?; + } + } + } else if let Some(sandbox) = native_attempt { + if !stopped.iter().any(|name| name == sandbox.name()) { + mark(&home, sandbox, false)?; + } + } + error.detail = home::with_field( + error.detail, + "stopped_sandboxes", + SdkValue::List(stopped.into_iter().map(SdkValue::Text).collect()), + ); + error.detail = home::with_field( + error.detail, + "unknown", + SdkValue::List(unknown.into_iter().map(SdkValue::Text).collect()), + ); + return Err(error); + } + { + let _home = lock::take(&home)?; + for machine in std::iter::once(name).chain(hosted.iter().map(String::as_str)) { + match std::fs::remove_file(paths::worker_record(&home, machine)) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(failed(error)), + } + update(&home, machine, |record| { + record.without_op().with( + "state", + SdkValue::text(if machine == "native" { + "ready" + } else { + "stopped" + }), + ) + })?; + } + } + // Do not call get(): a live probe could contact the worker we just stopped. + let mut out = layerfs_machine::stopped_summary(&home, name, crate::machine::RELATED)?; + for (key, value) in [ + ("forced", SdkValue::Bool(true)), + ( + "stopped_sandboxes", + SdkValue::List(stopped.into_iter().map(SdkValue::Text).collect()), + ), + ( + "unknown", + SdkValue::List(unknown.into_iter().map(SdkValue::Text).collect()), + ), + ] { + out = home::with_field(out, key, value); + } + Ok(out) +} + +/// A new explicit force call may reconcile its own previous unknown stop; +/// it never silently retries, nor clears another kind of unresolved operation. +fn force_record(home: &Path, name: &str) -> Result { + let record = MachineRecord::of(home, name)?; + if record + .op() + .is_some_and(|op| op.name() == "machine.stop" && op.uncertain()) + { + return Ok(record); + } + record.not_held()?; + Ok(record) +} + +/// Record an observed terminal fact, retaining runtime files and exact identity. +fn mark(home: &Path, sandbox: &SandboxRecord, stopped: bool) -> Result<()> { + let _home = lock::take(home)?; + let _sandbox = layerfs_sandbox::record::lock(home, sandbox.name())?; + if let Some(now) = inventory::sandbox(home, sandbox.name())? { + if now.to_record().get("runtime_id") != sandbox.to_record().get("runtime_id") { + return Err(refused( + "Unknown", + "the Sandbox runtime identity changed during the stop; its new record was retained", + )); + } + now.with_machine_stop(stopped).write(home)?; + } + if stopped { + direct::release(home, sandbox); + } + Ok(()) +} + +/// Native is the host itself: stop only this home's recorded runtime processes. +fn native_stop(home: &Path, sandbox: &SandboxRecord) -> Result<()> { + if sandbox.machine_stopped() { + return Ok(()); + } + let host = Host { + home: home.to_owned(), + store: String::new(), + project: String::new(), + sandbox: Some(sandbox.name().to_owned()), + store_record: None, + store_path: None, + deploy: Some(Deploy { + local_dir: Some(paths::run_root(home).display().to_string()), + container_prefix: Some(format!("lfs-{}-", &home_id(home)?[..8])), + ..Deploy::default() + }), + postgresql: None, + message: Message::Absent, + started: None, + }; + let Some(daemon) = files::started(&sandbox.dir(home))? else { + if sandbox.id().is_some() { + return Err(refused( + "Unavailable", + format!( + "Sandbox {} has no recorded native runtime; its stop cannot be confirmed", + sandbox.name() + ), + )); + } + return sandbox.free(); + }; + reopen::recorded(&host, sandbox.name(), &daemon)?.stop_now() +} + +/// These are lost/unknown uncommitted frontiers, never manufactured commits. +fn workspaces(home: &Path, sandbox: &SandboxRecord) -> Result> { + let mut workspaces = Vec::new(); + for (path, record) in files::workspaces(&sandbox.dir(home))? { + let fallback = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("unknown"); + let value = record.name().or_else(|| record.path()).unwrap_or(fallback); + workspaces.push(format!("{}:{value}", sandbox.name())); + } + Ok(workspaces) +} + +fn home_id(home: &Path) -> Result { + Ok(home::opened(home)?.id) +} + +fn update( + home: &Path, + name: &str, + change: impl FnOnce(MachineRecord) -> MachineRecord, +) -> Result<()> { + change(MachineRecord::of(home, name)?).write(home) +} diff --git a/control/crates/layerfs-machine/src/endpoint.rs b/control/crates/layerfs-machine/src/endpoint.rs index ed71b648ad..7f30823955 100644 --- a/control/crates/layerfs-machine/src/endpoint.rs +++ b/control/crates/layerfs-machine/src/endpoint.rs @@ -99,6 +99,7 @@ fn start( /// A wait for it, at most [`START_WAIT`], is not a retry. pub type StartLock = layerfs_home::home::entity_lock::EntityLock; +/// Take the machine lifecycle lock before a start, create, remove or forced stop. pub fn start_lock(home: &Path, name: &str) -> Result { let path = layerfs_home::home::paths::machine_dir(home, name).join("start.lock"); let held = format!("machine {name} was being started, made or removed by another process"); diff --git a/control/crates/layerfs-machine/src/lib.rs b/control/crates/layerfs-machine/src/lib.rs index b01dd35222..9ce32cdf03 100644 --- a/control/crates/layerfs-machine/src/lib.rs +++ b/control/crates/layerfs-machine/src/lib.rs @@ -76,3 +76,10 @@ pub use spec::{ pub use status::status; #[cfg(feature = "machine")] pub use update::{update, MachineUpdate}; + +/// The machine lifecycle lock, shared by the spanning forced-stop call. +#[cfg(feature = "machine")] +pub use endpoint::start_lock as force_stop_lock; +/// A confirmed stopped machine summary without a worker or provider probe. +#[cfg(feature = "machine")] +pub use power::stopped_summary; diff --git a/control/crates/layerfs-machine/src/power.rs b/control/crates/layerfs-machine/src/power.rs index abc612099d..088d6b614c 100644 --- a/control/crates/layerfs-machine/src/power.rs +++ b/control/crates/layerfs-machine/src/power.rs @@ -68,3 +68,17 @@ pub fn stop(home: &Path, name: &MachineName, related: Related) -> Outcome { } super::get::get(&home, name, related) } + +/// A provider-confirmed terminal state, without contacting its stopped worker. +pub fn stopped_summary(home: &Path, name: &str, related: Related) -> Outcome { + let machine = super::record::MachineRecord::of(home, name)?; + super::list::summary( + home, + &machine, + Some(( + if name == "native" { "ready" } else { "stopped" }.to_owned(), + None, + )), + related, + ) +} diff --git a/control/crates/layerfs-platform/src/adapter.rs b/control/crates/layerfs-platform/src/adapter.rs index bb24fabf36..5ea1a8104c 100644 --- a/control/crates/layerfs-platform/src/adapter.rs +++ b/control/crates/layerfs-platform/src/adapter.rs @@ -24,7 +24,7 @@ pub(crate) trait Adapter: Sync { /// Starts a stopped machine or its worker; `None`: nothing to start. fn start(&self, spec: &Spec, bundle: &Bundle) -> Result, MachineError>; /// Stops the worker and, where there is one, the VM. - fn stop(&self, spec: &Spec) -> Result<(), MachineError>; + fn stop(&self, spec: &Spec, force: bool) -> Result<(), MachineError>; /// Provider runtime state only, without worker contact or guest commands. fn provider_state(&self, spec: &Spec) -> Result; /// The machine's state now; starts nothing. diff --git a/control/crates/layerfs-platform/src/lib.rs b/control/crates/layerfs-platform/src/lib.rs index 1053cceaaa..ee51b4a1c9 100644 --- a/control/crates/layerfs-platform/src/lib.rs +++ b/control/crates/layerfs-platform/src/lib.rs @@ -31,7 +31,7 @@ pub use platform::{ExecutorKind, HostOs, How, KernelRoute, Platform, Profile, Ro pub use probe::{native_missing, probe, Probe}; pub use setup::{ cached_image, copy_in, copy_out, create, forward, forward_close, load_image, provider_within, - reach, remove, start, status, status_within, stop, State, Status, + reach, remove, start, status, status_within, stop, stop_force, State, Status, }; pub use spec::{ parse_size, Bundle, Endpoint, Forward, Lasting, Running, Spec, Tools, BUNDLE, DATA, IMAGE, diff --git a/control/crates/layerfs-platform/src/platforms/apple_container/adapter.rs b/control/crates/layerfs-platform/src/platforms/apple_container/adapter.rs index 8bcc1bb770..f39baa590b 100644 --- a/control/crates/layerfs-platform/src/platforms/apple_container/adapter.rs +++ b/control/crates/layerfs-platform/src/platforms/apple_container/adapter.rs @@ -39,8 +39,8 @@ impl Adapter for AppleContainer { fn start(&self, spec: &Spec, bundle: &Bundle) -> Result, MachineError> { start(spec, bundle).map(Some) } - fn stop(&self, spec: &Spec) -> Result<(), MachineError> { - stop(spec) + fn stop(&self, spec: &Spec, force: bool) -> Result<(), MachineError> { + stop(spec, force) } fn provider_state(&self, spec: &Spec) -> Result { super::observe::observe(spec) diff --git a/control/crates/layerfs-platform/src/platforms/apple_container/power.rs b/control/crates/layerfs-platform/src/platforms/apple_container/power.rs index 8406632f03..546f587080 100644 --- a/control/crates/layerfs-platform/src/platforms/apple_container/power.rs +++ b/control/crates/layerfs-platform/src/platforms/apple_container/power.rs @@ -18,8 +18,9 @@ pub(crate) fn start(spec: &Spec, bundle: &Bundle) -> Result Result<(), MachineError> { - if observe(spec)? == State::Running { +pub(crate) fn stop(spec: &Spec, force: bool) -> Result<(), MachineError> { + let state = observe(spec)?; + if state == State::Running || (force && state == State::Failed) { checked(spec, CLI, &["stop", &spec.vm_name], CREATE)?; } Ok(()) diff --git a/control/crates/layerfs-platform/src/platforms/docker/adapter.rs b/control/crates/layerfs-platform/src/platforms/docker/adapter.rs index 99c49124ad..25145485ca 100644 --- a/control/crates/layerfs-platform/src/platforms/docker/adapter.rs +++ b/control/crates/layerfs-platform/src/platforms/docker/adapter.rs @@ -55,8 +55,8 @@ impl Adapter for Docker { fn start(&self, spec: &Spec, bundle: &Bundle) -> Result, MachineError> { start(spec, bundle).map(Some) } - fn stop(&self, spec: &Spec) -> Result<(), MachineError> { - stop(spec) + fn stop(&self, spec: &Spec, force: bool) -> Result<(), MachineError> { + stop(spec, force) } fn provider_state(&self, spec: &Spec) -> Result { super::provider_state(spec) diff --git a/control/crates/layerfs-platform/src/platforms/docker/power.rs b/control/crates/layerfs-platform/src/platforms/docker/power.rs index 493cc56c41..b834d39cf0 100644 --- a/control/crates/layerfs-platform/src/platforms/docker/power.rs +++ b/control/crates/layerfs-platform/src/platforms/docker/power.rs @@ -123,13 +123,22 @@ pub(crate) fn start(spec: &Spec, bundle: &Bundle) -> Result Result<(), MachineError> { - if inspect(spec)?.is_none() { +pub(crate) fn stop(spec: &Spec, force: bool) -> Result<(), MachineError> { + let Some((state, role)) = inspect(spec)? else { + return Ok(()); + }; + if role != "machine-worker" { + return Err(MachineError::new( + Code::InvalidInput, + "the recorded container is not a LayerFS worker; it is left alone", + )); + } + if !matches!(state.as_str(), "running" | "restarting" | "paused") { return Ok(()); } run( spec, - &["stop", "-t", "5", &spec.vm_name], + &["stop", "-t", if force { "0" } else { "5" }, &spec.vm_name], "stopping the worker container", )? .answer("stopping the worker container")?; diff --git a/control/crates/layerfs-platform/src/platforms/docker/windows.rs b/control/crates/layerfs-platform/src/platforms/docker/windows.rs index df206e7d25..eb7bf93767 100644 --- a/control/crates/layerfs-platform/src/platforms/docker/windows.rs +++ b/control/crates/layerfs-platform/src/platforms/docker/windows.rs @@ -23,7 +23,7 @@ pub(crate) fn guest(spec: &Spec) -> Result { Err(unsupported(spec)) } -pub(crate) fn stop(spec: &Spec) -> Result<(), MachineError> { +pub(crate) fn stop(spec: &Spec, _force: bool) -> Result<(), MachineError> { Err(unsupported(spec)) } diff --git a/control/crates/layerfs-platform/src/platforms/lima/adapter.rs b/control/crates/layerfs-platform/src/platforms/lima/adapter.rs index 0efdc5b13c..8bc121835e 100644 --- a/control/crates/layerfs-platform/src/platforms/lima/adapter.rs +++ b/control/crates/layerfs-platform/src/platforms/lima/adapter.rs @@ -36,8 +36,8 @@ impl Adapter for Lima { fn start(&self, spec: &Spec, bundle: &Bundle) -> Result, MachineError> { start(spec, bundle).map(Some) } - fn stop(&self, spec: &Spec) -> Result<(), MachineError> { - stop(spec) + fn stop(&self, spec: &Spec, force: bool) -> Result<(), MachineError> { + stop(spec, force) } fn provider_state(&self, spec: &Spec) -> Result { super::observe::observe(spec).map(|(state, _)| state) diff --git a/control/crates/layerfs-platform/src/platforms/lima/power.rs b/control/crates/layerfs-platform/src/platforms/lima/power.rs index 9e08085326..a5668f3cba 100644 --- a/control/crates/layerfs-platform/src/platforms/lima/power.rs +++ b/control/crates/layerfs-platform/src/platforms/lima/power.rs @@ -20,9 +20,15 @@ pub(crate) fn start(spec: &Spec, bundle: &Bundle) -> Result Result<(), MachineError> { - if observe(spec)?.0 == State::Running { - checked(spec, CLI, &["stop", &spec.vm_name], BOOT)?; +pub(crate) fn stop(spec: &Spec, force: bool) -> Result<(), MachineError> { + let state = observe(spec)?.0; + if state == State::Running || (force && state == State::Failed) { + let mut words = vec!["stop"]; + if force { + words.push("--force"); + } + words.push(&spec.vm_name); + checked(spec, CLI, &words, BOOT)?; } Ok(()) } diff --git a/control/crates/layerfs-platform/src/platforms/native/adapter.rs b/control/crates/layerfs-platform/src/platforms/native/adapter.rs index fd6660b0b5..70bbfa9c80 100644 --- a/control/crates/layerfs-platform/src/platforms/native/adapter.rs +++ b/control/crates/layerfs-platform/src/platforms/native/adapter.rs @@ -30,7 +30,7 @@ impl Adapter for Native { fn start(&self, spec: &Spec, _bundle: &Bundle) -> Result, MachineError> { start(spec) } - fn stop(&self, _spec: &Spec) -> Result<(), MachineError> { + fn stop(&self, _spec: &Spec, _force: bool) -> Result<(), MachineError> { Ok(()) } fn provider_state(&self, _spec: &Spec) -> Result { diff --git a/control/crates/layerfs-platform/src/setup/calls.rs b/control/crates/layerfs-platform/src/setup/calls.rs index 8fb2e21e1d..e8896768a2 100644 --- a/control/crates/layerfs-platform/src/setup/calls.rs +++ b/control/crates/layerfs-platform/src/setup/calls.rs @@ -70,7 +70,14 @@ pub fn start(spec: &Spec, bundle: &Bundle) -> Result, MachineErr /// `docker` stops only its worker; `native` has nothing to stop. pub fn stop(spec: &Spec) -> Result<(), MachineError> { spec.validate()?; - adapter(spec.kind).stop(spec) + adapter(spec.kind).stop(spec, false) +} + +/// Stops directly through the provider, without contacting or starting its worker. +/// The caller owns Sandbox/Workspace reconciliation; registered machines and disks stay. +pub fn stop_force(spec: &Spec) -> Result<(), MachineError> { + spec.validate()?; + adapter(spec.kind).stop(spec, true) } /// `machine remove` deletes a VM and its data disk. `InUse` is the diff --git a/control/crates/layerfs-platform/src/setup/mod.rs b/control/crates/layerfs-platform/src/setup/mod.rs index 9f23307f64..4b3b0d3735 100644 --- a/control/crates/layerfs-platform/src/setup/mod.rs +++ b/control/crates/layerfs-platform/src/setup/mod.rs @@ -10,5 +10,5 @@ pub(crate) mod worker; pub(crate) use calls::with_worker; pub use calls::{ cached_image, copy_in, copy_out, create, forward, forward_close, load_image, provider_within, - reach, remove, start, status, status_within, stop, State, Status, + reach, remove, start, status, status_within, stop, stop_force, State, Status, }; diff --git a/control/crates/layerfs-platform/tests/machine_force.rs b/control/crates/layerfs-platform/tests/machine_force.rs new file mode 100644 index 0000000000..82933c6454 --- /dev/null +++ b/control/crates/layerfs-platform/tests/machine_force.rs @@ -0,0 +1,126 @@ +//! Force stop uses only provider power operations, retaining machines and disks. +#![cfg(unix)] +#[path = "support/standin.rs"] +mod standin; +use layerfs_platform::{self as machine, Code, Platform}; +use standin::Scratch; + +#[test] +fn native_force_stop_never_stops_the_host() { + let scratch = Scratch::new("force-native"); + machine::stop_force(&scratch.spec(Platform::Native)).unwrap(); + assert!(scratch.log().is_empty()); +} + +#[test] +fn docker_force_stops_only_the_owned_worker_with_zero_grace() { + for state in ["running", "paused", "restarting"] { + let scratch = Scratch::new("force-docker"); + scratch.tool("docker", &format!( + "case \"$3\" in inspect) printf '{state}\\tmachine-worker\\n';; stop) exit 0;; *) exit 99;; esac" + )); + machine::stop_force(&scratch.spec(Platform::Docker)).unwrap(); + let log = scratch.log(); + let calls: Vec<_> = log.lines().collect(); + assert_eq!(calls.len(), 2, "{log}"); + assert!(calls[0].contains(" inspect -f "), "{log}"); + assert_eq!( + calls[1], + "docker -H unix:///var/run/docker.sock stop -t 0 lfs-dev" + ); + } +} + +#[test] +fn docker_force_preserves_unowned_containers_and_reports_inspect_failure() { + for (answer, code) in [ + ("printf 'running\\tother-role\\n'", Code::InvalidInput), + ("echo 'engine unavailable' >&2; exit 1", Code::Failed), + ] { + let scratch = Scratch::new("force-docker-refuse"); + scratch.tool("docker", answer); + let error = machine::stop_force(&scratch.spec(Platform::Docker)).unwrap_err(); + assert_eq!(error.code, code, "{error}"); + assert_eq!(scratch.log().lines().count(), 1); + } +} + +#[test] +fn docker_missing_or_stopped_worker_does_not_boot_or_remove_anything() { + for answer in [ + "echo 'No such object: lfs-dev' >&2; exit 1", + "printf 'exited\\tmachine-worker\\n'", + ] { + let scratch = Scratch::new("force-docker-idle"); + scratch.tool("docker", answer); + machine::stop_force(&scratch.spec(Platform::Docker)).unwrap(); + assert_eq!(scratch.log().lines().count(), 1); + } +} + +#[test] +fn ordinary_docker_stop_retains_grace_period() { + let scratch = Scratch::new("normal-docker-stop"); + scratch.tool("docker", "case \"$3\" in inspect) printf 'running\\tmachine-worker\\n';; stop) exit 0;; *) exit 99;; esac"); + machine::stop(&scratch.spec(Platform::Docker)).unwrap(); + assert!(scratch.log().ends_with(" stop -t 5 lfs-dev\n")); +} + +#[test] +fn lima_force_stops_running_and_broken_instances_without_guest_calls() { + for state in ["Running", "Broken"] { + let scratch = Scratch::new("force-lima"); + scratch.tool( + "limactl", + &format!( + "case \"$1\" in list) printf '{state}\\t/unused\\n';; stop) exit 0;; *) exit 99;; esac" + ), + ); + machine::stop_force(&scratch.spec(Platform::Lima)).unwrap(); + let log = scratch.log(); + assert_eq!(log.lines().count(), 2, "{log}"); + assert!(log.ends_with("limactl stop --force lfs-dev\n"), "{log}"); + } +} + +#[test] +fn lima_idle_force_and_normal_stop_do_not_change_disks() { + for state in ["Stopped", ""] { + let scratch = Scratch::new("force-lima-idle"); + scratch.tool("limactl", &format!("printf '{state}\\n'")); + machine::stop_force(&scratch.spec(Platform::Lima)).unwrap(); + assert_eq!(scratch.log().lines().count(), 1); + } + let scratch = Scratch::new("normal-lima-stop"); + scratch.tool( + "limactl", + "case \"$1\" in list) printf 'Running\\t/unused\\n';; stop) exit 0;; *) exit 99;; esac", + ); + machine::stop(&scratch.spec(Platform::Lima)).unwrap(); + assert!(scratch.log().ends_with("limactl stop lfs-dev\n")); +} + +#[test] +fn apple_force_stops_running_or_failed_container_without_guest_calls() { + for state in ["running", "failed"] { + let scratch = Scratch::new("force-apple"); + scratch.tool("container", &format!( + "case \"$1\" in inspect) echo '[{{\"status\":{{\"state\":\"{state}\"}}}}]';; stop) exit 0;; *) exit 99;; esac" + )); + machine::stop_force(&scratch.spec(Platform::AppleContainer)).unwrap(); + assert_eq!( + scratch.log(), + "container inspect lfs-dev\ncontainer stop lfs-dev\n" + ); + } +} + +#[test] +fn apple_missing_or_stopped_container_does_not_start_or_delete() { + for answer in ["[]", "[{\"status\":{\"state\":\"stopped\"}}]"] { + let scratch = Scratch::new("force-apple-idle"); + scratch.tool("container", &format!("echo '{answer}'")); + machine::stop_force(&scratch.spec(Platform::AppleContainer)).unwrap(); + assert_eq!(scratch.log(), "container inspect lfs-dev\n"); + } +} diff --git a/control/crates/layerfs-sandbox-backend/src/backend/host_process/sandbox.rs b/control/crates/layerfs-sandbox-backend/src/backend/host_process/sandbox.rs index db5723fcdc..49ed80ea75 100644 --- a/control/crates/layerfs-sandbox-backend/src/backend/host_process/sandbox.rs +++ b/control/crates/layerfs-sandbox-backend/src/backend/host_process/sandbox.rs @@ -84,6 +84,31 @@ impl Sandbox { owner.endpoint()?; Ok(owner) } + /// Reopen only for an explicit crash stop, including a daemon already gone. + /// No endpoint or daemon RPC is needed. Stop still matches pid, start and boot. + pub fn reopen_for_stop( + directory: PathBuf, + id: ContainerId, + process: LocalProcess, + port: u16, + labels: Vec<(String, String)>, + ) -> Result> { + let mut owner = Self::unstarted(id, directory, labels); + owner.process = Some(process); + owner.port = port; + if !owner.directory.is_absolute() || port == 0 { + return Err(owner.invalid("saved local Sandbox")); + } + if process.boot.is_none() { + return Err(owner.invalid("saved local daemon has no boot identity; mounts retained")); + } + if let Ok(current) = LocalProcess::observe(process.pid) { + if current.alive() && !process.alive() { + return Err(owner.invalid("saved local daemon identity changed; mounts retained")); + } + } + Ok(owner) + } /// A recorded Sandbox that [`Self::reopen`] found stopped /// ([`WireFailure::container_stopped`]): only its one Delete is admitted. pub fn stopped( diff --git a/control/crates/layerfs-sandbox-backend/tests/local_stop_mount.rs b/control/crates/layerfs-sandbox-backend/tests/local_stop_mount.rs index 26a5a6b6e9..f086a5fa1c 100644 --- a/control/crates/layerfs-sandbox-backend/tests/local_stop_mount.rs +++ b/control/crates/layerfs-sandbox-backend/tests/local_stop_mount.rs @@ -6,6 +6,32 @@ use layerfs_bridge::process::LocalProcess; use layerfs_sandbox_backend::{host_process::Sandbox, ContainerId}; use std::{path::PathBuf, process::Command}; +#[test] +fn force_reopen_refuses_unconfirmed_process_identity_before_stop() { + let process = LocalProcess::observe(std::process::id()).unwrap(); + for unconfirmed in [ + LocalProcess { + boot: None, + ..process + }, + LocalProcess { + start: process.start + 1, + ..process + }, + ] { + let result = Sandbox::reopen_for_stop( + std::env::temp_dir().join("layerfs-force-unconfirmed-owner"), + ContainerId::parse(&"1".repeat(64)).unwrap(), + unconfirmed, + 12345, + Vec::new(), + ); + let error = result.unwrap_err(); + assert!(format!("{error:?}").contains("mounts retained")); + assert!(process.alive()); + } +} + #[test] #[ignore = "requires an explicitly supplied disposable mounted Sandbox"] fn stop_detaches_mounts_before_returning_without_delete() { diff --git a/control/crates/layerfs-sandbox/src/direct.rs b/control/crates/layerfs-sandbox/src/direct.rs index f6188b3306..82d46591e7 100644 --- a/control/crates/layerfs-sandbox/src/direct.rs +++ b/control/crates/layerfs-sandbox/src/direct.rs @@ -302,6 +302,9 @@ pub fn capacity(home: &Path, record: &SandboxRecord) -> Outcome { /// Store's lease (the Sandbox half of `workspace::direct::call`). fn served(home: &Path, record: &SandboxRecord, call: Call) -> Outcome { let home = home.to_path_buf(); + if let Some(machine) = layerfs_machine::record::MachineRecord::read(&home, record.machine())? { + machine.not_held()?; + } let target = resolve::target(&home, record.project().unwrap_or(""))?; let _lease = lease::hold(&home, &target.store)?; dispatch::call(host(&home, record, &target, Message::Absent), call) diff --git a/control/crates/layerfs-sandbox/src/get.rs b/control/crates/layerfs-sandbox/src/get.rs index cb8485bd77..81fe910f7f 100644 --- a/control/crates/layerfs-sandbox/src/get.rs +++ b/control/crates/layerfs-sandbox/src/get.rs @@ -59,6 +59,7 @@ pub fn get(home: &Path, events: &mut Events<'_>, request: &GetRequest) -> Result // (P4): a machine that is down answers the record with no Workspaces // and why let listed = match record.id() { + _ if record.machine_stopped() => Ok(Ok(SdkValue::Null)), // one reached directly lists its daemon's Workspaces from here (#535 G4) Some(_) if direct::forwarded(&record) => direct::list(&home, &record), Some(_) => reach( diff --git a/control/crates/layerfs-sandbox/src/record/sandbox.rs b/control/crates/layerfs-sandbox/src/record/sandbox.rs index 1da883f82c..49377d3396 100644 --- a/control/crates/layerfs-sandbox/src/record/sandbox.rs +++ b/control/crates/layerfs-sandbox/src/record/sandbox.rs @@ -62,6 +62,8 @@ enum Field { RuntimeId(String), Daemon(DaemonRecord), Left(String), + MachineStopped(bool), + MachineStopUnknown(bool), } impl fields::Field for Field { @@ -83,6 +85,14 @@ impl fields::Field for Field { "runtime_id" => Self::RuntimeId(text()?), "daemon" => Self::Daemon(DaemonRecord::decode(value)?), "left" => Self::Left(text()?), + "machine_stopped" => Self::MachineStopped(match value { + SdkValue::Bool(v) => *v, + _ => return None, + }), + "machine_stop_unknown" => Self::MachineStopUnknown(match value { + SdkValue::Bool(v) => *v, + _ => return None, + }), _ => return None, }) } @@ -104,6 +114,8 @@ impl fields::Field for Field { Self::RuntimeId(_) => "runtime_id", Self::Daemon(_) => "daemon", Self::Left(_) => "left", + Self::MachineStopped(_) => "machine_stopped", + Self::MachineStopUnknown(_) => "machine_stop_unknown", } } @@ -112,6 +124,7 @@ impl fields::Field for Field { Self::V(n) | Self::Created(n) => SdkValue::Int(*n), Self::Op(op) => op.to_record().clone(), Self::Daemon(daemon) => daemon.to_record(), + Self::MachineStopped(v) | Self::MachineStopUnknown(v) => SdkValue::Bool(*v), Self::Name(text) | Self::Project(text) | Self::Machine(text) @@ -382,6 +395,23 @@ impl SandboxRecord { })) } + /// Whether a provider-confirmed crash stop ended this exact runtime. + pub fn machine_stopped(&self) -> bool { + self.fields + .get(|f| match f { + Field::MachineStopped(v) => Some(*v), + _ => None, + }) + .unwrap_or(false) + } + + /// Record a confirmed stop, or an uncertain attempted stop, symmetrically. + pub fn with_machine_stop(mut self, stopped: bool) -> Self { + self.fields.set(Field::MachineStopped(stopped)); + self.fields.set(Field::MachineStopUnknown(!stopped)); + self + } + /// The Sandbox's state (#536 ยง7.2), from its record, its `op` and, /// where the call asked, the daemon's answer (`gone`: its machine /// answered that no such Sandbox runs): `creating` or `closing` while @@ -392,6 +422,19 @@ impl SandboxRecord { /// (compute-plane 2.0 P-1: it ended behind control's back; its /// `sandbox.close` clears it), else `running`. pub fn state(&self, gone: bool) -> &'static str { + if self.machine_stopped() { + return "gone"; + } + if self + .fields + .get(|f| match f { + Field::MachineStopUnknown(v) => Some(*v), + _ => None, + }) + .unwrap_or(false) + { + return "unresolved"; + } let op = self.op(); match op.as_ref().map(|op| (op.standing(), op.name())) { Some((Standing::Running, "sandbox.close")) => "closing", diff --git a/control/crates/layerfs-sandbox/src/reopen.rs b/control/crates/layerfs-sandbox/src/reopen.rs index 6add67b3ee..405b209193 100644 --- a/control/crates/layerfs-sandbox/src/reopen.rs +++ b/control/crates/layerfs-sandbox/src/reopen.rs @@ -283,6 +283,29 @@ fn local_record(_: &Host, id: &str, _: &DaemonRecord) -> Result { )) } impl Recorded { + /// Stop an exact recorded runtime without endpoint admission or daemon RPC. + pub fn stop_now(self) -> Result<()> { + let mut owner = match self { + #[cfg(any(target_os = "linux", target_os = "macos"))] + Self::Local { + directory, + id, + process, + port, + labels, + } => Box::new(Sandbox::HostProcess( + host_process::Sandbox::reopen_for_stop(directory, id, process, port, labels) + .map_err(failed)?, + )), + #[cfg(target_os = "linux")] + Self::Namespaces(runtime, id, record) => match runtime.reopen(id, &record) { + Ok(owner) => Box::new(owner), + Err(error) if error.container_stopped() => return Ok(()), + Err(error) => return Err(failed(error)), + }, + }; + owner.stop(0).map_err(failed) + } /// The runtime, observed running once (it starts nothing). pub fn reopen(self) -> std::result::Result, Box> { match self { diff --git a/control/crates/layerfs-sandbox/src/route.rs b/control/crates/layerfs-sandbox/src/route.rs index b560d7c0e0..f38e7c8811 100644 --- a/control/crates/layerfs-sandbox/src/route.rs +++ b/control/crates/layerfs-sandbox/src/route.rs @@ -248,6 +248,9 @@ fn host_op( ) -> Outcome { let home = home.to_path_buf(); let machine = record.machine(); + if let Some(machine) = layerfs_machine::record::MachineRecord::read(&home, machine)? { + machine.not_held()?; + } let project = record.project().unwrap_or(""); let target = resolve::target(&home, project)?; let Asked { diff --git a/control/crates/layerfs-sandbox/src/workspace/direct.rs b/control/crates/layerfs-sandbox/src/workspace/direct.rs index 705b631ecf..5ad11881f7 100644 --- a/control/crates/layerfs-sandbox/src/workspace/direct.rs +++ b/control/crates/layerfs-sandbox/src/workspace/direct.rs @@ -31,6 +31,9 @@ pub fn call( message: Message, ) -> Outcome { let home = home.to_path_buf(); + if let Some(machine) = layerfs_machine::record::MachineRecord::read(&home, record.machine())? { + machine.not_held()?; + } let target = resolve::target(&home, record.project().unwrap_or(""))?; let _lease = lease::hold(&home, &target.store)?; let host = host(&home, record, &target, message); diff --git a/control/crates/layerfs-store/src/lease.rs b/control/crates/layerfs-store/src/lease.rs index b631e722f5..5d08656335 100644 --- a/control/crates/layerfs-store/src/lease.rs +++ b/control/crates/layerfs-store/src/lease.rs @@ -149,7 +149,28 @@ pub fn holders(home: &Path, store: &str) -> Result, SdkRefusal> { live.push(pid); continue; } - std::fs::remove_file(path).map_err(failed)?; + match std::fs::remove_file(&path) { + Ok(()) => {} + // Its holder can drop the lease after the snapshot above. It is + // already released; no removal happened here to log. + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => { + let mut refusal = failed(&error); + refusal.detail = SdkValue::map([ + ("operation", SdkValue::text("remove_stale_lease")), + ("store", SdkValue::text(store)), + ("path", SdkValue::text(path.display().to_string())), + ( + "os_error", + error + .raw_os_error() + .map(|code| SdkValue::Int(i64::from(code))) + .unwrap_or_default(), + ), + ]); + return Err(refusal); + } + } home::log_line( home, "leases.jsonl", diff --git a/control/crates/layerfs-store/src/place.rs b/control/crates/layerfs-store/src/place.rs index a15c954d5c..b1e9c9994d 100644 --- a/control/crates/layerfs-store/src/place.rs +++ b/control/crates/layerfs-store/src/place.rs @@ -47,6 +47,9 @@ pub fn plan(home: &Path, store: &StoreRef) -> Result { /// Where a call on machine `machine` runs: its Sandboxes' host operations, /// and the Store calls of the sqlite Stores on it. pub fn on_machine(home: &Path, machine: &str) -> Result { + if let Some(record) = layerfs_machine::record::MachineRecord::read(home, machine)? { + record.not_held()?; + } if layerfs_machine::runs_here(home, machine)? { return Ok(Executor::Local); } diff --git a/control/crates/layerfs-store/tests/lease_race.rs b/control/crates/layerfs-store/tests/lease_race.rs new file mode 100644 index 0000000000..7a3a38bc07 --- /dev/null +++ b/control/crates/layerfs-store/tests/lease_race.rs @@ -0,0 +1,56 @@ +//! A holder can release its lease after stale inspection and before removal. +#![cfg(unix)] +use layerfs_home::home::{self, paths}; +use layerfs_store::{lease, record}; +use nix::sys::stat::Mode; +use std::fs::OpenOptions; +use std::io::Write; +use std::os::unix::fs::OpenOptionsExt; +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +#[test] +fn stale_lease_released_after_inspection_is_already_gone() { + let home = std::env::temp_dir().join(format!("layerfs-lease-race-{}", std::process::id())); + let directory = paths::lease_dir(&home, "shop"); + home::private_dir(&directory).unwrap(); + let path = directory.join("4294967295"); + nix::unistd::mkfifo(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); + let (send, recv) = mpsc::channel(); + let root = home.clone(); + let reader = std::thread::spawn(move || { + let _lock = record::lock(&root, "shop").unwrap(); + send.send(lease::holders(&root, "shop")).unwrap(); + }); + // Opening the FIFO for writing succeeds only after holders opened its read + // descriptor. Unlink before supplying EOF: its snapshot then names a lease + // already released by its holder, without timing guesses or product hooks. + let until = Instant::now() + Duration::from_secs(5); + let mut writer = loop { + match OpenOptions::new() + .write(true) + .custom_flags(libc::O_NONBLOCK) + .open(&path) + { + Ok(file) => break file, + Err(error) if error.raw_os_error() == Some(libc::ENXIO) => { + assert!(Instant::now() < until, "lease inspection never opened FIFO"); + std::thread::sleep(Duration::from_millis(5)); + } + Err(error) => panic!("open lease FIFO: {error}"), + } + }; + std::fs::remove_file(&path).unwrap(); + writer + .write_all(b"0 0 00000000000000000000000000000000\n") + .unwrap(); + drop(writer); + let result = recv.recv_timeout(Duration::from_secs(5)).unwrap(); + reader.join().unwrap(); + assert!(result.unwrap().is_empty()); + assert!( + !home.join("log/leases.jsonl").exists(), + "no removal was performed to log" + ); + std::fs::remove_dir_all(home).unwrap(); +} diff --git a/docs/architecture/api/sdk/machine/README.md b/docs/architecture/api/sdk/machine/README.md index 354fb0bb3c..44b646c6a0 100644 --- a/docs/architecture/api/sdk/machine/README.md +++ b/docs/architecture/api/sdk/machine/README.md @@ -48,7 +48,7 @@ state, cpus, memory, disk, stores, sandboxes, โ€ฆ }` | [`get`](get.md) | `name` (Name) | โ€” | summary | | [`status`](status.md) | `name` | โ€” | `{ name, platform, state, free_disk, tool_version, down_reason, stores, โ€ฆ , op? }` | | [`start`](start.md) | `name` | โ€” | summary, once the worker answers its secret | -| [`stop`](stop.md) | `name` | โ€” | summary with `state: "stopped"` | +| [`stop`](stop.md) | `name` | `force` (Bool) | summary with `state: "stopped"` (`native`: `ready`); force also reports stopped Sandboxes and unknown Workspace frontiers | | [`create`](create.md) | `name`, `platform` (Name) | `cpus` (Int), `memory`, `disk` (Size), `image` (Text) | summary | | [`remove`](remove.md) | `name` | โ€” | `{ name, removed: true }`; a removed platform's record adds `note` | | [`update`](update.md) | `name` | `cpus` (Int), `memory` (Size), `image`, `docker`, `wsl`, `daemon` (Text) | summary | diff --git a/docs/architecture/api/sdk/machine/stop.md b/docs/architecture/api/sdk/machine/stop.md index dae8b1a045..35d4143bd1 100644 --- a/docs/architecture/api/sdk/machine/stop.md +++ b/docs/architecture/api/sdk/machine/stop.md @@ -6,7 +6,7 @@ `machine.stop` stops a machine's worker and, where it has one, its VM, and records it `stopped`. Its disks and Stores stay. It is refused while -Sandboxes run on the machine. +Sandboxes run on the machine unless `force` is selected. | | | | --- | --- | @@ -19,36 +19,59 @@ Sandboxes run on the machine. ```js // JavaScript -await layerfs.machine.stop(name); +await layerfs.machine.stop(name, { force: true }); ``` ```python # Python -layerfs.machine.stop(name) +layerfs.machine.stop(name, force=True) ``` ```bash # CLI -layerfs machine stop +layerfs machine stop [--force] ``` ## Arguments | Name | Kind | Rule | Source | | --- | --- | --- | --- | -| `name` | Name | an existing machine not held by an `op`, with no Sandbox recorded on it | [`power.rs`](../../../../../control/crates/layerfs-machine/src/power.rs) | +| `name` | Name | an existing machine; without `force`, no Sandbox may be recorded on it | [`power.rs`](../../../../../control/crates/layerfs-machine/src/power.rs) | ## Options -None. +| Name | Kind | Default | Rule | +| --- | --- | --- | --- | +| `force` | Bool | false | stop while Sandboxes, mounted Workspaces or commands are active; their uncommitted state may be lost | + +Force stops the platform directly without starting the machine, contacting its +worker, committing changes, unmounting Workspaces one by one, or deleting any +registered machine, Sandbox, Store or disk. On `native` it terminates only this +homeโ€™s recorded Sandbox runtimes and commands; the host stays running and its +machine remains `ready`. On Docker, stopping this homeโ€™s worker container ends +its namespaces Sandboxes; Docker Desktop and the Store volume remain. Lima uses +`limactl stop --force`, including when the host agent reports a broken VM. + +The cross-entity orchestration lives in +[`machine_force.rs`](../../../../../control/crates/layerfs-control/src/machine_force.rs). +A provider failure or partial/uncertain outcome is reported; it is never replayed +or converted into a successful close. ## Returns The machine's summary, as [`machine.get`](get.md) answers it after a live -probe; normally `state: "stopped"` (`ready` for `native`). +probe; normally `state: "stopped"` (`ready` for `native`). Forced stop returns +the recorded summary without a worker probe, plus `forced: true`, +`stopped_sandboxes: [name]` and `unknown: [workspace]`. `sandbox:*` means the +worker-side uncommitted Workspace frontier could not be enumerated. These entries +are not saved Commits. The retained Sandbox records are marked gone for explicit +reclaim, with unknown custody retained if the stop could not be confirmed. ## Where it runs +The sequence and table below describe normal Stop. Explicit force uses the +provider-only path above and bypasses the normal active-Sandbox refusal. + ```text caller's process machine machine_of(name) Busy | Unresolved | NotFound @@ -79,7 +102,7 @@ All in the caller ([routing table](../../../control/routing/routing-table.md)). | --- | --- | --- | --- | | `InvalidInput`, `NotFound` | bad name; no such machine | no | [`record.rs`](../../../../../control/crates/layerfs-machine/src/record.rs) `of` | | `Busy`, `Unresolved` | an `op` holds the machine | no | [`op.rs`](../../../../../control/crates/layerfs-home/src/op.rs) | -| `InUse` | Sandboxes are recorded on it; `detail.related` lists `{kind: "sandbox", name}` | no | [`inventory.rs`](../../../../../control/crates/layerfs-control/src/inventory.rs) `can_stop_machine` | +| `InUse` | without `force`, Sandboxes are recorded on it; `detail.related` lists `{kind: "sandbox", name}` | no | [`inventory.rs`](../../../../../control/crates/layerfs-control/src/inventory.rs) `can_stop_machine` | | `Unavailable` | `start.lock` held for 600 s; nothing changed | no | [`home/entity_lock.rs`](../../../../../control/crates/layerfs-home/src/home/entity_lock.rs) | | `Unavailable`, `Denied`, `Failed` | the provider or guest did not answer or refused; the stop failed once. `worker.json` and the record stay as they were | no | [`error.rs`](../../../../../control/crates/layerfs-platform/src/error.rs) | | `Unknown` | **a provider command timed out: the machine may be stopped while still recorded with its worker.** Read `machine status` first | **yes** | [`setup/tool.rs`](../../../../../control/crates/layerfs-platform/src/setup/tool.rs) | diff --git a/docs/architecture/engine/store/backends.md b/docs/architecture/engine/store/backends.md index e1853446a9..c404f70de0 100644 --- a/docs/architecture/engine/store/backends.md +++ b/docs/architecture/engine/store/backends.md @@ -52,7 +52,7 @@ PostgreSQL runs them unchanged except the few it replaces in | `Busy` | `Busy` | `Busy` | Another writer held the lock; not retried. | | `Closed` | `Refused "Closed"` | `OwnershipUnavailable` | Store closed before this call's turn. | | `Rejected` | `Refused "Rejected"` | `OwnershipUnavailable` | PostgreSQL refused a new connection. | -| `Cancelled` | `Refused "Cancelled"` | `OwnershipUnavailable` | A signal interrupted the Linux writer wait. | +| `Cancelled` | `Refused "Cancelled"` | `OwnershipUnavailable` | A signal interrupted the SQLite writer wait. | | `ReadOnly` | `Refused "ReadOnly"` | `ContinuityUnavailable` | Write on a read-only session. | | `Capacity` | `Refused "Capacity"` | `Capacity("persistence")` | A bound or the disk margin was reached. | | `Integrity` | `Malformed` | `Integrity("persistence")` | Stored or supplied data broke a rule. | @@ -97,8 +97,8 @@ it never queues behind a held lane. SQLite write: Session::run(true, body) any process, one host 1 admission.enter in-process FIFO turn on this session 2 state mutex the connection - 3 Writer::acquire Linux: flock(LOCK_EX) on the Store's parent - directory; macOS: none (skipped if nested) + 3 Writer::acquire Linux/macOS: flock(LOCK_EX) on the Store's + parent directory (skipped if nested) 4 BEGIN IMMEDIATE SQLite's write lock; busy_timeout 0 -> Busy 5 body; COMMIT or ROLLBACK; Unknown -> quarantine 6 finish_writer release the flock @@ -116,20 +116,40 @@ it never queues behind a held lane. | Rule | Enforced by | | --- | --- | | One write transaction at a time per session | `Admission` turn: [`admission.rs`](../../../../engine/crates/layerfs-persistence/src/backend/admission.rs) | -| One writer across processes on one Linux host | Directory `flock`: [`writer_admission.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs) | +| One writer across processes on one Linux or macOS host | Directory `flock`: [`writer_admission.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs) | | One writer in the SQLite file | `BEGIN IMMEDIATE` with `busy_timeout=0`: [`transaction.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/transaction.rs) | | One writer across PostgreSQL clients | `store_lane` row `FOR UPDATE`: [`profile.rs`](../../../../engine/crates/layerfs-persistence/src/backend/postgresql/session/profile.rs) | | No busy handler, no retry | `busy_timeout` reads back 0; `Busy` is returned as is | The lock is taken on a separate descriptor of the Store's directory, never on the main, WAL or SHM file: closing a main-file descriptor would drop SQLite's -own POSIX locks. Several Store files in one directory share this lock. On -macOS `flock` conflicts with SQLite's locks, so there is no cross-process -writer wait: a second writer gets SQLite's `Busy`. Reads (`BEGIN`, or one +own POSIX locks. Several Store files in one directory share this lock. Linux +and macOS both use the directory descriptor, so neither takes an advisory +lock on SQLite's own files. Reads (`BEGIN`, or one implicit snapshot) take the turn but never the directory lock or the lane. The publication lane holds steps 1 and 3 across several transactions; see [History publication](history-publication.md#the-publish-lane). +SQLite connection lifecycle also takes this directory owner: before opening +and applying/checking the profile (including the Disposable background +connection), and around the last Session connection's close. WAL startup and +last-close checkpoint/sidecar teardown can otherwise meet another process's +writer before its ordinary transaction admission. This is a short lifecycle +hold, not a hold over a Store call or the lifetime of an open Session. Read-only +and diagnostic connection initialization uses the same hold; their subsequent +reads remain independent of the writer lane. + +The background connection closes while the Session's main connection is still +alive, so it is never the last connection. It must not reacquire the lane while +the Session joins its thread: a quarantined Session can already retain that +owner. The main connection closes before its retained/new lifecycle owner is +released. If a destructor's single admission wait is interrupted or fails, +it retains the connection until process exit instead of performing unadmitted +teardown or retrying. Checked `seal` retains its explicit close/error result. +Source: [`connection.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs), +[`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs), +[`background.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs). + ## Related - [SQLite](sqlite.md#sessions) and [PostgreSQL](postgresql.md#connection-and-session): each engine's session. diff --git a/docs/architecture/glossary.md b/docs/architecture/glossary.md index 468af21e96..c7c9f13016 100644 --- a/docs/architecture/glossary.md +++ b/docs/architecture/glossary.md @@ -50,7 +50,7 @@ LayerFS (backend, install, scope), the row gives both. | Platform | A machine's kind (`native`, `docker`, `apple-container`, `lima`). It fixes the Sandbox backend, the worker's address and what the support matrix allows. | [Machines: platforms and profiles](control/machines/platforms-and-profiles.md#platform) | | Project | A named LayerStack in a Store, written `/`. | [Project Init](engine/project-init/README.md) | | Publish | `branch.publish`: puts a Branch's head Commit on `main` as a new Layer. | [Store calls: branches](engine/store-calls/branches.md#publish) | -| Publish lane | The Store's admission turn plus its cross-process writer lock (the `flock` on the Store's directory on Linux; the `store_lane` row on PostgreSQL), held by a Commit across its head check, last merge step and publish. | [Store: history publication](engine/store/history-publication.md#the-publish-lane) | +| Publish lane | The Store's admission turn plus its cross-process writer lock (the `flock` on the Store's directory on Linux and macOS; the `store_lane` row on PostgreSQL), held by a Commit across its head check, last merge step and publish. | [Store: history publication](engine/store/history-publication.md#the-publish-lane) | | Reach | How a daemon on a machine dials a PostgreSQL Store: the URL rewritten for that platform and checked once at Sandbox create. | [Network: reach](control/network/reach.md) | | Sandbox | One daemon started by a backend on a machine, plus the commands run in it. The daemon serves the Workspaces; the Sandbox runtime owns the commands. | [Sandboxes](control/sandboxes/README.md) | | Save | A Storage write session: it accepts objects, packs them and `Save::finish` completes them while the Store stays open. | [Physical storage: saves](core/physical-storage/saves.md) | diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/lane/lane.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/lane/lane.rs index 8c9e0b32a9..2cad0e8705 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/lane/lane.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/lane/lane.rs @@ -1,4 +1,4 @@ -//! The Store's publication lane: one connection turn and, on Linux, the +//! The Store's publication lane: one connection turn and, on Linux and macOS, the //! cooperative writer lock, held by one thread across a short read-decide-write //! sequence. Its own SQL work runs as ordinary transactions inside the held //! turn; no other publication of any process can interleave. diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs index f9ed78e2d9..a89b853c5b 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs @@ -1,7 +1,7 @@ -//! Linux local-volume cooperative ownership before one SQLite write attempt. +//! Local-volume cooperative ownership before one SQLite write attempt. use crate::backend::records::BackendError; use std::path::Path; -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] use { nix::{ errno::Errno, @@ -11,13 +11,13 @@ use { }; pub(crate) struct Writer { - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] lock: Flock, } impl Writer { pub(crate) fn acquire(path: &Path) -> Result { - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] { // Never open another FD to SQLite's main/WAL/SHM files here: // closing any main-file FD cancels every process POSIX lock on @@ -47,13 +47,6 @@ impl Writer { Err(_) => Err(BackendError::Integrity), } } - #[cfg(target_os = "macos")] - { - // Darwin's flock conflicts with SQLite's open-file locks. Host Init - // is sole-owner; cross-session contention retains exact SQLite Busy. - let _ = path; - Ok(Self {}) - } #[cfg(not(any(target_os = "linux", target_os = "macos")))] { let _ = path; @@ -67,7 +60,7 @@ impl Writer { /// This is a single attempt, never a retry: no loop, sleep or busy handler /// follows it here, and a caller must not wrap it in one. pub(crate) fn try_acquire(path: &Path) -> Result, BackendError> { - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] { // The same independent directory description as `acquire`. let directory = path @@ -89,15 +82,15 @@ impl Writer { Err(_) => Err(BackendError::Integrity), } } - #[cfg(not(target_os = "linux"))] + #[cfg(not(any(target_os = "linux", target_os = "macos")))] { - // As `acquire`: no advisory lane off Linux. + // As `acquire`: unsupported platforms have no advisory lane. Self::acquire(path).map(Some) } } pub(crate) fn release(self) -> Result<(), Self> { - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] { match self.lock.unlock() { Ok(file) => { @@ -107,7 +100,7 @@ impl Writer { Err((lock, _)) => Err(Self { lock }), } } - #[cfg(not(target_os = "linux"))] + #[cfg(not(any(target_os = "linux", target_os = "macos")))] { Ok(()) } diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/lifecycle/seal.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/lifecycle/seal.rs index e0c9969e4b..236359d030 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/lifecycle/seal.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/lifecycle/seal.rs @@ -12,20 +12,17 @@ impl Session { } // The checkpoint thread ends and closes its connection first. drop(self.background.take()); - let state = self + let mut state = self .state .into_inner() .map_err(|_| PersistenceError::Uncertain)?; if state.quarantined || !state.connection.is_autocommit() { return Err(PersistenceError::Uncertain); } - #[cfg(target_os = "linux")] - let state = { - let mut state = state; + if state.writer_hold.is_none() { state.writer_hold = Some(crate::backend::sqlite::lane::writer_admission::Writer::acquire(&self.path)?); - state - }; + } #[cfg(target_os = "macos")] let identity = super::seal_allocation::Identity::capture(&self.path)?; #[cfg(target_os = "macos")] @@ -40,13 +37,14 @@ impl Session { if row.get::(0)? != 0 { return Err(PersistenceError::Busy); } - state - .connection - .close() - .map_err(|(_, error)| rows::error(error))?; + let connection = state.connection.take().ok_or(PersistenceError::Malformed)?; + if let Err((connection, error)) = connection.close() { + state.connection.restore(connection); + return Err(rows::error(error).into()); + } // SQLite has checked-closed before advisory ownership is released. // Earlier error exits drop State.connection before its retained owner. - if let Some(writer) = state.writer_hold { + if let Some(writer) = state.writer_hold.take() { writer.release().map_err(|_| PersistenceError::Uncertain)?; } for suffix in ["-wal", "-shm", "-journal"] { diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs index e4981e609f..6efb4375cb 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs @@ -103,6 +103,9 @@ impl Checkpointer { impl Drop for Checkpointer { /// Ends the thread after its current pass and closes its connection, /// before the session's own connection closes. + /// This connection is therefore never SQLite's last one: State still owns + /// the main connection. Do not reacquire its directory lane here; a + /// quarantined State may already retain it while this join completes. fn drop(&mut self) { self.wake.take(); if let Some(thread) = self.thread.take() { diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs index 6744ceba04..703d5c21ed 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs @@ -145,7 +145,8 @@ pub struct SqlWork { pub connects: u64, } pub(crate) struct State { - pub(crate) connection: Connection, + pub(crate) connection: super::lifecycle::ConnectionSlot, + pub(crate) path: PathBuf, pub(crate) quarantined: bool, pub(crate) work: RefCell, pub(crate) writer_hold: Option, @@ -205,6 +206,11 @@ impl Session { if !cfg!(any(target_os = "macos", target_os = "linux")) { return Err(BackendError::Integrity); } + // Opening/configuring WAL can initialize shared state before any + // transaction. The same short owner excludes another writer or last + // close, including on read-only and diagnostic opens. Declared before + // the connection so error exits close SQLite before releasing it. + let lifecycle = crate::backend::sqlite::lane::writer_admission::Writer::acquire(path)?; let flags = if writable { OpenFlags::SQLITE_OPEN_READ_WRITE } else { @@ -314,13 +320,17 @@ impl Session { let background = (writable && selected == SqlitePersistenceProfile::Disposable) .then(|| super::background::Checkpointer::start(path)) .transpose()?; + // Session::run takes its own writer ownership; never retain lifecycle + // ownership into those calls or acquire it again in background start. + drop(lifecycle); Ok(Self { diagnostic, background, admission: crate::backend::admission::Admission::default(), path: path.to_path_buf(), state: Mutex::new(State { - connection, + connection: super::lifecycle::ConnectionSlot::new(connection), + path: path.to_path_buf(), quarantined: false, work, writer_hold: None, diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs new file mode 100644 index 0000000000..77b88eb0b9 --- /dev/null +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs @@ -0,0 +1,61 @@ +//! Connection custody at the last Session close, including quarantined owners. +use super::connection::State; +use crate::backend::sqlite::lane::writer_admission::Writer; +use rusqlite::Connection; +use std::ops::Deref; + +/// Allows checked seal and last-owner Drop to close before releasing admission. +pub(crate) struct ConnectionSlot(Option); + +impl ConnectionSlot { + pub(crate) fn new(connection: Connection) -> Self { + Self(Some(connection)) + } + + pub(crate) fn take(&mut self) -> Option { + self.0.take() + } + + pub(crate) fn restore(&mut self, connection: Connection) { + self.0 = Some(connection); + } + + fn is_closed(&self) -> bool { + self.0.is_none() + } +} + +impl Deref for ConnectionSlot { + type Target = Connection; + + fn deref(&self) -> &Connection { + self.0.as_ref().expect("live Session connection") + } +} + +impl Drop for State { + fn drop(&mut self) { + if self.connection.is_closed() { + return; + } + // An uncertain transaction already owns the lane. Taking it again + // through an independent directory descriptor would deadlock. + let writer = match self.writer_hold.take() { + Some(writer) => writer, + None => match Writer::acquire(&self.path) { + Ok(writer) => writer, + Err(_) => { + // Drop cannot report a cancelled/failed admission. Retain + // the SQLite connection until process exit instead of + // attempting unadmitted WAL teardown or retrying the wait. + std::mem::forget(self.connection.take()); + return; + } + }, + }; + // The background connection has already ended; this may be SQLite's + // last connection and can checkpoint/delete the WAL and SHM files. + drop(self.connection.take()); + drop(writer); + } +} diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/mod.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/mod.rs index 2c5b88c914..4127ec2f0b 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/mod.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/mod.rs @@ -2,6 +2,7 @@ pub(crate) mod background; pub(crate) mod checkpoint; pub(crate) mod connection; +pub(crate) mod lifecycle; pub(crate) mod prepared; pub(crate) mod profile; pub(crate) mod query; diff --git a/engine/crates/layerfs-persistence/tests/sqlite_lock_custody.rs b/engine/crates/layerfs-persistence/tests/sqlite_lock_custody.rs index e1d919c135..2c9c480002 100644 --- a/engine/crates/layerfs-persistence/tests/sqlite_lock_custody.rs +++ b/engine/crates/layerfs-persistence/tests/sqlite_lock_custody.rs @@ -1,6 +1,6 @@ //! Cooperative readiness must preserve SQLite's own POSIX lock custody. //! Uses public Stores and an independently bounded raw SQLite writer process. -#![cfg(target_os = "linux")] +#![cfg(any(target_os = "linux", target_os = "macos"))] mod support; use layerfs_content::{FinalizedObject, ObjectRole}; use layerfs_history::HistoryCatalogConfig; diff --git a/engine/crates/layerfs-persistence/tests/writer_admission.rs b/engine/crates/layerfs-persistence/tests/writer_admission.rs index eb9fce9fa5..3911b0f7db 100644 --- a/engine/crates/layerfs-persistence/tests/writer_admission.rs +++ b/engine/crates/layerfs-persistence/tests/writer_admission.rs @@ -4,7 +4,7 @@ use layerfs_content::{FinalizedObject, ObjectRole}; use layerfs_history::HistoryCatalogConfig; use layerfs_persistence::{PersistenceConfig, SqliteHandles, SqlitePersistenceProfile}; use layerfs_storage::{location::PackInfo, port::*, Storage, StoragePolicy}; -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] use std::{ io::{Read, Write}, os::unix::net::{UnixListener, UnixStream}, @@ -26,7 +26,7 @@ fn config(path: &Path) -> PersistenceConfig { fn create(path: &Path) -> SqliteHandles { SqliteHandles::create(config(path), StoragePolicy::frozen_default(), &authority()).unwrap() } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] fn open(path: &Path) -> SqliteHandles { SqliteHandles::open_writable( config(path), @@ -130,12 +130,12 @@ fn shared_session_reservation_waits_for_an_ordinary_public_read_owner() { assert_eq!(after.rollbacks, before.rollbacks); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] struct Held { child: Child, socket: UnixStream, } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] impl Held { fn start(path: &Path, socket_path: &Path) -> Self { let listener = UnixListener::bind(socket_path).unwrap(); @@ -168,6 +168,8 @@ impl Held { } } }; + // Darwin inherits the listener's nonblocking flag on accepted sockets. + socket.set_nonblocking(false).unwrap(); let mut held = Self { child, socket }; held.socket .set_read_timeout(Some(Duration::from_secs(3))) @@ -193,7 +195,7 @@ impl Held { } } } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] impl Drop for Held { fn drop(&mut self) { let _ = self.child.kill(); @@ -202,7 +204,7 @@ impl Drop for Held { } #[test] -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] fn another_process_admission_precedes_the_single_sqlite_begin() { // Keep the owned AF_UNIX path below Darwin's sockaddr_un capacity. let temp = support::Temp::new("wa"); @@ -243,7 +245,68 @@ fn another_process_admission_precedes_the_single_sqlite_begin() { } #[test] -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] +fn another_process_admission_precedes_read_only_connection_open() { + let temp = support::Temp::new("wa-open"); + let path = temp.join("store.sqlite"); + let original = create(&path); + let held = Held::start(&path, &temp.join("s")); + let (ready_tx, ready_rx) = mpsc::channel(); + let (result_tx, result_rx) = mpsc::channel(); + let opener = std::thread::spawn(move || { + ready_tx.send(()).unwrap(); + let result = SqliteHandles::open_read_only( + config(&path), + &authority().binding_key, + authority().cursor_key, + ) + .map(|handles| handles.profile().clone()); + result_tx.send(result).unwrap(); + }); + ready_rx.recv_timeout(Duration::from_secs(2)).unwrap(); + let premature = result_rx.recv_timeout(Duration::from_millis(100)).ok(); + held.release(); + let outcome = premature + .clone() + .unwrap_or_else(|| result_rx.recv_timeout(Duration::from_secs(3)).unwrap()); + opener.join().unwrap(); + assert!( + premature.is_none(), + "connection initialization bypassed its lifecycle owner" + ); + assert_eq!(outcome.unwrap().busy_timeout, 0); + drop(original); +} + +#[test] +#[cfg(any(target_os = "linux", target_os = "macos"))] +fn another_process_admission_precedes_last_connection_close() { + let temp = support::Temp::new("wa-close"); + let path = temp.join("store.sqlite"); + let handles = create(&path); + let held = Held::start(&path, &temp.join("s")); + let (ready_tx, ready_rx) = mpsc::channel(); + let (result_tx, result_rx) = mpsc::channel(); + let closer = std::thread::spawn(move || { + ready_tx.send(()).unwrap(); + drop(handles); + result_tx.send(()).unwrap(); + }); + ready_rx.recv_timeout(Duration::from_secs(2)).unwrap(); + let premature = result_rx.recv_timeout(Duration::from_millis(100)).ok(); + held.release(); + if premature.is_none() { + result_rx.recv_timeout(Duration::from_secs(3)).unwrap(); + } + closer.join().unwrap(); + assert!( + premature.is_none(), + "last connection close bypassed its lifecycle owner" + ); +} + +#[test] +#[cfg(any(target_os = "linux", target_os = "macos"))] fn two_opened_handles_admit_eight_distinct_reservations_without_conflict() { let temp = support::Temp::new("writer-admission-two-handles"); let path = temp.join("store.sqlite"); From eda9c04f126ee0a85a13ed8024e0b77c844cf6cf Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Thu, 12 Nov 2026 12:01:58 +0800 Subject: [PATCH 2/6] feat(sandbox): forced native close without Store or daemon discovery A forced sandbox.close on native now stops the Sandbox from its own records: no Store open, daemon control call, active-Workspace refusal or retry. The backend Stop checks the runtime identity and ends its mounts and processes; Delete checks the physical detach before the runtime goes. Each recorded or held Workspace is answered as unknown. This is the remaining part of c1cf6fd169 that the 7b5f5a5630 port did not carry. The forced machine.stop on native shares the same host builder (layerfs_sandbox::force::native_host). Also unroll two single-element loops in the machine_force test, which warning-denying Clippy refused. Validation (registered private worktree, macOS): - warning-denying all-targets Clippy: layerfs-sandbox, -control, -api, -machine, -platform, -store (postgresql,machine); layerfs-cli (postgresql); layerfs-daemon. - layerfs-api machine_force and live machine_force_native with a matching private daemon (native machine.stop and sandbox.close force end the exact daemon, command and mount without opening the Store): passed. - layerfs-api sandbox_* and machine_* suites: 34 passed. Platform machine_force/machine_lima 17, sandbox-backend local_stop_mount 1, store lease_race 1, persistence writer_admission/sqlite_lock_custody 12, CLI capabilities/stream_framed 7, sandbox capacity_read/pool_admission 8, api sandbox_capacity 8: all passed. - The debug CLI's help --json advertises machine.stop and sandbox.close force. Production LOC: 159152 -> 159204 (delta +52) Scope: all first-party core, engine, control and API product sources; tests, docs, tooling and generated artifacts excluded. Method: pinned tools/production_loc.py (blob 35e4a6d601fa) --root --json on the parent and the staged tree. Co-Authored-By: Claude Opus 5.5 --- api/crates/layerfs-api/tests/machine_force.rs | 10 +-- .../layerfs-control/src/machine_force.rs | 18 +---- control/crates/layerfs-sandbox/src/close.rs | 22 ++++-- control/crates/layerfs-sandbox/src/force.rs | 77 +++++++++++++++++++ control/crates/layerfs-sandbox/src/lib.rs | 2 + 5 files changed, 98 insertions(+), 31 deletions(-) create mode 100644 control/crates/layerfs-sandbox/src/force.rs diff --git a/api/crates/layerfs-api/tests/machine_force.rs b/api/crates/layerfs-api/tests/machine_force.rs index e12b6d5799..109f888aea 100644 --- a/api/crates/layerfs-api/tests/machine_force.rs +++ b/api/crates/layerfs-api/tests/machine_force.rs @@ -61,9 +61,7 @@ esac ("address", t(&listener.local_addr().unwrap().to_string())), ("secret", t("aa")), ]); - for machine in ["vm"] { - write_record(&paths::worker_record(&home, machine), &worker).unwrap(); - } + write_record(&paths::worker_record(&home, "vm"), &worker).unwrap(); for (sandbox, machine) in [("one", "vm"), ("two", "vm")] { write_record( &paths::sandbox_record(&home, sandbox), @@ -117,10 +115,8 @@ esac matches!(listener.accept(),Err(error) if error.kind()==std::io::ErrorKind::WouldBlock), "worker must never be contacted" ); - for machine in ["vm"] { - assert!(!paths::worker_record(&home, machine).exists()); - assert!(paths::machine_record(&home, machine).exists()); - } + assert!(!paths::worker_record(&home, "vm").exists()); + assert!(paths::machine_record(&home, "vm").exists()); let sandbox = call(&home, "sandbox.get", &[("name", t("one"))]).unwrap(); assert_eq!(sandbox.get_text("state"), Some("gone")); let logs = std::fs::read_to_string(root.join("calls")).unwrap(); diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs index 78a0c32aa3..9dc59f5a1a 100644 --- a/control/crates/layerfs-control/src/machine_force.rs +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -8,7 +8,6 @@ use layerfs_home::outcome::{failed, refused, Outcome}; use layerfs_machine::platform::{self as adapters, Platform}; use layerfs_machine::record::MachineRecord; use layerfs_machine::{machine_refusal as refusal, machine_spec}; -use layerfs_sandbox::host::{Deploy, Host, Message}; use layerfs_sandbox::record::SandboxRecord; use layerfs_sandbox::{direct, files, reopen}; use std::path::Path; @@ -202,22 +201,7 @@ fn native_stop(home: &Path, sandbox: &SandboxRecord) -> Result<()> { if sandbox.machine_stopped() { return Ok(()); } - let host = Host { - home: home.to_owned(), - store: String::new(), - project: String::new(), - sandbox: Some(sandbox.name().to_owned()), - store_record: None, - store_path: None, - deploy: Some(Deploy { - local_dir: Some(paths::run_root(home).display().to_string()), - container_prefix: Some(format!("lfs-{}-", &home_id(home)?[..8])), - ..Deploy::default() - }), - postgresql: None, - message: Message::Absent, - started: None, - }; + let host = layerfs_sandbox::force::native_host(home, sandbox.name())?; let Some(daemon) = files::started(&sandbox.dir(home))? else { if sandbox.id().is_some() { return Err(refused( diff --git a/control/crates/layerfs-sandbox/src/close.rs b/control/crates/layerfs-sandbox/src/close.rs index 188c10f7f3..7974b2570c 100644 --- a/control/crates/layerfs-sandbox/src/close.rs +++ b/control/crates/layerfs-sandbox/src/close.rs @@ -104,13 +104,21 @@ pub fn close(home: &Path, events: &mut Events<'_>, request: &CloseRequest) -> Re } } } - let closed = on_machine( - &home, - events, - &record, - Call::Close { force, held }, - crate::dispatch::call, - ); + let closed = if force && record.machine() == "native" { + // a forced close on `native` needs no Store or daemon discovery, + // even when active mounts or a broken Store would block the + // ordinary route + crate::force::native_host(&home, name) + .and_then(|host| crate::force::close(&host, held.as_deref().unwrap_or(&[]))) + } else { + on_machine( + &home, + events, + &record, + Call::Close { force, held }, + crate::dispatch::call, + ) + }; let _lock = home::lock::take(&home)?; // on `native` the Sandbox's own side removed the folder already let dir = record.dir(&home); diff --git a/control/crates/layerfs-sandbox/src/force.rs b/control/crates/layerfs-sandbox/src/force.rs new file mode 100644 index 0000000000..2ae081f1c5 --- /dev/null +++ b/control/crates/layerfs-sandbox/src/force.rs @@ -0,0 +1,77 @@ +//! The forced close of a `native` Sandbox, which bypasses Store and daemon +//! discovery and live unmount admission: only its runtime stops; the +//! machine, its Stores and saved history stay. A forced `machine.stop` on +//! `native` stops each Sandbox's runtime the same way ([`native_host`], +//! `Recorded::stop_now`). +use crate::host::{Deploy, Host, Message}; +use layerfs_bridge::control::{SdkRefusal, SdkValue}; +use layerfs_home::home::{self, paths}; +use layerfs_home::outcome::{failed, refused, Outcome}; +use std::path::Path; + +/// The host operation's view of `native` Sandbox `sandbox` from its own +/// records alone: no Store, project or daemon is resolved. +pub fn native_host(home: &Path, sandbox: &str) -> Result { + Ok(Host { + home: home.to_path_buf(), + store: String::new(), + project: String::new(), + sandbox: Some(sandbox.to_owned()), + store_record: None, + store_path: None, + deploy: Some(Deploy { + local_dir: Some(paths::run_root(home).display().to_string()), + container_prefix: Some(format!("lfs-{}-", &home::opened(home)?.id[..8])), + ..Deploy::default() + }), + postgresql: None, + message: Message::Absent, + started: None, + }) +} + +/// Forced `sandbox.close` of the Sandbox `host` names, on this process's +/// kernel: no daemon control call, Store open, active-Workspace refusal or +/// retry. The backend's Stop checks the runtime identity and ends its +/// mounts and processes; its Delete independently checks the physical +/// detach before the runtime goes. Answers each recorded or `held` +/// Workspace as `unknown`: its uncommitted changes end with the Sandbox. +pub fn close(host: &Host, held: &[String]) -> Outcome { + let id = host.sandbox()?.to_owned(); + let directory = crate::files::sandbox_dir(host)?; + let _guard = layerfs_store::project::discovery::guard(&directory)?; + let value = crate::files::started(&directory)? + .ok_or_else(|| refused("NoSuchSandbox", format!("no Sandbox {id} is recorded")))?; + let mut unknown = crate::files::workspaces(&directory)? + .into_iter() + .map(|(path, record)| { + record + .path() + .map(str::to_owned) + .unwrap_or_else(|| path.display().to_string()) + }) + .collect::>(); + for path in held { + if !unknown.contains(path) { + unknown.push(path.to_owned()); + } + } + crate::reopen::recorded(host, &id, &value)?.stop_now()?; + crate::reopen::recorded(host, &id, &value)? + .stopped() + .delete() + .map_err(failed)?; + crate::registry::SANDBOXES + .lock() + .map_err(|_| refused("Failed", "the Sandbox table's lock was poisoned"))? + .remove(&directory); + std::fs::remove_dir_all(&directory).map_err(failed)?; + Ok(SdkValue::map([ + ("kind", SdkValue::text("Closed")), + ("sandbox", SdkValue::text(id)), + ( + "unknown", + SdkValue::List(unknown.into_iter().map(SdkValue::Text).collect()), + ), + ])) +} diff --git a/control/crates/layerfs-sandbox/src/lib.rs b/control/crates/layerfs-sandbox/src/lib.rs index d376e97536..c52fb96ba3 100644 --- a/control/crates/layerfs-sandbox/src/lib.rs +++ b/control/crates/layerfs-sandbox/src/lib.rs @@ -32,6 +32,8 @@ pub mod exec; pub mod family; #[doc(hidden)] pub mod files; +/// The forced close of a `native` Sandbox, from its own records alone. +pub mod force; mod get; #[doc(hidden)] pub mod host; From d4d9df91cf31585d2a810ff9210228e74c29765a Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Sat, 14 Nov 2026 17:12:17 +0800 Subject: [PATCH 3/6] fix(store, machine): read-only closes take no lane; native force-stop refusals before a signal stay certain Fixes the two blocking findings of the independent review of #656 at 18b4df7a38. 1. A read-only or diagnostic SQLite session's close no longer takes the Store directory's writer lane. The last-close lane added for #656 made every session wait for it, so after one uncertain write (a quarantined writer keeps the lane) a daemon's reader close blocked forever under the reader-set mutex, and with it mount, unmount and session lookup; the diagnostic open and control's idle-Store reaper hung the same way. A read-only connection cannot checkpoint or remove -wal/-shm, so the lane bought no custody there. State now keeps the Store path only for writable sessions (close_lane); writable closes are unchanged. 2. A forced machine.stop on native records a Sandbox as uncertain only once its stop has been sent. Recorded::for_stop reopens the recorded runtime without signalling (identity and boot checks, missing runtime); a refusal there now clears the op instead of leaving an uncertain machine.stop on native, which the new machine gates turned into a home-wide lockout with no call to clear it. Partial progress no longer makes a native refusal uncertain either: each native Sandbox is recorded as it stops. Recorded::stop_now is for_stop followed by the stop, as before. Tests added: writer_admission read_only_close_does_not_wait_for_a_held_writer_lane; api machine_force native_force_stop_refused_before_any_signal_leaves_no_uncertain_op. Both fail on 18b4df7a38's product code and pass here. No test deleted. Docs: engine/store/sqlite.md (Last close row, including the existing checkpoint-thread start exception the review noted), api/sdk/machine.md (machine.stop force on native). Checks (toolchain 1.85.1, registered worktree via tools/worktree.py): - Clippy --all-targets -D warnings, macOS and Linux: layerfs-persistence, -sandbox, -control, -api (machine, postgresql), -store, -cli (postgresql), -daemon: clean. rustfmt clean on changed files. - macOS: persistence writer_admission 14, sqlite_lock_custody 2, idle_checkpoint 4; api machine_force 2, sandbox_records 2, sandbox_lost 1, sandbox_family 1; platform machine_force 9: passed. - Linux: persistence writer_admission 15, sqlite_lock_custody 2, idle_checkpoint 4; store lease_race 1, lease_stale 1; platform machine_force 9; api machine_force 2, sandbox_lost 1: passed. - Linux mount lane (tools/ci_tests.py mount-targets, 45 binaries from layerfs-daemon and layerfs-control, each in a fresh container with /dev/fuse, SYS_ADMIN, apparmor=unconfined and a fresh ext4 /tmp, each under timeout 120 with --test-threads=1): 45/45 passed, 116 tests passed, 3 ignored by their own attributes. - Not run: machine_force_native (needs a matching private daemon); Docker/Lima providers live. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Production LOC: 161104 -> 161117 (delta +13); core 39787 -> 39787, engine 74810 -> 74811, control 40527 -> 40539, api 5980 -> 5980. Reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Method: tools/production_loc.py (blob 35e4a6d601fa) --root --json on the parent and the staged tree. Co-Authored-By: Claude Opus 5.5 --- api/crates/layerfs-api/tests/machine_force.rs | 46 +++++++++++++++++++ .../layerfs-control/src/machine_force.rs | 27 ++++++++--- control/crates/layerfs-sandbox/src/reopen.rs | 16 +++++-- docs/architecture/api/sdk/machine.md | 7 ++- docs/architecture/engine/store/sqlite.md | 2 +- .../src/backend/sqlite/session/connection.rs | 7 ++- .../src/backend/sqlite/session/lifecycle.rs | 17 ++++--- .../tests/writer_admission.rs | 33 +++++++++++++ 8 files changed, 133 insertions(+), 22 deletions(-) diff --git a/api/crates/layerfs-api/tests/machine_force.rs b/api/crates/layerfs-api/tests/machine_force.rs index 109f888aea..720de2fd68 100644 --- a/api/crates/layerfs-api/tests/machine_force.rs +++ b/api/crates/layerfs-api/tests/machine_force.rs @@ -190,3 +190,49 @@ esac assert_eq!(native.get_text("state"), Some("ready")); std::fs::remove_dir_all(root).unwrap(); } + +#[test] +fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { + let root = std::env::temp_dir().join(format!("lfs-force-pre-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let home = root.join("home"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::set_permissions(&home, std::fs::Permissions::from_mode(0o700)).unwrap(); + home::open(&home).unwrap(); + // A started Sandbox whose runtime record is missing: the stop is refused + // before anything is signalled, so nothing about it is uncertain. + write_record( + &paths::sandbox_record(&home, "lost"), + &SdkValue::map([ + ("name", t("lost")), + ("id", t("lost")), + ("machine", t("native")), + ("project", t("shop/app")), + ]), + ) + .unwrap(); + let refused = call( + &home, + "machine.stop", + &[("name", t("native")), ("force", SdkValue::Bool(true))], + ) + .unwrap_err(); + assert_eq!(refused.code, "Unavailable", "{refused:?}"); + let op = read_record(&paths::machine_record(&home, "native")) + .unwrap() + .and_then(|record| record.get("op").cloned()); + assert!( + op.is_none(), + "a refusal before any signal left an op on native: {op:?}" + ); + let sandbox = read_record(&paths::sandbox_record(&home, "lost")) + .unwrap() + .unwrap(); + for field in ["machine_stopped", "machine_stop_unknown"] { + assert!( + sandbox.get(field).is_none(), + "an unsignalled Sandbox was marked {field}: {sandbox:?}" + ); + } + std::fs::remove_dir_all(root).unwrap(); +} diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs index 9dc59f5a1a..cdf16776c4 100644 --- a/control/crates/layerfs-control/src/machine_force.rs +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -62,9 +62,14 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { let attempted = (|| -> Result<()> { if spec.kind == Platform::Native { for sandbox in &sandboxes { - native_attempt = Some(sandbox); - native_stop(&home, sandbox)?; + // A refusal before the signal leaves this runtime as it was: + // only a sent stop makes its outcome uncertain. + if let Some(mut runtime) = native_runtime(&home, sandbox)? { + native_attempt = Some(sandbox); + runtime.stop(0).map_err(failed)?; + } mark(&home, sandbox, true)?; + native_attempt = None; stopped.push(sandbox.name().to_owned()); unknown.extend(workspaces(&home, sandbox)?); } @@ -84,9 +89,11 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { Ok(()) })(); if let Err(mut error) = attempted { + // Native Sandboxes are recorded one by one as they stop, so only the + // one being signalled is uncertain; a provider stop covers them all. let uncertain = error.code == "Unknown" - || !stopped.is_empty() || native_attempt.is_some() + || (spec.kind != Platform::Native && !stopped.is_empty()) || (spec.kind != Platform::Native && error.code != "Unsupported"); { let _home = lock::take(&home)?; @@ -197,9 +204,14 @@ fn mark(home: &Path, sandbox: &SandboxRecord, stopped: bool) -> Result<()> { } /// Native is the host itself: stop only this home's recorded runtime processes. -fn native_stop(home: &Path, sandbox: &SandboxRecord) -> Result<()> { +/// The Sandbox's recorded native runtime, reopened for a stop without +/// sending anything; `None` when there is nothing left to stop. +fn native_runtime( + home: &Path, + sandbox: &SandboxRecord, +) -> Result>> { if sandbox.machine_stopped() { - return Ok(()); + return Ok(None); } let host = layerfs_sandbox::force::native_host(home, sandbox.name())?; let Some(daemon) = files::started(&sandbox.dir(home))? else { @@ -212,9 +224,10 @@ fn native_stop(home: &Path, sandbox: &SandboxRecord) -> Result<()> { ), )); } - return sandbox.free(); + sandbox.free()?; + return Ok(None); }; - reopen::recorded(&host, sandbox.name(), &daemon)?.stop_now() + reopen::recorded(&host, sandbox.name(), &daemon)?.for_stop() } /// These are lost/unknown uncommitted frontiers, never manufactured commits. diff --git a/control/crates/layerfs-sandbox/src/reopen.rs b/control/crates/layerfs-sandbox/src/reopen.rs index 405b209193..2ddd38c07c 100644 --- a/control/crates/layerfs-sandbox/src/reopen.rs +++ b/control/crates/layerfs-sandbox/src/reopen.rs @@ -285,7 +285,16 @@ fn local_record(_: &Host, id: &str, _: &DaemonRecord) -> Result { impl Recorded { /// Stop an exact recorded runtime without endpoint admission or daemon RPC. pub fn stop_now(self) -> Result<()> { - let mut owner = match self { + match self.for_stop()? { + Some(mut owner) => owner.stop(0).map_err(failed), + None => Ok(()), + } + } + /// The exact recorded runtime, reopened for a stop that has not started: + /// nothing is signalled here, so a refusal leaves the runtime as it was. + /// `None`: the runtime is already stopped. + pub fn for_stop(self) -> Result>> { + Ok(Some(match self { #[cfg(any(target_os = "linux", target_os = "macos"))] Self::Local { directory, @@ -300,11 +309,10 @@ impl Recorded { #[cfg(target_os = "linux")] Self::Namespaces(runtime, id, record) => match runtime.reopen(id, &record) { Ok(owner) => Box::new(owner), - Err(error) if error.container_stopped() => return Ok(()), + Err(error) if error.container_stopped() => return Ok(None), Err(error) => return Err(failed(error)), }, - }; - owner.stop(0).map_err(failed) + })) } /// The runtime, observed running once (it starts nothing). pub fn reopen(self) -> std::result::Result, Box> { diff --git a/docs/architecture/api/sdk/machine.md b/docs/architecture/api/sdk/machine.md index e04bb60018..53e8875600 100644 --- a/docs/architecture/api/sdk/machine.md +++ b/docs/architecture/api/sdk/machine.md @@ -240,8 +240,11 @@ probe, plus `forced: true`, `stopped_sandboxes: [name]` and `unknown: [workspace]` (`sandbox:*` when a worker-side Workspace frontier could not be listed; these are not saved Commits). Stopped Sandboxes are recorded `gone` for explicit reclaim, or `unresolved` when the stop could not be -confirmed. A provider failure or uncertain outcome is reported, never replayed -or turned into success +confirmed. On `native` a Sandbox is uncertain only once its stop has been +sent: a refusal before that (no recorded runtime, or a recorded process +identity that no longer matches) leaves that Sandbox and the machine as they +were, with no op left on `native`. A provider failure or uncertain outcome is +reported, never replayed or turned into success ([`machine_force.rs`](../../../../control/crates/layerfs-control/src/machine_force.rs)). ### machine.create diff --git a/docs/architecture/engine/store/sqlite.md b/docs/architecture/engine/store/sqlite.md index 6aecb3bc4f..13994b1060 100644 --- a/docs/architecture/engine/store/sqlite.md +++ b/docs/architecture/engine/store/sqlite.md @@ -56,7 +56,7 @@ UPDATE and DELETE on `pack` and `pack_unit`. Open checks `application_id`, | Guard | `busy_timeout=0`; `SQLITE_DBCONFIG_DEFENSIVE` on | same | | Settings | `profile::apply`, then read back into `ConnectionProfile`; a mismatch is `Integrity`; an existing Store not in `wal` mode is refused | [`profile.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/profile.rs) | | Checkpoint thread | writable Disposable session only: a second connection on its own thread; failure to start it refuses the open | [`background.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs) | -| Last close | Dropping the session takes `Writer::acquire(path)` (or reuses the one an uncertain transaction already holds) around the main connection's close, which may be SQLite's last and checkpoint or remove `-wal`/`-shm`; the checkpoint thread's connection closes first and never takes it. If that wait fails, the connection is kept until process exit rather than torn down without the lane or retried | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | +| Last close | Dropping a writable session takes `Writer::acquire(path)` (or reuses the one an uncertain transaction already holds) around the main connection's close, which may be SQLite's last and checkpoint or remove `-wal`/`-shm`; the checkpoint thread's connection closes first and never takes it. If that wait fails, the connection is kept until process exit rather than torn down without the lane or retried. A read-only or diagnostic session cannot checkpoint or remove those files, so its close takes no lane and never waits behind a writer kept after an uncertain write. One exception: an open whose checkpoint thread fails to start drops its new connection after the open's lane is released | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | | Setting | Disposable | Source | | --- | --- | --- | diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs index 77e2362087..c080697dae 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs @@ -146,7 +146,10 @@ pub struct SqlWork { } pub(crate) struct State { pub(crate) connection: super::lifecycle::ConnectionSlot, - pub(crate) path: PathBuf, + /// The Store path whose lane a writable session takes around its last + /// close; `None` for read-only and diagnostic sessions, whose close cannot + /// checkpoint or remove `-wal`/`-shm` and so takes no lane. + pub(crate) close_lane: Option, pub(crate) quarantined: bool, pub(crate) work: RefCell, pub(crate) writer_hold: Option, @@ -335,7 +338,7 @@ impl Session { path: path.to_path_buf(), state: Mutex::new(State { connection: super::lifecycle::ConnectionSlot::new(connection), - path: path.to_path_buf(), + close_lane: writable.then(|| path.to_path_buf()), quarantined: false, work, writer_hold: None, diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs index 77b88eb0b9..1689725a49 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs @@ -40,10 +40,15 @@ impl Drop for State { } // An uncertain transaction already owns the lane. Taking it again // through an independent directory descriptor would deadlock. - let writer = match self.writer_hold.take() { - Some(writer) => writer, - None => match Writer::acquire(&self.path) { - Ok(writer) => writer, + let writer = match (self.writer_hold.take(), &self.close_lane) { + (Some(writer), _) => Some(writer), + // A read-only or diagnostic connection cannot checkpoint or remove + // the WAL and SHM files, so its close takes no lane: it must not + // wait behind a writer another session keeps after an uncertain + // write. + (None, None) => None, + (None, Some(path)) => match Writer::acquire(path) { + Ok(writer) => Some(writer), Err(_) => { // Drop cannot report a cancelled/failed admission. Retain // the SQLite connection until process exit instead of @@ -53,8 +58,8 @@ impl Drop for State { } }, }; - // The background connection has already ended; this may be SQLite's - // last connection and can checkpoint/delete the WAL and SHM files. + // The background connection has already ended; a writable close may be + // SQLite's last connection and can checkpoint/delete the WAL and SHM. drop(self.connection.take()); drop(writer); } diff --git a/engine/crates/layerfs-persistence/tests/writer_admission.rs b/engine/crates/layerfs-persistence/tests/writer_admission.rs index 64625fa1b8..10768f3451 100644 --- a/engine/crates/layerfs-persistence/tests/writer_admission.rs +++ b/engine/crates/layerfs-persistence/tests/writer_admission.rs @@ -379,6 +379,39 @@ fn another_process_admission_precedes_last_connection_close() { ); } +#[test] +#[cfg(any(target_os = "linux", target_os = "macos"))] +fn read_only_close_does_not_wait_for_a_held_writer_lane() { + // A writer kept after an uncertain write holds the directory lane until + // its session goes; a reader's close must still return meanwhile. + let temp = support::Temp::new("wa-ro-close"); + let path = temp.join("store.sqlite"); + let original = create(&path); + let reader = SqliteHandles::open_read_only( + config(&path), + &authority().binding_key, + authority().cursor_key, + ) + .unwrap(); + let held = Held::start(&path, &temp.join("s")); + let (result_tx, result_rx) = mpsc::channel(); + let closer = std::thread::spawn(move || { + drop(reader); + result_tx.send(()).unwrap(); + }); + let closed = result_rx.recv_timeout(Duration::from_secs(2)).ok(); + held.release(); + if closed.is_none() { + result_rx.recv_timeout(Duration::from_secs(3)).unwrap(); + } + closer.join().unwrap(); + assert!( + closed.is_some(), + "a read-only close waited for the writer lane" + ); + drop(original); +} + #[test] #[cfg(any(target_os = "linux", target_os = "macos"))] fn two_opened_handles_admit_eight_distinct_reservations_without_conflict() { From b4a028321e5c0df15226380727d4b38deffb546d Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Sat, 14 Nov 2026 17:32:09 +0800 Subject: [PATCH 4/6] fix(store, machine, sandbox): close without waiting; force-stop custody and continuation Fixes the non-blocking findings of the independent reviews of #656 at 18b4df7a38 and d4d9df91cf. Store (persistence): - A writable session's last close makes one non-blocking lane attempt. With the lane it closes as before (checkpoint, WAL/SHM removal under the lane). Without it (held elsewhere, or the attempt failed) it sets SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE and closes: SQLite then takes no exclusive lock, runs no checkpoint and unlinks nothing, so the close neither waits behind a writer kept after an uncertain write nor tears the WAL down unadmitted, and the connection is no longer leaked. Only if that setting fails is the connection kept until process exit. (Review findings A and 3.) Force stop (control): - native: a refusal before the signal no longer ends the stop; the other Sandboxes are still stopped, and the call answers the first refusal with detail.refused_sandboxes beside stopped_sandboxes and unknown. (C) - native uncertainty is phase-based only: a Sandbox is uncertain once its stop was sent; a code alone no longer makes it so. (B) - Recording the outcome no longer replaces the stop's own refusal: a failure while writing records is reported in detail.custody_errors. (6) - A forced stop may settle an uncertain machine.stop this process left, or one whose owner has ended; one a live process still holds is Busy, as the op standing table says. (7) Forced native sandbox.close (control): - Refused Busy while a live process holds native (e.g. a forced machine.stop in progress); an intent an ended process left does not stop it. (10) - Once the runtime stopped, a later failure (delete, registry, folder removal) carries detail.runtime_stopped and the record is marked stopped instead of left as it was. (5) Docs: engine/store/sqlite.md (Last close), api/sdk/machine.md (force on native, refused_sandboxes, custody_errors, settling an uncertain stop; docker InvalidInput for a container that is not a LayerFS worker, which the ordinary stop now also checks: finding 9), api/sdk/sandbox.md (forced native close: no per-command settle or mount check, Busy rule, runtime_stopped), control/sandboxes/README.md (record format: the machine_stopped and machine_stop_unknown fields, "v" unchanged, older builds: finding 8). Tests: - Replaced writer_admission another_process_admission_precedes_last_connection_close (added by this PR; it asserted the waiting close this commit removes) with writable_last_close_without_the_lane_neither_waits_nor_tears_down_the_wal (no wait and the WAL kept while the lane is held; an admitted close removes it). - api machine_force native_force_stop_refused_before_any_signal_leaves_no_uncertain_op now has two Sandboxes and checks the stop continues past the refused one. - api machine_force_native gains an identity case: a recorded pid naming an unrelated live process is refused before any signal, neither process is signalled, and no op is left on native. - The two changed unit tests fail on d4d9df91cf's product code and pass here. Checks (toolchain 1.85.1, registered worktree via tools/worktree.py): - Clippy --all-targets -D warnings, macOS and Linux, on persistence, sandbox, sandbox-backend (Linux), control, platform, machine (Linux), api (machine, postgresql), store, cli (postgresql), daemon: clean. - macOS: persistence writer_admission 14, sqlite_lock_custody 2, idle_checkpoint 4; store lease_race 1, lease_stale 1; platform machine_force 9, machine_lima 8; cli capabilities 4, stream_framed 3; api machine_force 2, sandbox_records 2, sandbox_lost 1, sandbox_family 1, sandbox_capacity_worker 3: passed. machine_force_native run live with LAYERFS_DAEMON (debug macOS daemon of this tree): 2 passed in 2.0 s, no daemon or helper left. - Linux: persistence writer_admission 15, sqlite_lock_custody 2, idle_checkpoint 4; store lease_race 1, lease_stale 1; platform machine_force 9; api machine_force 2, sandbox_lost 1, sandbox_capacity_worker 3: passed. - Linux mount lane (45 binaries, each in a fresh container with /dev/fuse, SYS_ADMIN, fresh ext4 /tmp, timeout 120, --test-threads=1): 44 binaries passed; kernel_coherence failed once in size_never_shrinks_under_concurrent_append_and_refusals_stay_refused ("read length: 0 after 1000", a FUSE read racing appends; overlay and fuse do not depend on layerfs-persistence, and the test closes no Store during the race). One diagnostic rerun of that binary passed 6/6. The same binary passed at d4d9df91cf. Reported as an intermittent failure, not counted as a pass. Totals of the first run: 110 passed, 1 failed, 3 ignored. - Not run: live Docker/Lima providers. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Production LOC: 161117 -> 161173 (delta +56); core 39787 -> 39787, engine 74811 -> 74818, control 40539 -> 40588, api 5980 -> 5980. Reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Method: tools/production_loc.py (blob 35e4a6d601fa) --root --json on the parent and the staged tree. Co-Authored-By: Claude Opus 5.5 --- api/crates/layerfs-api/tests/machine_force.rs | 36 +++++--- .../layerfs-api/tests/machine_force_native.rs | 54 +++++++++++- .../layerfs-control/src/machine_force.rs | 84 +++++++++++++------ control/crates/layerfs-sandbox/src/close.rs | 13 ++- control/crates/layerfs-sandbox/src/force.rs | 28 ++++++- docs/architecture/api/sdk/machine.md | 13 ++- docs/architecture/api/sdk/sandbox.md | 5 ++ docs/architecture/control/sandboxes/README.md | 1 + docs/architecture/engine/store/sqlite.md | 2 +- .../src/backend/sqlite/session/lifecycle.rs | 40 +++++---- .../tests/writer_admission.rs | 21 ++--- 11 files changed, 226 insertions(+), 71 deletions(-) diff --git a/api/crates/layerfs-api/tests/machine_force.rs b/api/crates/layerfs-api/tests/machine_force.rs index 720de2fd68..533c1fd726 100644 --- a/api/crates/layerfs-api/tests/machine_force.rs +++ b/api/crates/layerfs-api/tests/machine_force.rs @@ -200,17 +200,25 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { std::fs::set_permissions(&home, std::fs::Permissions::from_mode(0o700)).unwrap(); home::open(&home).unwrap(); // A started Sandbox whose runtime record is missing: the stop is refused - // before anything is signalled, so nothing about it is uncertain. - write_record( - &paths::sandbox_record(&home, "lost"), - &SdkValue::map([ - ("name", t("lost")), - ("id", t("lost")), + // before anything is signalled, so nothing about it is uncertain, and the + // stop goes on to the next Sandbox (one never started, so nothing runs). + for (name, id) in [("a-lost", Some("a-lost")), ("b-idle", None)] { + let mut fields = vec![ + ("name", t(name)), ("machine", t("native")), ("project", t("shop/app")), - ]), - ) - .unwrap(); + ]; + if let Some(id) = id { + fields.push(("id", t(id))); + } + let record = SdkValue::Map( + fields + .into_iter() + .map(|(field, value)| (field.to_owned(), value)) + .collect(), + ); + write_record(&paths::sandbox_record(&home, name), &record).unwrap(); + } let refused = call( &home, "machine.stop", @@ -218,6 +226,14 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { ) .unwrap_err(); assert_eq!(refused.code, "Unavailable", "{refused:?}"); + assert_eq!( + refused.detail.get("refused_sandboxes"), + Some(&SdkValue::List(vec![t("a-lost")])) + ); + assert_eq!( + refused.detail.get("stopped_sandboxes"), + Some(&SdkValue::List(vec![t("b-idle")])) + ); let op = read_record(&paths::machine_record(&home, "native")) .unwrap() .and_then(|record| record.get("op").cloned()); @@ -225,7 +241,7 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { op.is_none(), "a refusal before any signal left an op on native: {op:?}" ); - let sandbox = read_record(&paths::sandbox_record(&home, "lost")) + let sandbox = read_record(&paths::sandbox_record(&home, "a-lost")) .unwrap() .unwrap(); for field in ["machine_stopped", "machine_stop_unknown"] { diff --git a/api/crates/layerfs-api/tests/machine_force_native.rs b/api/crates/layerfs-api/tests/machine_force_native.rs index 16a274ffed..99bf29932d 100644 --- a/api/crates/layerfs-api/tests/machine_force_native.rs +++ b/api/crates/layerfs-api/tests/machine_force_native.rs @@ -1,6 +1,6 @@ //! Live native machine force stop and Sandbox force close with mounted, executing Workspaces. #![cfg(all(target_os = "macos", feature = "machine"))] -use layerfs_api::home::{self, paths, read_record}; +use layerfs_api::home::{self, paths, read_record, write_record}; use layerfs_bridge::control::SdkValue; use layerfs_bridge::process::LocalProcess; use std::{ @@ -161,6 +161,57 @@ fn live_case(home: &Path, name: &str, machine: bool) { ); assert_eq!(saved.get_text("text"), Some("saved\n")); } +/// A recorded pid that now names an unrelated live process: the forced stop +/// refuses that Sandbox before any signal, leaves no op on `native`, and +/// signals neither process. +fn identity_case(home: &Path) { + call( + home, + "sandbox.create", + &[ + ("project", t("shop/app")), + ("name", t("moved")), + ("machine", t("native")), + ], + ); + let path = paths::sandbox_record(home, "moved"); + let original = read_record(&path).unwrap().unwrap(); + let daemon = original.get("daemon").unwrap().clone(); + let real = LocalProcess::observe(daemon.get_int("pid").unwrap() as u32).unwrap(); + let mut other = Helper(Command::new("/bin/sleep").arg("30").spawn().unwrap()); + let moved = home::with_field(daemon, "pid", SdkValue::Int(i64::from(other.0.id()))); + write_record(&path, &home::with_field(original.clone(), "daemon", moved)).unwrap(); + let refused = layerfs_api::call( + Some(home), + "machine.stop", + SdkValue::map([("name", t("native")), ("force", SdkValue::Bool(true))]), + &mut |_| {}, + ) + .unwrap_err(); + assert!( + format!("{:?}", refused.detail).contains("moved"), + "refused_sandboxes names it: {refused:?}" + ); + assert!( + other.0.try_wait().unwrap().is_none(), + "the unrelated process was signalled" + ); + assert!(real.alive(), "the real daemon was signalled"); + let op = read_record(&paths::machine_record(home, "native")) + .unwrap() + .and_then(|record| record.get("op").cloned()); + assert!( + op.is_none(), + "a refusal before any signal left {op:?} on native" + ); + write_record(&path, &original).unwrap(); + call( + home, + "sandbox.close", + &[("name", t("moved")), ("force", SdkValue::Bool(true))], + ); + assert!(!real.alive(), "the restored record's daemon ended"); +} #[test] fn native_force_stops_live_daemon_and_command_and_detaches_mount_without_opening_store() { let Ok(daemon) = std::env::var("LAYERFS_DAEMON") else { @@ -200,5 +251,6 @@ fn native_force_stops_live_daemon_and_command_and_detaches_mount_without_opening home::import_env(&home, &[("LAYERFS_DAEMON".to_owned(), daemon)]).unwrap(); live_case(&home, "live", true); live_case(&home, "close", false); + identity_case(&home); std::fs::remove_dir_all(&home).unwrap(); } diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs index cdf16776c4..be49fb1998 100644 --- a/control/crates/layerfs-control/src/machine_force.rs +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -58,13 +58,24 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { }; let mut unknown = Vec::new(); let mut stopped = Vec::new(); + let mut skipped = Vec::new(); let mut native_attempt = None; let attempted = (|| -> Result<()> { if spec.kind == Platform::Native { + let mut first_refusal = None; for sandbox in &sandboxes { - // A refusal before the signal leaves this runtime as it was: - // only a sent stop makes its outcome uncertain. - if let Some(mut runtime) = native_runtime(&home, sandbox)? { + // A refusal before the signal leaves this runtime as it was, + // so the stop goes on to the next Sandbox: only a sent stop + // makes an outcome uncertain. + let runtime = match native_runtime(&home, sandbox) { + Ok(runtime) => runtime, + Err(refusal) => { + skipped.push(sandbox.name().to_owned()); + first_refusal.get_or_insert(refusal); + continue; + } + }; + if let Some(mut runtime) = runtime { native_attempt = Some(sandbox); runtime.stop(0).map_err(failed)?; } @@ -73,6 +84,9 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { stopped.push(sandbox.name().to_owned()); unknown.extend(workspaces(&home, sandbox)?); } + if let Some(refusal) = first_refusal { + return Err(refusal); + } } else { adapters::stop_force(&spec).map_err(refusal)?; for sandbox in &sandboxes { @@ -91,11 +105,22 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { if let Err(mut error) = attempted { // Native Sandboxes are recorded one by one as they stop, so only the // one being signalled is uncertain; a provider stop covers them all. - let uncertain = error.code == "Unknown" - || native_attempt.is_some() - || (spec.kind != Platform::Native && !stopped.is_empty()) - || (spec.kind != Platform::Native && error.code != "Unsupported"); - { + let uncertain = match spec.kind { + Platform::Native => native_attempt.is_some(), + _ => error.code != "Unsupported" || !stopped.is_empty(), + }; + // Recording the outcome must not replace the stop's own refusal: a + // failure here is reported beside it. + let mut custody = Vec::new(); + let mut keep = |result: Result<()>| { + if let Err(failure) = result { + custody.push(SdkValue::text(format!( + "{}: {}", + failure.code, failure.message + ))); + } + }; + keep((|| { let _home = lock::take(&home)?; for machine in std::iter::once(name).chain(hosted.iter().map(String::as_str)) { update(&home, machine, |record| { @@ -106,30 +131,40 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { } })?; } - } + Ok(()) + })()); // A multi-container provider stop may have completed only part of its work. // Keep that uncertainty distinct from both a running or a confirmed gone runtime. if spec.kind != Platform::Native && error.code != "Unsupported" { for sandbox in &sandboxes { if !stopped.iter().any(|name| name == sandbox.name()) { - mark(&home, sandbox, false)?; + keep(mark(&home, sandbox, false)); } } } else if let Some(sandbox) = native_attempt { if !stopped.iter().any(|name| name == sandbox.name()) { - mark(&home, sandbox, false)?; + keep(mark(&home, sandbox, false)); } } - error.detail = home::with_field( - error.detail, - "stopped_sandboxes", - SdkValue::List(stopped.into_iter().map(SdkValue::Text).collect()), - ); + for (key, list) in [ + ("stopped_sandboxes", stopped), + ("refused_sandboxes", skipped), + ] { + error.detail = home::with_field( + error.detail, + key, + SdkValue::List(list.into_iter().map(SdkValue::Text).collect()), + ); + } error.detail = home::with_field( error.detail, "unknown", SdkValue::List(unknown.into_iter().map(SdkValue::Text).collect()), ); + if !custody.is_empty() { + error.detail = + home::with_field(error.detail, "custody_errors", SdkValue::List(custody)); + } return Err(error); } { @@ -170,14 +205,15 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { Ok(out) } -/// A new explicit force call may reconcile its own previous unknown stop; -/// it never silently retries, nor clears another kind of unresolved operation. +/// A new explicit force call may reconcile an unknown stop that this +/// process left, or one whose owner has ended (`Unresolved`); one a live +/// process still holds stays `Busy`. It never silently retries, nor clears +/// another kind of unresolved operation. fn force_record(home: &Path, name: &str) -> Result { let record = MachineRecord::of(home, name)?; - if record - .op() - .is_some_and(|op| op.name() == "machine.stop" && op.uncertain()) - { + if record.op().is_some_and(|op| { + op.name() == "machine.stop" && op.uncertain() && (op.mine() || !op.owner_alive()) + }) { return Ok(record); } record.not_held()?; @@ -203,8 +239,8 @@ fn mark(home: &Path, sandbox: &SandboxRecord, stopped: bool) -> Result<()> { Ok(()) } -/// Native is the host itself: stop only this home's recorded runtime processes. -/// The Sandbox's recorded native runtime, reopened for a stop without +/// Native is the host itself: only this home's recorded runtime processes +/// stop. The Sandbox's recorded native runtime, reopened for a stop without /// sending anything; `None` when there is nothing left to stop. fn native_runtime( home: &Path, diff --git a/control/crates/layerfs-sandbox/src/close.rs b/control/crates/layerfs-sandbox/src/close.rs index 7974b2570c..e9e1321373 100644 --- a/control/crates/layerfs-sandbox/src/close.rs +++ b/control/crates/layerfs-sandbox/src/close.rs @@ -108,7 +108,8 @@ pub fn close(home: &Path, events: &mut Events<'_>, request: &CloseRequest) -> Re // a forced close on `native` needs no Store or daemon discovery, // even when active mounts or a broken Store would block the // ordinary route - crate::force::native_host(&home, name) + crate::force::native_free(&home) + .and_then(|()| crate::force::native_host(&home, name)) .and_then(|host| crate::force::close(&host, held.as_deref().unwrap_or(&[]))) } else { on_machine( @@ -148,10 +149,16 @@ pub fn close(home: &Path, events: &mut Events<'_>, request: &CloseRequest) -> Re op.log_uncertain(&home, "sandbox", name, &refusal.message); Err(refusal) } - // a certain failure leaves the Sandbox as it was + // a certain failure leaves the Sandbox as it was, except a forced + // close whose runtime already stopped: the record says so Err(refusal) => { if let Some(now) = SandboxRecord::read(&home, name)? { - now.without_op().write(&home)?; + let stopped = refusal.detail.get_bool(crate::force::RUNTIME_STOPPED); + match stopped { + true => now.without_op().with_machine_stop(true), + false => now.without_op(), + } + .write(&home)?; } Err(refusal) } diff --git a/control/crates/layerfs-sandbox/src/force.rs b/control/crates/layerfs-sandbox/src/force.rs index 2ae081f1c5..0d91b32cb3 100644 --- a/control/crates/layerfs-sandbox/src/force.rs +++ b/control/crates/layerfs-sandbox/src/force.rs @@ -30,6 +30,19 @@ pub fn native_host(home: &Path, sandbox: &str) -> Result { }) } +/// The refusal detail field a forced close sets once its runtime stopped. +pub const RUNTIME_STOPPED: &str = "runtime_stopped"; + +/// Refuses while a live process holds `native` (a forced `machine.stop` in +/// progress, for one); an unresolved intent left by an ended process does not +/// stop a forced close. +pub fn native_free(home: &Path) -> Result<(), SdkRefusal> { + match layerfs_machine::record::MachineRecord::read(home, "native")? { + Some(machine) => machine.not_running(), + None => Ok(()), + } +} + /// Forced `sandbox.close` of the Sandbox `host` names, on this process's /// kernel: no daemon control call, Store open, active-Workspace refusal or /// retry. The backend's Stop checks the runtime identity and ends its @@ -57,15 +70,22 @@ pub fn close(host: &Host, held: &[String]) -> Outcome { } } crate::reopen::recorded(host, &id, &value)?.stop_now()?; - crate::reopen::recorded(host, &id, &value)? + // The runtime is confirmed stopped from here on: a later failure says so, + // so the record shows it stopped instead of as it was. + let after_stop = |mut refusal: SdkRefusal| { + refusal.detail = home::with_field(refusal.detail, RUNTIME_STOPPED, SdkValue::Bool(true)); + refusal + }; + crate::reopen::recorded(host, &id, &value) + .map_err(after_stop)? .stopped() .delete() - .map_err(failed)?; + .map_err(|error| after_stop(failed(error)))?; crate::registry::SANDBOXES .lock() - .map_err(|_| refused("Failed", "the Sandbox table's lock was poisoned"))? + .map_err(|_| after_stop(refused("Failed", "the Sandbox table's lock was poisoned")))? .remove(&directory); - std::fs::remove_dir_all(&directory).map_err(failed)?; + std::fs::remove_dir_all(&directory).map_err(|error| after_stop(failed(error)))?; Ok(SdkValue::map([ ("kind", SdkValue::text("Closed")), ("sandbox", SdkValue::text(id)), diff --git a/docs/architecture/api/sdk/machine.md b/docs/architecture/api/sdk/machine.md index 53e8875600..29864239ae 100644 --- a/docs/architecture/api/sdk/machine.md +++ b/docs/architecture/api/sdk/machine.md @@ -222,7 +222,9 @@ CLI layerfs machine stop [--force] `apple-container` end the worker and VM. `worker.json` is deleted and the state recorded `stopped`. Nothing stops a machine because it is unused. Errors: `InUse` while Sandboxes are recorded on it and `force` is not set -(`detail.related`, `can_stop_machine`); `Unavailable` (`start.lock` held 600 s, +(`detail.related`, `can_stop_machine`); `InvalidInput` on `docker` when the +container named for the machine is not a LayerFS worker (its role label), which +is left alone; `Unavailable` (`start.lock` held 600 s, or the provider did not answer: records stay as they were); `Unknown` (a provider command timed out: the machine may be stopped while still recorded with its worker). @@ -242,8 +244,13 @@ not be listed; these are not saved Commits). Stopped Sandboxes are recorded `gone` for explicit reclaim, or `unresolved` when the stop could not be confirmed. On `native` a Sandbox is uncertain only once its stop has been sent: a refusal before that (no recorded runtime, or a recorded process -identity that no longer matches) leaves that Sandbox and the machine as they -were, with no op left on `native`. A provider failure or uncertain outcome is +identity that no longer matches) leaves that Sandbox as it was, and the stop +goes on to the next one. The call then answers the first such refusal, with +`detail.refused_sandboxes`, `stopped_sandboxes` and `unknown`, and leaves no op +on `native`. If recording the outcome fails, the stop's own refusal is still +answered, with `detail.custody_errors`. A later forced stop may settle an +uncertain stop this process left, or one whose process has ended; one a live +process still holds is `Busy`. A provider failure or uncertain outcome is reported, never replayed or turned into success ([`machine_force.rs`](../../../../control/crates/layerfs-control/src/machine_force.rs)). diff --git a/docs/architecture/api/sdk/sandbox.md b/docs/architecture/api/sdk/sandbox.md index 0816b93c97..c51a7d7d1b 100644 --- a/docs/architecture/api/sdk/sandbox.md +++ b/docs/architecture/api/sdk/sandbox.md @@ -152,6 +152,11 @@ On `native`, a forced close works from the Sandbox's own records instead: it opens no Store and calls no daemon, checks the runtime's identity, ends its mounts and processes, and answers every recorded or held Workspace as `unknown` ([`force.rs`](../../../../control/crates/layerfs-sandbox/src/force.rs)). +It does not settle commands one by one or check mounts first: the runtime's +stop ends them. It is `Busy` while a live process holds `native` (a forced +`machine.stop`, for one), but not for an intent an ended process left. If the +runtime stopped and a later step (delete, folder removal) failed, the refusal +carries `detail.runtime_stopped: true` and the record is marked stopped. | Case | Answer | | --- | --- | diff --git a/docs/architecture/control/sandboxes/README.md b/docs/architecture/control/sandboxes/README.md index 6d420a489e..bc5bd7ce38 100644 --- a/docs/architecture/control/sandboxes/README.md +++ b/docs/architecture/control/sandboxes/README.md @@ -64,6 +64,7 @@ runtime owns command launch, streams, exit status and cancel | Direct route | A Sandbox whose daemon port its machine forwards to this host (`lima`'s `ssh -L`) keeps its keys in the home's `control.json` (0600); its Workspace calls dial the daemon from the home, while exec still goes through the worker. | [`direct.rs`](../../../../control/crates/layerfs-sandbox/src/direct.rs) | | `sandbox.get` capacity | Reads the daemon's `backend.daemon.connection_failures` on a lent connection, bounded to 2 s and fenced at that deadline. | [`daemon/capacity.rs`](../../../../control/crates/layerfs-sandbox/src/daemon/capacity.rs), [`get.rs`](../../../../control/crates/layerfs-sandbox/src/get.rs) | | Retired docker records | A record whose `backend` or `runtime` is `docker`, or a started docker-machine record with no `runtime`: every call but `sandbox.close` is `Unsupported`, naming `docker rm -f `; `sandbox.close` forgets it. | [`record/sandbox.rs`](../../../../control/crates/layerfs-sandbox/src/record/sandbox.rs) `retired_container`, [`reopen.rs`](../../../../control/crates/layerfs-sandbox/src/reopen.rs) `retired`, [`close.rs`](../../../../control/crates/layerfs-sandbox/src/close.rs) `forget` | +| Machine-stop fields (record format) | A forced `machine.stop` adds two Bool fields to the Sandbox's `record.json`: `machine_stopped` (its runtime was stopped with the machine; shown `gone`) and `machine_stop_unknown` (the stop was not confirmed; shown `unresolved`). An absent field reads `false`, so older records need no change and `"v"` stays `1`. A build older than these fields keeps them as read but shows such a Sandbox as it was before the stop. | [`record/sandbox.rs`](../../../../control/crates/layerfs-sandbox/src/record/sandbox.rs) `machine_stopped`, `with_machine_stop`; [`machine_force.rs`](../../../../control/crates/layerfs-control/src/machine_force.rs) `mark` | ## Invariants diff --git a/docs/architecture/engine/store/sqlite.md b/docs/architecture/engine/store/sqlite.md index 13994b1060..3bf0540a2b 100644 --- a/docs/architecture/engine/store/sqlite.md +++ b/docs/architecture/engine/store/sqlite.md @@ -56,7 +56,7 @@ UPDATE and DELETE on `pack` and `pack_unit`. Open checks `application_id`, | Guard | `busy_timeout=0`; `SQLITE_DBCONFIG_DEFENSIVE` on | same | | Settings | `profile::apply`, then read back into `ConnectionProfile`; a mismatch is `Integrity`; an existing Store not in `wal` mode is refused | [`profile.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/profile.rs) | | Checkpoint thread | writable Disposable session only: a second connection on its own thread; failure to start it refuses the open | [`background.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs) | -| Last close | Dropping a writable session takes `Writer::acquire(path)` (or reuses the one an uncertain transaction already holds) around the main connection's close, which may be SQLite's last and checkpoint or remove `-wal`/`-shm`; the checkpoint thread's connection closes first and never takes it. If that wait fails, the connection is kept until process exit rather than torn down without the lane or retried. A read-only or diagnostic session cannot checkpoint or remove those files, so its close takes no lane and never waits behind a writer kept after an uncertain write. One exception: an open whose checkpoint thread fails to start drops its new connection after the open's lane is released | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | +| Last close | A writable session's close makes one non-blocking `Writer::try_acquire(path)` (or reuses the lane an uncertain transaction already holds). With the lane, the main connection's close may be SQLite's last and checkpoint or remove `-wal`/`-shm`. Without it (held elsewhere, or the attempt failed), the close sets `SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE` first, so SQLite takes no exclusive lock, runs no checkpoint and removes nothing; the next holder of the lane finds the WAL. A close never waits, so a writer kept after an uncertain write cannot hang another session's close. If even that setting fails, the connection is kept until process exit. A read-only or diagnostic session cannot checkpoint or remove those files and takes no lane. The checkpoint thread's connection closes first and never takes it. One exception: an open whose checkpoint thread fails to start drops its new connection after the open's lane is released | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | | Setting | Disposable | Source | | --- | --- | --- | diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs index 1689725a49..52115caca3 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs @@ -1,6 +1,7 @@ //! Connection custody at the last Session close, including quarantined owners. use super::connection::State; use crate::backend::sqlite::lane::writer_admission::Writer; +use rusqlite::config::DbConfig; use rusqlite::Connection; use std::ops::Deref; @@ -38,28 +39,37 @@ impl Drop for State { if self.connection.is_closed() { return; } - // An uncertain transaction already owns the lane. Taking it again - // through an independent directory descriptor would deadlock. let writer = match (self.writer_hold.take(), &self.close_lane) { + // An uncertain transaction already owns the lane. Taking it again + // through an independent directory descriptor would deadlock. (Some(writer), _) => Some(writer), // A read-only or diagnostic connection cannot checkpoint or remove - // the WAL and SHM files, so its close takes no lane: it must not - // wait behind a writer another session keeps after an uncertain - // write. + // the WAL and SHM files, so its close takes no lane. (None, None) => None, - (None, Some(path)) => match Writer::acquire(path) { - Ok(writer) => Some(writer), - Err(_) => { - // Drop cannot report a cancelled/failed admission. Retain - // the SQLite connection until process exit instead of - // attempting unadmitted WAL teardown or retrying the wait. - std::mem::forget(self.connection.take()); - return; + // A writable close makes one non-blocking attempt: it never waits, + // not even behind a writer another session keeps after an + // uncertain write. Without the lane it closes with SQLite's + // checkpoint-on-close turned off, so it takes no exclusive lock + // and leaves the WAL and SHM files for the lane's next holder. + (None, Some(path)) => match Writer::try_acquire(path) { + Ok(Some(writer)) => Some(writer), + Ok(None) | Err(_) => { + let quiet = self + .connection + .set_db_config(DbConfig::SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, true); + if quiet.is_err() { + // Drop cannot report it: retain the connection until + // process exit rather than tear the WAL down unadmitted. + std::mem::forget(self.connection.take()); + return; + } + None } }, }; - // The background connection has already ended; a writable close may be - // SQLite's last connection and can checkpoint/delete the WAL and SHM. + // The background connection has already ended; a writable close that + // holds the lane may be SQLite's last and checkpoint/delete the WAL + // and SHM files. drop(self.connection.take()); drop(writer); } diff --git a/engine/crates/layerfs-persistence/tests/writer_admission.rs b/engine/crates/layerfs-persistence/tests/writer_admission.rs index 10768f3451..401fcc2f55 100644 --- a/engine/crates/layerfs-persistence/tests/writer_admission.rs +++ b/engine/crates/layerfs-persistence/tests/writer_admission.rs @@ -354,29 +354,30 @@ fn another_process_admission_precedes_the_single_sqlite_begin() { #[test] #[cfg(any(target_os = "linux", target_os = "macos"))] -fn another_process_admission_precedes_last_connection_close() { +fn writable_last_close_without_the_lane_neither_waits_nor_tears_down_the_wal() { let temp = support::Temp::new("wa-close"); let path = temp.join("store.sqlite"); + let wal = temp.join("store.sqlite-wal"); let handles = create(&path); + assert!(wal.exists(), "the test needs a WAL file before the close"); let held = Held::start(&path, &temp.join("s")); - let (ready_tx, ready_rx) = mpsc::channel(); let (result_tx, result_rx) = mpsc::channel(); let closer = std::thread::spawn(move || { - ready_tx.send(()).unwrap(); drop(handles); result_tx.send(()).unwrap(); }); - ready_rx.recv_timeout(Duration::from_secs(2)).unwrap(); - let premature = result_rx.recv_timeout(Duration::from_millis(100)).ok(); + let closed = result_rx.recv_timeout(Duration::from_secs(2)).ok(); + let wal_kept = wal.exists(); held.release(); - if premature.is_none() { + if closed.is_none() { result_rx.recv_timeout(Duration::from_secs(3)).unwrap(); } closer.join().unwrap(); - assert!( - premature.is_none(), - "last connection close bypassed its lifecycle owner" - ); + assert!(closed.is_some(), "a writable close waited for the lane"); + assert!(wal_kept, "a close without the lane removed the WAL"); + // With the lane free, the last close checkpoints and removes it. + drop(open(&path)); + assert!(!wal.exists(), "an admitted last close left the WAL"); } #[test] From 0fd2a99af0c0d0dec22492bcf05b0aed46d8aaff Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Sat, 14 Nov 2026 17:41:20 +0800 Subject: [PATCH 5/6] fix(store, machine, sandbox): writable closes wait again; never-started Sandboxes are not stopped Follows the independent review of #656 at b4a028321e. - N2 (regression in b4a028321e): a writable session's last close waits for the lane again, so it checkpoints and removes -wal/-shm under the lane. Closing without it whenever the lane was briefly held left the WAL behind, which bucket pull's verify close reports as a failure and backup restore's in-use check as InUse. Finding A (a writable close waits behind a writer kept after an uncertain write) is therefore kept by design: every write behind that writer already waits; read-only and diagnostic closes still take no lane. Only a failed wait (interrupted, or the directory cannot be opened) closes with SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE (no exclusive lock, no checkpoint, no unlink; no second attempt), replacing the connection leak (finding 3). - N3: the transfer leftover-WAL comment names the failed-wait close too; control open.rs's comment holds again as written. - N5: a native Sandbox that never started (no id, no runtime record) is neither marked stopped nor listed in stopped_sandboxes. - N6: a forced native sandbox.close is not refused for an unresolved machine.stop intent this process left (it already was not for one an ended process left). - D: the live identity case asserts refused_sandboxes exactly. Tests: - Replaced writer_admission writable_last_close_without_the_lane_neither_waits_nor_tears_down_the_wal (added by this PR in b4a028321e; it asserted the behaviour reverted here) with writable_last_close_waits_for_the_lane_then_removes_the_wal. - api machine_force native_force_stop_refused_before_any_signal_leaves_no_uncertain_op now uses a refused, an already stopped and a never-started Sandbox. Docs: engine/store/sqlite.md (Last close), api/sdk/machine.md, api/sdk/sandbox.md. Checks (toolchain 1.85.1, registered worktree via tools/worktree.py): - Clippy --all-targets -D warnings, macOS and Linux, on persistence, sandbox, sandbox-backend (Linux), control, platform, machine (Linux), api (machine, postgresql), store, cli (postgresql), daemon: clean. - macOS: persistence writer_admission, sqlite_lock_custody, idle_checkpoint; store lease_race, lease_stale; platform machine_force, machine_lima; cli capabilities, stream_framed; api machine_force, sandbox_records, sandbox_lost, sandbox_family, sandbox_capacity_worker: 57 passed, 0 failed. machine_force_native live with LAYERFS_DAEMON (debug macOS daemon of this tree): 2 passed in 1.9 s, no daemon left. - Linux: persistence, store, platform machine_force, api machine_force, sandbox_lost, sandbox_capacity_worker: 38 passed, 0 failed. - Linux mount lane (45 binaries, each in a fresh container with /dev/fuse, SYS_ADMIN, fresh ext4 /tmp, timeout 120, --test-threads=1): 45/45, 116 passed, 0 failed, 3 ignored by their own attributes. - Not run: live Docker/Lima providers. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Production LOC: 161173 -> 161187 (delta +14); core 39787 -> 39787, engine 74818 -> 74818, control 40588 -> 40602, api 5980 -> 5980. Reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Method: tools/production_loc.py (blob 35e4a6d601fa) --root --json on the parent and the staged tree. Co-Authored-By: Claude Opus 5.5 --- api/crates/layerfs-api/tests/machine_force.rs | 37 ++++++++++++------- .../layerfs-api/tests/machine_force_native.rs | 7 ++-- .../layerfs-control/src/machine_force.rs | 37 ++++++++++++++----- control/crates/layerfs-sandbox/src/force.rs | 13 ++++--- .../src/sqlite/transfer/steps.rs | 3 +- docs/architecture/api/sdk/machine.md | 2 +- docs/architecture/api/sdk/sandbox.md | 3 +- docs/architecture/engine/store/sqlite.md | 2 +- .../src/backend/sqlite/session/lifecycle.rs | 18 +++++---- .../tests/writer_admission.rs | 20 ++++++---- 10 files changed, 91 insertions(+), 51 deletions(-) diff --git a/api/crates/layerfs-api/tests/machine_force.rs b/api/crates/layerfs-api/tests/machine_force.rs index 533c1fd726..c20e077afb 100644 --- a/api/crates/layerfs-api/tests/machine_force.rs +++ b/api/crates/layerfs-api/tests/machine_force.rs @@ -199,10 +199,16 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { std::fs::create_dir_all(&home).unwrap(); std::fs::set_permissions(&home, std::fs::Permissions::from_mode(0o700)).unwrap(); home::open(&home).unwrap(); - // A started Sandbox whose runtime record is missing: the stop is refused - // before anything is signalled, so nothing about it is uncertain, and the - // stop goes on to the next Sandbox (one never started, so nothing runs). - for (name, id) in [("a-lost", Some("a-lost")), ("b-idle", None)] { + // a-lost: started, but its runtime record is missing, so its stop is + // refused before anything is signalled and nothing about it is uncertain. + // b-done: already stopped with its machine; the stop goes on to it. + // c-new: never started, so nothing is stopped or recorded for it. + let sandboxes = [ + ("a-lost", Some("a-lost"), false), + ("b-done", Some("b-done"), true), + ("c-new", None, false), + ]; + for (name, id, stopped) in sandboxes { let mut fields = vec![ ("name", t(name)), ("machine", t("native")), @@ -211,6 +217,9 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { if let Some(id) = id { fields.push(("id", t(id))); } + if stopped { + fields.push(("machine_stopped", SdkValue::Bool(true))); + } let record = SdkValue::Map( fields .into_iter() @@ -232,7 +241,7 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { ); assert_eq!( refused.detail.get("stopped_sandboxes"), - Some(&SdkValue::List(vec![t("b-idle")])) + Some(&SdkValue::List(vec![t("b-done")])) ); let op = read_record(&paths::machine_record(&home, "native")) .unwrap() @@ -241,14 +250,16 @@ fn native_force_stop_refused_before_any_signal_leaves_no_uncertain_op() { op.is_none(), "a refusal before any signal left an op on native: {op:?}" ); - let sandbox = read_record(&paths::sandbox_record(&home, "a-lost")) - .unwrap() - .unwrap(); - for field in ["machine_stopped", "machine_stop_unknown"] { - assert!( - sandbox.get(field).is_none(), - "an unsignalled Sandbox was marked {field}: {sandbox:?}" - ); + for name in ["a-lost", "c-new"] { + let sandbox = read_record(&paths::sandbox_record(&home, name)) + .unwrap() + .unwrap(); + for field in ["machine_stopped", "machine_stop_unknown"] { + assert!( + sandbox.get(field).is_none(), + "an unsignalled Sandbox was marked {field}: {sandbox:?}" + ); + } } std::fs::remove_dir_all(root).unwrap(); } diff --git a/api/crates/layerfs-api/tests/machine_force_native.rs b/api/crates/layerfs-api/tests/machine_force_native.rs index 99bf29932d..2bdf3702ea 100644 --- a/api/crates/layerfs-api/tests/machine_force_native.rs +++ b/api/crates/layerfs-api/tests/machine_force_native.rs @@ -188,9 +188,10 @@ fn identity_case(home: &Path) { &mut |_| {}, ) .unwrap_err(); - assert!( - format!("{:?}", refused.detail).contains("moved"), - "refused_sandboxes names it: {refused:?}" + assert_eq!( + refused.detail.get("refused_sandboxes"), + Some(&SdkValue::List(vec![t("moved")])), + "{refused:?}" ); assert!( other.0.try_wait().unwrap().is_none(), diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs index be49fb1998..72b5ab9b12 100644 --- a/control/crates/layerfs-control/src/machine_force.rs +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -75,9 +75,14 @@ pub(crate) fn stop(home: &Path, name: &MachineName) -> Outcome { continue; } }; - if let Some(mut runtime) = runtime { - native_attempt = Some(sandbox); - runtime.stop(0).map_err(failed)?; + match runtime { + // Nothing ever ran: nothing is stopped or recorded. + Native::NeverStarted => continue, + Native::Stopped => {} + Native::Stop(mut runtime) => { + native_attempt = Some(sandbox); + runtime.stop(0).map_err(failed)?; + } } mark(&home, sandbox, true)?; native_attempt = None; @@ -242,12 +247,19 @@ fn mark(home: &Path, sandbox: &SandboxRecord, stopped: bool) -> Result<()> { /// Native is the host itself: only this home's recorded runtime processes /// stop. The Sandbox's recorded native runtime, reopened for a stop without /// sending anything; `None` when there is nothing left to stop. -fn native_runtime( - home: &Path, - sandbox: &SandboxRecord, -) -> Result>> { +/// What a forced stop finds for one native Sandbox. +enum Native { + /// Its recorded runtime, reopened for the stop; nothing sent yet. + Stop(Box), + /// Its runtime is already stopped. + Stopped, + /// It never started (no id and no runtime record). + NeverStarted, +} + +fn native_runtime(home: &Path, sandbox: &SandboxRecord) -> Result { if sandbox.machine_stopped() { - return Ok(None); + return Ok(Native::Stopped); } let host = layerfs_sandbox::force::native_host(home, sandbox.name())?; let Some(daemon) = files::started(&sandbox.dir(home))? else { @@ -261,9 +273,14 @@ fn native_runtime( )); } sandbox.free()?; - return Ok(None); + return Ok(Native::NeverStarted); }; - reopen::recorded(&host, sandbox.name(), &daemon)?.for_stop() + Ok( + match reopen::recorded(&host, sandbox.name(), &daemon)?.for_stop()? { + Some(runtime) => Native::Stop(runtime), + None => Native::Stopped, + }, + ) } /// These are lost/unknown uncommitted frontiers, never manufactured commits. diff --git a/control/crates/layerfs-sandbox/src/force.rs b/control/crates/layerfs-sandbox/src/force.rs index 0d91b32cb3..3562a9090e 100644 --- a/control/crates/layerfs-sandbox/src/force.rs +++ b/control/crates/layerfs-sandbox/src/force.rs @@ -34,12 +34,15 @@ pub fn native_host(home: &Path, sandbox: &str) -> Result { pub const RUNTIME_STOPPED: &str = "runtime_stopped"; /// Refuses while a live process holds `native` (a forced `machine.stop` in -/// progress, for one); an unresolved intent left by an ended process does not -/// stop a forced close. +/// progress, for one). An unresolved intent does not stop a forced close, +/// whether an ended process left it or this process did. pub fn native_free(home: &Path) -> Result<(), SdkRefusal> { - match layerfs_machine::record::MachineRecord::read(home, "native")? { - Some(machine) => machine.not_running(), - None => Ok(()), + let Some(machine) = layerfs_machine::record::MachineRecord::read(home, "native")? else { + return Ok(()); + }; + match machine.op() { + Some(op) if op.mine() && op.uncertain() => Ok(()), + _ => machine.not_running(), } } diff --git a/control/crates/layerfs-store/src/sqlite/transfer/steps.rs b/control/crates/layerfs-store/src/sqlite/transfer/steps.rs index 4ecc4981df..ef250836ef 100644 --- a/control/crates/layerfs-store/src/sqlite/transfer/steps.rs +++ b/control/crates/layerfs-store/src/sqlite/transfer/steps.rs @@ -61,7 +61,8 @@ fn digest(file: &Path, progress: Option<&mut dyn FnMut(u64, u64)>) -> Outcome { ])) } -/// Checkpoints and closes a crashed opener's leftover `-wal` beside `file`, +/// Checkpoints and closes a leftover `-wal` beside `file` (a crashed opener's, +/// or one whose close could not wait for the writer lane), /// through the same `seal` a graceful close takes (#86): a Store whose last /// opener was killed keeps its committed frames in the log, which blocks a /// snapshot until one opener folds them in. Nothing runs while no live diff --git a/docs/architecture/api/sdk/machine.md b/docs/architecture/api/sdk/machine.md index 29864239ae..a73b553941 100644 --- a/docs/architecture/api/sdk/machine.md +++ b/docs/architecture/api/sdk/machine.md @@ -242,7 +242,7 @@ probe, plus `forced: true`, `stopped_sandboxes: [name]` and `unknown: [workspace]` (`sandbox:*` when a worker-side Workspace frontier could not be listed; these are not saved Commits). Stopped Sandboxes are recorded `gone` for explicit reclaim, or `unresolved` when the stop could not be -confirmed. On `native` a Sandbox is uncertain only once its stop has been +confirmed. A Sandbox that never started is neither stopped nor listed. On `native` a Sandbox is uncertain only once its stop has been sent: a refusal before that (no recorded runtime, or a recorded process identity that no longer matches) leaves that Sandbox as it was, and the stop goes on to the next one. The call then answers the first such refusal, with diff --git a/docs/architecture/api/sdk/sandbox.md b/docs/architecture/api/sdk/sandbox.md index c51a7d7d1b..166f653851 100644 --- a/docs/architecture/api/sdk/sandbox.md +++ b/docs/architecture/api/sdk/sandbox.md @@ -154,7 +154,8 @@ mounts and processes, and answers every recorded or held Workspace as `unknown` ([`force.rs`](../../../../control/crates/layerfs-sandbox/src/force.rs)). It does not settle commands one by one or check mounts first: the runtime's stop ends them. It is `Busy` while a live process holds `native` (a forced -`machine.stop`, for one), but not for an intent an ended process left. If the +`machine.stop`, for one), but not for an unresolved intent, whether an ended +process or this one left it. If the runtime stopped and a later step (delete, folder removal) failed, the refusal carries `detail.runtime_stopped: true` and the record is marked stopped. diff --git a/docs/architecture/engine/store/sqlite.md b/docs/architecture/engine/store/sqlite.md index 3bf0540a2b..855eba95c0 100644 --- a/docs/architecture/engine/store/sqlite.md +++ b/docs/architecture/engine/store/sqlite.md @@ -56,7 +56,7 @@ UPDATE and DELETE on `pack` and `pack_unit`. Open checks `application_id`, | Guard | `busy_timeout=0`; `SQLITE_DBCONFIG_DEFENSIVE` on | same | | Settings | `profile::apply`, then read back into `ConnectionProfile`; a mismatch is `Integrity`; an existing Store not in `wal` mode is refused | [`profile.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/profile.rs) | | Checkpoint thread | writable Disposable session only: a second connection on its own thread; failure to start it refuses the open | [`background.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/background.rs) | -| Last close | A writable session's close makes one non-blocking `Writer::try_acquire(path)` (or reuses the lane an uncertain transaction already holds). With the lane, the main connection's close may be SQLite's last and checkpoint or remove `-wal`/`-shm`. Without it (held elsewhere, or the attempt failed), the close sets `SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE` first, so SQLite takes no exclusive lock, runs no checkpoint and removes nothing; the next holder of the lane finds the WAL. A close never waits, so a writer kept after an uncertain write cannot hang another session's close. If even that setting fails, the connection is kept until process exit. A read-only or diagnostic session cannot checkpoint or remove those files and takes no lane. The checkpoint thread's connection closes first and never takes it. One exception: an open whose checkpoint thread fails to start drops its new connection after the open's lane is released | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | +| Last close | A writable session's close waits for `Writer::acquire(path)` like any write (or reuses the lane an uncertain transaction already holds), so its close, which may be SQLite's last, checkpoints and removes `-wal`/`-shm` under the lane; a pull's verify close and a restore's in-use check rely on that. If the wait itself fails (interrupted, or the directory cannot be opened), the close sets `SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE` first, so SQLite takes no exclusive lock, runs no checkpoint and removes nothing; there is no second attempt. If even that setting fails, the connection is kept until process exit. A read-only or diagnostic session cannot checkpoint or remove those files and takes no lane, so it never waits behind a writer kept after an uncertain write. The checkpoint thread's connection closes first and never takes it. One exception: an open whose checkpoint thread fails to start drops its new connection after the open's lane is released | [`lifecycle.rs`](../../../../engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs) | | Setting | Disposable | Source | | --- | --- | --- | diff --git a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs index 52115caca3..20a22f5e84 100644 --- a/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs +++ b/engine/crates/layerfs-persistence/src/backend/sqlite/session/lifecycle.rs @@ -46,14 +46,16 @@ impl Drop for State { // A read-only or diagnostic connection cannot checkpoint or remove // the WAL and SHM files, so its close takes no lane. (None, None) => None, - // A writable close makes one non-blocking attempt: it never waits, - // not even behind a writer another session keeps after an - // uncertain write. Without the lane it closes with SQLite's - // checkpoint-on-close turned off, so it takes no exclusive lock - // and leaves the WAL and SHM files for the lane's next holder. - (None, Some(path)) => match Writer::try_acquire(path) { - Ok(Some(writer)) => Some(writer), - Ok(None) | Err(_) => { + // A writable close waits for the lane like any write: its close + // may be SQLite's last, which checkpoints and removes the WAL and + // SHM files that a pull's verify and a restore's in-use check + // rely on. If the wait itself fails (interrupted, or the Store's + // directory cannot be opened), the close turns SQLite's + // checkpoint-on-close off first, so it takes no exclusive lock and + // removes nothing; there is no second attempt at the lane. + (None, Some(path)) => match Writer::acquire(path) { + Ok(writer) => Some(writer), + Err(_) => { let quiet = self .connection .set_db_config(DbConfig::SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, true); diff --git a/engine/crates/layerfs-persistence/tests/writer_admission.rs b/engine/crates/layerfs-persistence/tests/writer_admission.rs index 401fcc2f55..dcb888af97 100644 --- a/engine/crates/layerfs-persistence/tests/writer_admission.rs +++ b/engine/crates/layerfs-persistence/tests/writer_admission.rs @@ -354,7 +354,10 @@ fn another_process_admission_precedes_the_single_sqlite_begin() { #[test] #[cfg(any(target_os = "linux", target_os = "macos"))] -fn writable_last_close_without_the_lane_neither_waits_nor_tears_down_the_wal() { +fn writable_last_close_waits_for_the_lane_then_removes_the_wal() { + // A pull's verify and a restore's in-use check rely on the last close + // folding the WAL in: a writable close waits for a held lane instead of + // leaving the WAL behind. let temp = support::Temp::new("wa-close"); let path = temp.join("store.sqlite"); let wal = temp.join("store.sqlite-wal"); @@ -366,18 +369,19 @@ fn writable_last_close_without_the_lane_neither_waits_nor_tears_down_the_wal() { drop(handles); result_tx.send(()).unwrap(); }); - let closed = result_rx.recv_timeout(Duration::from_secs(2)).ok(); + let premature = result_rx.recv_timeout(Duration::from_millis(200)).ok(); let wal_kept = wal.exists(); held.release(); - if closed.is_none() { + if premature.is_none() { result_rx.recv_timeout(Duration::from_secs(3)).unwrap(); } closer.join().unwrap(); - assert!(closed.is_some(), "a writable close waited for the lane"); - assert!(wal_kept, "a close without the lane removed the WAL"); - // With the lane free, the last close checkpoints and removes it. - drop(open(&path)); - assert!(!wal.exists(), "an admitted last close left the WAL"); + assert!( + premature.is_none(), + "a writable close bypassed the held lane" + ); + assert!(wal_kept, "the WAL went before the close was admitted"); + assert!(!wal.exists(), "the admitted last close left the WAL"); } #[test] From d303c7b03c2b3b067f3501e556afefa3350539ec Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Sat, 14 Nov 2026 17:43:29 +0800 Subject: [PATCH 6/6] docs(machine): put native_runtime's doc comment back on the function Review finding M1 at 0fd2a99af0: the comment written for native_runtime sat on the Native enum and still described an Option. Comment text only. Checks: Clippy --all-targets -D warnings on layerfs-control (macOS) clean; rustfmt clean. Production LOC: 161187 -> 161187 (delta 0); comment-only change. Method: tools/production_loc.py (blob 35e4a6d601fa) --root --json on the parent and the staged tree. Co-Authored-By: Claude Opus 5.5 --- control/crates/layerfs-control/src/machine_force.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/control/crates/layerfs-control/src/machine_force.rs b/control/crates/layerfs-control/src/machine_force.rs index 72b5ab9b12..05b6cc3449 100644 --- a/control/crates/layerfs-control/src/machine_force.rs +++ b/control/crates/layerfs-control/src/machine_force.rs @@ -244,10 +244,8 @@ fn mark(home: &Path, sandbox: &SandboxRecord, stopped: bool) -> Result<()> { Ok(()) } -/// Native is the host itself: only this home's recorded runtime processes -/// stop. The Sandbox's recorded native runtime, reopened for a stop without -/// sending anything; `None` when there is nothing left to stop. -/// What a forced stop finds for one native Sandbox. +/// What a forced stop finds for one native Sandbox. Native is the host +/// itself: only this home's recorded runtime processes stop. enum Native { /// Its recorded runtime, reopened for the stop; nothing sent yet. Stop(Box), @@ -257,6 +255,8 @@ enum Native { NeverStarted, } +/// The Sandbox's recorded native runtime, reopened for a stop without sending +/// anything, or why there is nothing to stop. fn native_runtime(home: &Path, sandbox: &SandboxRecord) -> Result { if sandbox.machine_stopped() { return Ok(Native::Stopped);