diff --git a/.agents/skills/layerfs-js-sdk/references/methods/workspace.md b/.agents/skills/layerfs-js-sdk/references/methods/workspace.md index be8405a72..a6e4e4743 100644 --- a/.agents/skills/layerfs-js-sdk/references/methods/workspace.md +++ b/.agents/skills/layerfs-js-sdk/references/methods/workspace.md @@ -282,7 +282,7 @@ Unmounts a Workspace. A Sandbox left with no Workspace closes itself two minutes | --- | --- | | `ws` | The Workspace from `mount` | | `discard` | `true` to drop uncommitted changes instead of refusing | -| `force` | `true` to abort the Workspace's kernel connection instead; drops uncommitted changes. Use it only for an unmount that stalled (below) | +| `force` | `true` to abort the Workspace's kernel connection instead; drops uncommitted changes. A normal unmount that stalls at `Join` ends itself with one abort write, use `force` for a live connection with a usable abort control, not to replay an attempted abort | **Returns:** `{ kind: "Unmounted" }`, or with `force` `{ kind: "ForceUnmounted", abort, detach, cleanup }`. @@ -294,7 +294,8 @@ Unmounts a Workspace. A Sandbox left with no Workspace closes itself two minutes | `Dirty` | Uncommitted changes; `detail.changed` lists paths | Commit them, so the step is recorded; discard only a Workspace you're abandoning | | `Busy` | A background process still runs there | Stop it (`backend.daemon.process.cancel`, in the observe skill), or wait for it | | `NotMounted` | Already unmounted, or the wrong Workspace | Use the Workspace from `mount` | -| `Failed` with `TeardownCustody { stage: Join` in the message | The mount is gone but its connection did not end, so the unmount stalled; every later plain unmount answers the same | `workspace.unmount(ws, true, true)` once | +| `Failed` with `TeardownCustody { stage: Join` in the message | The mount is gone but its drain did not finish; the custody records whether an abort was attempted. Later unmounts retain that custody | Preserve the custody; do not retry an attempted abort. Replace the Sandbox if its connection cannot be ended | +| `Failed` with `TeardownCustody { stage: Abort` in the message | The one abort write failed or returned a short count; the original cause and abort facts are retained | Preserve the custody and replace the Sandbox; neither normal nor forced unmount repeats the abort | ```js // ws: from workspace.mount. Every mount creates a new Workspace; never hard-code one. diff --git a/api/agent/src/workspace/unmount.js b/api/agent/src/workspace/unmount.js index 62d881284..42b76efa7 100644 --- a/api/agent/src/workspace/unmount.js +++ b/api/agent/src/workspace/unmount.js @@ -1,7 +1,9 @@ // workspace.unmount: ends the Workspace; `discard` drops its uncommitted -// changes. `force` aborts its kernel connection instead, also after a normal -// unmount stalled at Join, and drops uncommitted changes. A Sandbox left with -// no Workspace closes once it has been idle. +// changes. A normal unmount whose drain stops at Join makes one abort write +// on the already-detached connection and ends there. `force` aborts the +// live connection when an abort control is usable, and drops uncommitted +// changes. An attempted abort is never repeated. A Sandbox left with no Workspace closes once it +// has been idle. import { invokeClient, workspaceArgs } from "../client.js"; export function unmount(client, ws, discard = false, force = false) { return invokeClient(client, "workspace.unmount", { ...workspaceArgs(ws), discard, force }); diff --git a/core/docs/architecture/76-native-mount-session.md b/core/docs/architecture/76-native-mount-session.md index 5621c8f87..4932c3641 100644 --- a/core/docs/architecture/76-native-mount-session.md +++ b/core/docs/architecture/76-native-mount-session.md @@ -189,19 +189,27 @@ its existing Commit-custody admission and then, for an attached Workspace: 3. **Drain.** [`drain`](../../crates/layerfs-fuse/src/session/drain.rs) establishes that every created loop is joined, the owner and watcher returned, no received, admitted or retained request remains, and releases the - lane. `drain_wait` is an observation deadline. + lane. `drain_wait` is an observation deadline. A stop at `Join` means the + connection outlived its mount (another mount namespace holds a copy of it, + #426 and #460): the unmount makes one abort write on the already-detached + connection and drains again, so the stalled connection still ends. A write + that is not accepted whole, or a second stop, keeps the entry `Retained` + exactly as any other stop does. 4. **Revocation.** `NativeJob::Revoke` logically revokes the engine mount. 5. **Close and registry removal** through the existing terminal Close. Any step that is not established leaves the entry `Retained` with its stage -(`Detach`, `Join`, `Owner`, `Requests`, `Lane`, `Revoke`, `Close` or `Registry`), +(`Detach`, `Join`, `Abort`, `Owner`, `Requests`, `Lane`, `Revoke`, `Close` or `Registry`), the observer and the original evidence. The reply is `Retained` with `TeardownCustody`; it is not reported as an uncertain mutation and nothing is replayed. Later Attach or Unmount on that entry returns the same custody, and -so does ForceUnmount, except after a detached drain stopped at `Join`: that one -[stalled normal unmount](80-forced-teardown.md#stalled-normal-unmount) is ended -by one forced unmount. The normal path never uses the abort control and never -sets the terminal fence. +so does ForceUnmount, except for the one kind of stop that can still be ended: +a detached drain stopped at `Join` whose abort control is bound and unused +([stalled normal unmount](80-forced-teardown.md#stalled-normal-unmount)). The +normal path uses the abort control only there. An accepted abort sets the +terminal service fence before the second drain. A failed or short abort reports +`Abort` with its original errno or byte count; later drain, Revoke and Close +failures retain the recorded abort effects in `TeardownCustody.forced`. ## Forced unmount diff --git a/core/docs/architecture/80-forced-teardown.md b/core/docs/architecture/80-forced-teardown.md index 9fe2d6f88..6ede45548 100644 --- a/core/docs/architecture/80-forced-teardown.md +++ b/core/docs/architecture/80-forced-teardown.md @@ -14,8 +14,10 @@ The daemon half is [`control/force.rs`](../../crates/layerfs-daemon/src/control/ The connection half (the abort write, the terminal fence and the forced drain) belongs to Fuse and is described in [native request service](75-native-request-service.md#terminal-fence-and-gated-ports). -The normal path is unchanged; see -[native mount session](76-native-mount-session.md#normal-unmount). +A normal unmount that stalls at `Join` makes the same single abort write +itself ([stalled normal unmount](#stalled-normal-unmount)); `ForceUnmount` +remains for the one stalled custody that could not be aborted, and for +aborting a live connection without waiting for its processes. ## Request and replies @@ -122,11 +124,24 @@ A normal unmount can detach successfully and still stop at `Join`: when another mount of the same superblock outlives the detach, for example a copy in a mount namespace made by a process outside the Sandbox, the kernel never ends the connection, so its receive loops stay blocked in their reads -(issues #426 and #460). The entry keeps that undrained session as `Retained` -at `Join` with no forced facts, and every later normal `Unmount` returns that -custody unchanged. - -`ForceUnmount` is the one way out. It takes the session back and makes the +(issues #426 and #460). The unmount now makes one abort write itself +(`NativeSession::abort_detached`) and drains again, so the connection ends and +the unmount completes with `Unmounted`. + +When no abort control is bound, the entry stays `Retained` at `Join` with +no abort facts. A failed or short write stops at `Abort`, preserving the +original errno or count in `detail` and the disposition in `forced`. An accepted +write followed by a failed second drain keeps that drain's stage and records +`abort: Written`. Revoke and Close failures also retain these facts. Every +attempted abort records `detach: Detached` and `commit: Absent`, since normal +admission established no Commit custody before the plain detach. + +A later normal or forced unmount returns the retained stage, detail and abort +facts unchanged. An unavailable control cannot be forced, and a consumed +control is never written again. Successful completion still replies `Unmounted`. + +`ForceUnmount` remains for a stalled custody that is still forceable (one +whose abort was not attempted) and for a live connection. It takes the session back and makes the same sequence with two differences. Step 1 is `NativeSession::abort_detached`, which refuses unless the connection is detached and not yet aborted. Step 2 makes no detach, because the normal unmount's plain detach is the diff --git a/core/docs/issues/496/ABORT-CUSTODY-FIX-20261029.md b/core/docs/issues/496/ABORT-CUSTODY-FIX-20261029.md new file mode 100644 index 000000000..0d93c41ae --- /dev/null +++ b/core/docs/issues/496/ABORT-CUSTODY-FIX-20261029.md @@ -0,0 +1,93 @@ +# Normal unmount abort custody + +> **Status:** Dated implementation checkpoint; not release qualification. + +PR #496 at `3a0564b6bb235dc7a0a62d732a5d8ec58e0ad865` ended a normal +unmount stalled at Join by attempting one abort and draining again. Its failed +abort return still carried the pre-abort `Undrained.forced`, and normal daemon +teardown supplied no abort facts to retained drain, Revoke or Close replies. + +## Change + +- Refresh the returned FUSE receipt from the session's recorded abort outcome. +- Report short/failed writes at `Abort`, with the original byte count/errno. +- Carry `Written` through a failed second drain, Revoke and Close. Normal + admission supplies `commit: Absent`; its completed detach supplies `Detached`. +- Keep successful normal replies `Unmounted`. Keep control consumption, one + attempt, drain predicates and custody ownership unchanged. +- Document that retained abort facts also apply to a stalled normal unmount. + Update the SDK guidance: an unavailable or consumed control is not forceable. +- Record the expanded test's functional duration in the CI slow inventory while + retaining its existing Linux mount lane. + +No test was deleted. No dependency, wire encoding or persistence profile changed. + +## Regression evidence + +The new failed/short-write cases were run before the production fix. Both failed: +`stage: Join`, `forced: None`, generic drain detail. The existing success case +passed. The post-fix six-case binary passes: + +1. Copied mount namespace: normal unmount completes, with a Written abort receipt. +2. Kernel-denied abort write: EIO and Failed remain in custody. +3. Zero-byte abort write: exact count and Short remain in custody. +4. An acknowledged write followed by another Join stop: Written remains in custody. +5. A failed owner job before Revoke: Written remains in Revoke custody. +6. An already-closed overlay: Revoke succeeds, Close refuses, Written remains in custody. + +The write failures use a test-only thread-local seccomp filter on the exact +already-bound descriptor of the test's own connection. Case 4 uses seccomp user +notification to simulate a successful write without ending the connection; it +checks propagation after a second drain failure, not a naturally reproduced +kernel failure. No descriptor is replaced, no other connection is inspected, +and no test code writes fusectl. The copied namespace is released before final +assertions, and subsequent normal/forced replies preserve the stage, detail and +abort facts without another abort. Work counters remain live observations. + +An initial Close fixture used a missing-object read and expected quarantine. +That expectation failed: definite missing-object reads do not quarantine the +reader. The fixture was replaced with a public owner Close to stage its exact +refusal, without weakening the expected custody assertion. + +## Validation + +Linux ARM64 Docker, Rust 1.85.1, locked dependencies, repository ARM64 flags; +Disposable Store explicitly selected by the fixture. Builds precede execution. +Every mounted invocation has a 100-second wall timeout; the FUSE/Bridge test +invocation has a 90-second execution timeout and tooling tests a 120-second cap. +These are functional tests, not a performance campaign. + +- `stalled_unmount`: 6 passed (21.28 s); the strengthened success receipt assertion + is additionally checked with only that case selected. +- `forced_unmount`: 3 passed; `forced_unavailable`: 1 passed; + `fp21_detach_stages`: 2 passed. +- `layerfs-fuse` and `layerfs-bridge`, all targets: 33 tests passed. +- Warning-denying Clippy for daemon/FUSE/Bridge, all targets: passed; the final + strengthened stalled-unmount test is checked separately. +- Product boundary: 956 production files passed; authorized fuser integrity + passed; 155 tooling tests passed, followed by 6 CI-selection tests after the + slow-inventory update. +- Rust formatting, diff whitespace and Ruff 0.17.2 formatting: passed. + An optional Ruff lint invocation reported the pre-existing EXE001 on + `tools/ci_tests.py` (the parent tree has a shebang and mode 100644). +- Durable: NOT_RUN — disabled by owner. No full SDK E2E or benchmark run. + +Machine-local append-only receipts, including the original failures, are under +`~/.cache/layerfs-worktrees/layerfs-pr496-fix-8cc8b1c1603d/builds/`. +The pre-fix receipt is `pr496-mount-stalled_unmount-7f1b9e7200234dcda9fc2312db1e4b87`; +the complete passing six-case receipt is +`pr496-mount-stalled_unmount-1b8071b206174205b677283a4ff02124`. + +## Production source size + +Production LOC: 102777 -> 102816 (delta +39). +Core: 38395 -> 38395 (0); engine: 61723 -> 61762 (+39); +API: 2659 -> 2659 (0). Reference absent (retired R9c; last counted 65,417 +at tree `498dd1917812`). + +Method: `tools/production_loc.py` blob +`3341e4d0362c822b6d9a08419c432942b3a13800`, `--root --json`, +against the first parent and final staged tree. First-party production Rust, +runtime SQL and JavaScript only; tests, tooling, docs, generated and third-party +code excluded. Exact tree IDs and the confirmed comparison are in the commit +message. This is a source-size comparison, not a performance claim. diff --git a/engine/crates/layerfs-bridge/src/control_mount.rs b/engine/crates/layerfs-bridge/src/control_mount.rs index 635ec9b55..ca87197f4 100644 --- a/engine/crates/layerfs-bridge/src/control_mount.rs +++ b/engine/crates/layerfs-bridge/src/control_mount.rs @@ -148,7 +148,8 @@ pub struct TeardownCustody { pub work: Option, /// Bounded original cause description; never parsed to decide custody. pub detail: String, - /// Forced-teardown effects; absent from a normal unmount or Attach custody. + /// Abort effects, including a normal unmount's stalled connection; absent + /// when no abort was attempted. pub forced: Option, } fn put_receipt(out: &mut Writer, value: &MountReceipt) -> Result<(), ControlError> { diff --git a/engine/crates/layerfs-daemon/src/control/detach.rs b/engine/crates/layerfs-daemon/src/control/detach.rs index e84e879aa..0bd63f90a 100644 --- a/engine/crates/layerfs-daemon/src/control/detach.rs +++ b/engine/crates/layerfs-daemon/src/control/detach.rs @@ -1,4 +1,7 @@ //! Linux half of normal unmount: plain detach once, then the drain barrier. +//! A drain that stops at `Join` makes the one abort write a forced unmount +//! would and drains again, so a connection that outlived its mount is ended +//! rather than kept (#426, #460). use super::{ kernel::Kernel, registry::bound_mut, @@ -7,11 +10,30 @@ use super::{ }; use crate::store::BoundWorkspace; use layerfs_bridge::control::{ - Activity, ControlCode, ForcedFacts, MountPhase, TeardownStage, WorkspaceToken, + AbortDisposition, Activity, CommitKnowledge, ControlCode, DetachDisposition, ForcedFacts, + MountPhase, TeardownStage, WorkspaceToken, +}; +use layerfs_fuse::{ + mount::AbortWrite, + session::{Detach, DrainStage, Forced, SessionObserver}, }; -use layerfs_fuse::session::{Detach, DrainStage, SessionObserver}; use std::{fmt, sync::Arc}; +/// Abort effects after normal admission, which established no Commit custody +/// and completed the one plain detach before attempting the write. +pub(super) fn detached_abort_facts(forced: Option) -> Option { + forced.map(|forced| ForcedFacts { + abort: match forced.abort { + AbortWrite::Written => AbortDisposition::Written, + AbortWrite::Short(_) => AbortDisposition::Short, + AbortWrite::Failed(_) => AbortDisposition::Failed, + }, + detach: DetachDisposition::Detached, + commit: CommitKnowledge::Absent, + fenced: forced.terminal_replies, + }) +} + /// Terminal admission over a connection-owning entry. No daemon-side request /// count refuses here: the kernel's own reversible answer is the fence, because /// every blocked caller, descriptor, working directory and mapping holds a @@ -84,21 +106,42 @@ impl Service { self.leaving(token, MountPhase::Draining); let drained = match session.drain() { Ok(drained) => drained, - Err(undrained) => { - let stopped = stage(undrained.stage); - let detail = format!( - "connection drain stopped at {:?}: loops {:?}, work {:?}", - undrained.stage, undrained.loops, undrained.work - ); - // Detached yet unjoined: the connection outlived its mount, - // and one forced unmount may still end it. - let evidence = match undrained.stage { - DrainStage::Join => Evidence::Stalled(undrained), - _ => Evidence::Other(undrained), - }; - return Err(self.keep(token, stopped, detail, Some(observer), evidence, None)); - } + // Detached yet unjoined: the connection outlived its mount because + // another mount namespace holds a copy of it (an engine or VM agent + // outside the Sandbox). Make the same one abort write a forced + // unmount makes, then drain again; a connection whose control is + // unusable, or one that still stops, keeps its custody (#426, #460). + Err(undrained) => match undrained.end_stalled() { + Ok(drained) => drained, + Err(undrained) => { + let facts = detached_abort_facts(undrained.forced); + let (stopped, detail) = match undrained.forced.map(|forced| forced.abort) { + Some(AbortWrite::Failed(errno)) => ( + TeardownStage::Abort, + format!("abort write: {errno}; nothing further attempted"), + ), + Some(AbortWrite::Short(count)) => ( + TeardownStage::Abort, + format!("abort write returned {count} of 1 byte; nothing further attempted"), + ), + abort => ( + stage(undrained.stage), + format!( + "connection drain stopped at {:?}: abort {abort:?}, abort_bound {}, loops {:?}, work {:?}", + undrained.stage, observer.facts().abort_bound, + undrained.loops, undrained.work + ), + ), + }; + let evidence = match undrained.stage { + DrainStage::Join => Evidence::Stalled(undrained), + _ => Evidence::Other(undrained), + }; + return Err(self.keep(token, stopped, detail, Some(observer), evidence, facts)); + } + }, }; + let facts = detached_abort_facts(drained.forced); // Drain barrier holds: one fixed logical mark, then indexed retirement. match self.job( workspace.route(), @@ -112,7 +155,7 @@ impl Service { failure.wire().detail, Some(observer), Box::new((failure, drained)), - None, + facts, )); } }; diff --git a/engine/crates/layerfs-daemon/src/control/serving.rs b/engine/crates/layerfs-daemon/src/control/serving.rs index 1385b560f..8a936eba6 100644 --- a/engine/crates/layerfs-daemon/src/control/serving.rs +++ b/engine/crates/layerfs-daemon/src/control/serving.rs @@ -58,8 +58,8 @@ pub(super) struct Kept { pub observer: Option, /// The original owner, exactly as the stopping boundary left it. pub evidence: Evidence, - /// Effects of a forced teardown as recorded where it stopped; absent for - /// a normal unmount or an Attach. + /// Abort effects as recorded where teardown stopped, including a normal + /// unmount's stalled connection. Absent when no abort was attempted. pub forced: Option, } impl fmt::Debug for Kept { diff --git a/engine/crates/layerfs-daemon/src/control/unmount.rs b/engine/crates/layerfs-daemon/src/control/unmount.rs index ca1d959a6..062b26a19 100644 --- a/engine/crates/layerfs-daemon/src/control/unmount.rs +++ b/engine/crates/layerfs-daemon/src/control/unmount.rs @@ -78,7 +78,12 @@ impl Service { workspace: &BoundWorkspace, kernel: Receipt, ) -> Result { - let kernel = self.closed(token, workspace, kernel, None)?; + #[cfg(target_os = "linux")] + let forced = + super::detach::detached_abort_facts(kernel.as_ref().and_then(|done| done.forced)); + #[cfg(not(target_os = "linux"))] + let forced = None; + let kernel = self.closed(token, workspace, kernel, forced)?; self.depart( token, Success { diff --git a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs index 43da7ea76..6a74af6f4 100644 --- a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs +++ b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs @@ -5,16 +5,18 @@ //! engine or VM agent outside a Sandbox can. The daemon's plain detach then //! succeeds in its own namespace while the superblock lives on in the copy, so //! the kernel never ends the connection: the receive loops stay blocked and -//! the drain stops at Join. A second normal unmount returns the same custody. -//! One forced unmount ends it with one abort write, then drains, revokes and -//! closes. Removing the mount directory then detaches the copy's mount too -//! (Linux detaches the mounts on a removed mount point in every namespace), -//! so the connection's control entry goes while the copy still runs, and the -//! copy finds no mount there. +//! the drain stops at Join. The normal unmount then makes the same single +//! abort write a forced unmount would, drains again, revokes and closes, so it +//! ends the connection itself. Removing the mount directory then detaches the +//! copy's mount too (Linux detaches the mounts on a removed mount point in +//! every namespace), so the connection's control entry goes while the copy +//! still runs, and the copy finds no mount there. //! //! The safety rule of `support/fusectl.rs` binds this file: nothing here //! writes under `/sys/fs/fuse/connections` or reads another connection. #![cfg(target_os = "linux")] +#[path = "support/abort_write.rs"] +mod abort_write; #[allow(dead_code)] #[path = "support/fusectl.rs"] mod fusectl; @@ -31,7 +33,8 @@ mod mounted; #[path = "support/mounted_commit.rs"] mod rig; use layerfs_bridge::control::{ - AbortDisposition, CommitKnowledge, DetachDisposition, MountPhase, Reply, TeardownStage, + AbortDisposition, CommitKnowledge, DetachDisposition, Reply, Request, TeardownStage, + UnmountControl, }; use layerfs_daemon::control::Failure; use mounted::{mount_entry, until}; @@ -125,7 +128,7 @@ impl Drop for Copy { } #[test] -fn a_normal_unmount_stalled_at_join_is_ended_by_one_forced_unmount() { +fn a_normal_unmount_stalled_at_join_ends_itself_with_one_abort_write() { fusectl::mount(); let rig = Rig::new("stalled-join"); let ready = rig.mount(1); @@ -134,67 +137,32 @@ fn a_normal_unmount_stalled_at_join_is_ended_by_one_forced_unmount() { let connection = fusectl::own(&ready); let mut copy = Copy::spawn(root(&ready)); - // The plain detach succeeds here; the connection outlives it. - let custody = match rig.harness.try_unmount(token) { - Err(Failure::Retained(custody)) => *custody, - other => panic!( - "normal unmount with a copied mount: {:?}", - other.map(|done| done.reply) - ), + // The plain detach succeeds here; the connection outlives it, so the drain + // stops at Join and the normal unmount ends it with one abort write. + let done = match rig.harness.try_unmount(token) { + Ok(done) => done, + Err(failure) => panic!("normal unmount of a copied mount: {failure:?}"), }; - println!("STALLED custody={custody:?}"); - assert_eq!(custody.stage, TeardownStage::Join, "{custody:?}"); - assert!(custody.detached && custody.forced.is_none(), "{custody:?}"); - let work = custody.work.expect("custody work"); + println!("STALLED plain unmount={:?}", done.reply); + assert_eq!(done.reply, Reply::Unmounted(token)); + let drained = done.kernel.as_ref().expect("drain receipt"); + let forced = drained.forced.expect("normal unmount made its abort"); + assert_eq!(forced.abort, layerfs_fuse::mount::AbortWrite::Written); assert_eq!( - (work.loops_exited, work.active, work.retained), - (0, 0, 0), - "{work:?}" + forced.detach, + layerfs_fuse::session::DetachAttempt::Detached ); assert!( mount_entry(&ready.directory).is_none(), "the daemon's mount row remains" ); - assert!(connection.exists(), "the connection ended with its mount"); - assert_eq!(rig.harness.phase(token), MountPhase::Retained); - - // Plain unmount is unchanged: a second one returns the same custody. - match rig.harness.try_unmount(token) { - Err(Failure::Retained(again)) => assert_eq!( - (again.stage, again.detail.as_str()), - (custody.stage, custody.detail.as_str()) - ), - other => panic!("second normal unmount: {:?}", other.map(|done| done.reply)), - } - - // One forced unmount ends the stalled custody. - let outcome = match rig.harness.try_force(token, false) { - Ok(done) => match done.reply { - Reply::ForceUnmounted(closed) => { - assert_eq!(closed.token, token); - closed.outcome - } - other => panic!("forced unmount replied {other:?}"), - }, - Err(failure) => panic!("forced unmount of a stalled Join: {failure:?}"), - }; - println!("FORCED outcome={outcome:?}"); - assert_eq!(outcome.facts.abort, AbortDisposition::Written); - assert_eq!(outcome.facts.detach, DetachDisposition::Detached); - assert_eq!(outcome.facts.commit, CommitKnowledge::Absent); - assert!( - outcome.work.loops_configured != 0 - && outcome.work.loops_joined == outcome.work.loops_configured, - "{:?}", - outcome.work - ); assert!( !Path::new(&ready.directory).exists(), "mount directory left behind" ); - // The removed directory took the copy's mount with it: the connection - // ends while the copy still runs, and the copy finds nothing there. + // The removed directory took the copy's mount with it: the connection ends + // while the copy still runs, and the copy finds nothing there. until("own connection entry removed", || !connection.exists()); let answer = copy.ask("probe"); assert!( @@ -206,3 +174,154 @@ fn a_normal_unmount_stalled_at_join_is_ended_by_one_forced_unmount() { drop(copy); rig.finish(); } + +fn rejected_abort(errno: Option, disposition: AbortDisposition, detail: &str) { + fusectl::mount(); + let rig = Rig::new("stalled-abort-error"); + let ready = rig.mount(1); + let token = ready.token; + let descriptor = abort_write::descriptor(&ready); + let mut copy = Copy::spawn(root(&ready)); + let service = rig.harness.service.clone(); + let (send, receive) = mpsc::channel(); + let (listen, listener) = mpsc::channel(); + let thread = thread::spawn(move || { + match errno { + Some(errno) => abort_write::deny_write(descriptor, errno), + None => listen + .send(abort_write::intercept_write(descriptor)) + .unwrap(), + } + let result = service.execute_control(&Request::Unmount(UnmountControl { + token, + discard: true, + })); + send.send(result).unwrap(); + }); + if errno.is_none() { + abort_write::acknowledge_write(listener.recv_timeout(WAIT).unwrap(), descriptor); + } + let result = receive.recv_timeout(Duration::from_secs(15)).unwrap(); + thread.join().unwrap(); + let custody = match result { + Err(Failure::Retained(custody)) => *custody, + other => panic!("rejected abort: {other:?}"), + }; + // Capture later replies before releasing the copied mount. No operation + // may retry the denied write or infer success from a later observation. + let again = rig.harness.try_unmount(token); + let forced = rig.harness.try_force(token, false); + let evidence = rig.harness.service.retained_kernel(token).unwrap().unwrap(); + assert!(copy.leave()); + drop(copy); + until("retained connection loops joined", || { + let work = rig.harness.status(token).kernel.unwrap().work.unwrap(); + work.loops_configured != 0 && work.loops_joined == work.loops_configured + }); + // Return all actual holds before asserting the expected failure receipt. + rig.harness.stop_retained(); + drop(rig.store); + rig.fixture.cleanup(); + let stage = if errno.is_some() { + TeardownStage::Abort + } else { + TeardownStage::Join + }; + assert_eq!(custody.stage, stage, "{custody:?}"); + let facts = custody.forced.as_ref().expect("attempted abort facts"); + assert_eq!(facts.abort, disposition); + assert_eq!(facts.detach, DetachDisposition::Detached); + assert_eq!(facts.commit, CommitKnowledge::Absent); + assert_eq!(facts.fenced, 0); + assert!(custody.detail.contains(detail), "{custody:?}"); + assert!(evidence.contains("forced: Some(Forced"), "{evidence}"); + for result in [again, forced] { + let Err(Failure::Retained(later)) = result else { + panic!("retained custody changed: {result:?}"); + }; + assert_eq!(later.stage, custody.stage); + assert_eq!(later.detail, custody.detail); + assert_eq!(later.forced, custody.forced); + } +} + +#[test] +fn stalled_abort_failure_preserves_errno_and_custody() { + rejected_abort(Some(nix::libc::EIO as u32), AbortDisposition::Failed, "EIO"); +} + +#[test] +fn stalled_short_abort_preserves_count_and_custody() { + rejected_abort(Some(0), AbortDisposition::Short, "0 of 1 byte"); +} + +fn later_failure(stage: TeardownStage) { + fusectl::mount(); + let rig = Rig::new("stalled-later-failure"); + let ready = rig.mount(1); + let token = ready.token; + let mut copy = Copy::spawn(root(&ready)); + let operation = rig.harness.service.operation(token).unwrap(); + match stage { + TeardownStage::Revoke => { + let owner = operation.overlay().clone(); + let route = operation.workspace().route(); + // A failed owner job poisons only this Workspace's connection. + assert!(thread::spawn(move || { + owner.call::<(), _>(route, layerfs_daemon::ServiceClass::General, None, |_| { + panic!("staged owner failure before Revoke") + }) + }) + .join() + .unwrap() + .is_err()); + } + TeardownStage::Close => { + // Stage an already-closed overlay through its public owner API. + // Revoke accepts a closed Workspace; a second Close must refuse + // Stale while preserving the native abort receipt. + operation + .overlay() + .close(operation.workspace().route()) + .unwrap(); + } + _ => unreachable!(), + } + drop(operation); + let result = rig.harness.try_unmount(token); + let again = rig.harness.try_force(token, false); + assert!(copy.leave()); + drop(copy); + rig.harness.stop_retained(); + drop(rig.store); + rig.fixture.cleanup(); + let Err(Failure::Retained(custody)) = result else { + panic!("expected retained {stage:?}: {result:?}"); + }; + assert_eq!(custody.stage, stage, "{custody:?}"); + let facts = custody.forced.as_ref().expect("successful abort facts"); + assert_eq!(facts.abort, AbortDisposition::Written); + assert_eq!(facts.detach, DetachDisposition::Detached); + assert_eq!(facts.commit, CommitKnowledge::Absent); + let Err(Failure::Retained(later)) = again else { + panic!("expected unchanged retained custody: {again:?}"); + }; + assert_eq!(later.stage, custody.stage); + assert_eq!(later.detail, custody.detail); + assert_eq!(later.forced, custody.forced); +} + +#[test] +fn stalled_abort_facts_survive_revoke_failure() { + later_failure(TeardownStage::Revoke); +} + +#[test] +fn stalled_abort_facts_survive_close_failure() { + later_failure(TeardownStage::Close); +} + +#[test] +fn stalled_abort_facts_survive_a_second_drain_stop() { + rejected_abort(None, AbortDisposition::Written, "Some(Written)"); +} diff --git a/engine/crates/layerfs-daemon/tests/support/abort_write.rs b/engine/crates/layerfs-daemon/tests/support/abort_write.rs new file mode 100644 index 000000000..b44885a40 --- /dev/null +++ b/engine/crates/layerfs-daemon/tests/support/abort_write.rs @@ -0,0 +1,122 @@ +//! Kernel-denied writes to this test's own, already-bound abort descriptor. +//! The filter belongs only to the calling thread, which exits after Unmount. +//! No descriptor is replaced and no test code writes a fusectl file. +use layerfs_bridge::control::ReadyMount; +use nix::libc; +use std::{ + fs, io, + os::fd::{AsRawFd, FromRawFd, OwnedFd}, + path::PathBuf, +}; + +pub fn descriptor(ready: &ReadyMount) -> u32 { + let own = super::fusectl::own(ready); + let path = PathBuf::from(super::fusectl::CONNECTIONS) + .join(own.minor.to_string()) + .join("abort"); + let descriptors: Vec<_> = fs::read_dir("/proc/self/fd") + .unwrap() + .filter_map(Result::ok) + .filter(|entry| fs::read_link(entry.path()).is_ok_and(|target| target == path)) + .map(|entry| entry.file_name().to_string_lossy().parse().unwrap()) + .collect(); + assert_eq!(descriptors.len(), 1, "one bound control for this mount"); + descriptors[0] +} + +/// errno=0 makes write return zero bytes; EIO preserves a concrete failure. +pub fn deny_write(fd: u32, errno: u32) { + install(fd, libc::SECCOMP_RET_ERRNO | errno, false); +} + +/// Intercept the one write so the test can acknowledge it without ending the +/// connection, staging a second Join stop after an apparently accepted abort. +pub fn intercept_write(fd: u32) -> OwnedFd { + let listener = install(fd, libc::SECCOMP_RET_USER_NOTIF, true); + // SAFETY: a successful NEW_LISTENER call returns a new owned descriptor. + unsafe { OwnedFd::from_raw_fd(listener) } +} + +fn install(fd: u32, action: u32, notify: bool) -> i32 { + let instruction = |code, jt, jf, k| libc::sock_filter { code, jt, jf, k }; + let load = (libc::BPF_LD | libc::BPF_W | libc::BPF_ABS) as u16; + let equal = (libc::BPF_JMP | libc::BPF_JEQ | libc::BPF_K) as u16; + let ret = (libc::BPF_RET | libc::BPF_K) as u16; + let mut instructions = [ + instruction( + load, + 0, + 0, + std::mem::offset_of!(libc::seccomp_data, nr) as u32, + ), + instruction(equal, 0, 3, libc::SYS_write as u32), + instruction( + load, + 0, + 0, + std::mem::offset_of!(libc::seccomp_data, args) as u32, + ), + instruction(equal, 0, 1, fd), + instruction(ret, 0, 0, action), + instruction(ret, 0, 0, libc::SECCOMP_RET_ALLOW), + ]; + let program = libc::sock_fprog { + len: instructions.len() as u16, + filter: instructions.as_mut_ptr(), + }; + // SAFETY: the kernel reads the live, correctly sized filter in this call. + // No TSYNC: other test/product threads retain their syscall permissions. + unsafe { + assert_eq!(libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), 0); + let flags = if notify { + libc::SECCOMP_FILTER_FLAG_NEW_LISTENER + } else { + 0 + }; + let result = libc::syscall( + libc::SYS_seccomp, + libc::SECCOMP_SET_MODE_FILTER, + flags, + &program, + ); + assert!( + result >= 0, + "install filter: {}", + io::Error::last_os_error() + ); + result as i32 + } +} + +pub fn acknowledge_write(listener: OwnedFd, expected_fd: u32) { + // SAFETY: all-zero notification structures are valid ioctl output/input + // buffers. Poll is bounded; this listener has exactly one consumer. + unsafe { + let fd = listener.as_raw_fd(); + let mut poll = libc::pollfd { + fd, + events: libc::POLLIN, + revents: 0, + }; + assert_eq!( + libc::poll(&mut poll, 1, 10_000), + 1, + "abort notification deadline" + ); + let mut request: libc::seccomp_notif = std::mem::zeroed(); + assert_eq!( + libc::ioctl(fd, libc::SECCOMP_IOCTL_NOTIF_RECV, &mut request), + 0 + ); + assert_eq!(request.data.nr, libc::SYS_write as i32); + assert_eq!(request.data.args[0], u64::from(expected_fd)); + assert_eq!(request.data.args[2], 1); + let reply = libc::seccomp_notif_resp { + id: request.id, + val: 1, + error: 0, + flags: 0, + }; + assert_eq!(libc::ioctl(fd, libc::SECCOMP_IOCTL_NOTIF_SEND, &reply), 0); + } +} diff --git a/engine/crates/layerfs-fuse/src/session/force.rs b/engine/crates/layerfs-fuse/src/session/force.rs index 156878576..277e15559 100644 --- a/engine/crates/layerfs-fuse/src/session/force.rs +++ b/engine/crates/layerfs-fuse/src/session/force.rs @@ -86,3 +86,25 @@ impl KernelSession { self.release(work) } } + +impl Undrained { + /// A normal unmount whose plain detach succeeded but whose drain stopped at + /// `Join`: another mount of the same superblock outlived the detach, so the + /// kernel never ended the connection and the receive loops stay blocked + /// (#426, #460). End it exactly the way a forced unmount does: one abort + /// write on the already-detached connection, then the same drain. Only a + /// `Join` stop is a candidate. A refused or short write keeps the owner + /// and records the attempted abort; its facts admit no second abort. + pub fn end_stalled(mut self) -> Result, Box> { + if self.stage != DrainStage::Join { + return Err(Box::new(self)); + } + match self.session.abort_detached() { + Ok(AbortWrite::Written) => self.session.force_drain(), + Ok(_) | Err(_) => { + self.forced = self.session.forced(); + Err(Box::new(self)) + } + } + } +} diff --git a/tools/ci_tests.py b/tools/ci_tests.py index 6ba4690a9..ac284bf54 100644 --- a/tools/ci_tests.py +++ b/tools/ci_tests.py @@ -40,6 +40,8 @@ # (package, test target): run seconds on the macOS host, 2026-10-23. SLOW = { + # Six Linux FUSE teardown cases, including two bounded Join observations. + ("layerfs-daemon", "stalled_unmount"): 21.28, # Linux native binding proof, owner-excluded from CI; run explicitly. ("layerfs-node", "native_admission"): 60.52, # Linux SDK mount proof after the ownership move, 2026-10-27. @@ -79,10 +81,13 @@ } # SDK timings are recorded above without adding the owner-excluded API to CI. +# stalled_unmount's timing is recorded above; it runs in the mount lane. NIGHTLY = { target for target in set(SLOW) | set(HEAVY) - if target[0] != "layerfs-node" and not (target[0] == "layerfs-api" and target[1] in API_MOUNT_TARGETS) + if target[0] != "layerfs-node" + and not (target[0] == "layerfs-api" and target[1] in API_MOUNT_TARGETS) + and target != (MOUNT_PACKAGE, "stalled_unmount") }