From 3a0564b6bb235dc7a0a62d732a5d8ec58e0ad865 Mon Sep 17 00:00:00 2001 From: rain Date: Wed, 28 Oct 2026 22:08:30 +0800 Subject: [PATCH] fix(daemon): end a stalled normal unmount at Join with its own abort write (#426, #460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A normal unmount can detach successfully and still stop at Join: when another mount of the same superblock outlives the detach, the kernel never ends the connection, its receive loops stay blocked, and the entry was kept as Retained at Join with no way out but a forced unmount — or, where no abort control is bound, no way out at all (#426, #460). The unmount now makes the same single abort write a forced unmount makes, on the already-detached connection, and drains again. It then completes as Unmounted, revokes and closes, so the stalled connection ends where the unmount asked. Only a refused or short write, or a second stop, keeps the custody, exactly as any other stop does; such an entry is not forceable either, because its abort control has been used. - layerfs-fuse: Undrained::end_stalled, one abort_detached write then force_drain, only for a Join stop. - layerfs-daemon: unmount_kernel calls it on a Join stop; a second stop keeps the evidence as before. - tests: stalled_unmount now asserts the normal unmount ends the stall itself (the copy of the mount answers ENOENT) and keeps no custody. - docs: native mount session and forced teardown describe the automatic abort; the SDK skill and the unmount comment no longer send callers to --force for this case. Tests: - cargo test -p layerfs-daemon --test stalled_unmount --release in a privileged Linux container: passes; the parent's code keeps custody, so the test fails without this change. - also ran mounted_drain, fp21_detach_stages, control_root, mounted_fusectl, forced_unmount and forced_unavailable; all pass except the environment bound fp31 reclaim observation in mounted_drain (container cgroup memory reclaim, unrelated to unmount). Production LOC: 102763 -> 102777 (delta +14) core 38395 -> 38395 (0), engine 61709 -> 61723 (+14), api 2659 (0). --- .../references/methods/workspace.md | 4 +- api/agent/src/workspace/unmount.js | 8 +- .../architecture/76-native-mount-session.md | 16 ++-- core/docs/architecture/80-forced-teardown.md | 24 ++++-- .../layerfs-daemon/src/control/detach.rs | 37 +++++---- .../layerfs-daemon/tests/stalled_unmount.rs | 79 ++++--------------- .../crates/layerfs-fuse/src/session/force.rs | 19 +++++ 7 files changed, 94 insertions(+), 93 deletions(-) diff --git a/.agents/skills/layerfs-js-sdk/references/methods/workspace.md b/.agents/skills/layerfs-js-sdk/references/methods/workspace.md index be8405a72..b8a9cb742 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, so this is for an entry that kept its custody because no abort control was usable | **Returns:** `{ kind: "Unmounted" }`, or with `force` `{ kind: "ForceUnmounted", abort, detach, cleanup }`. @@ -294,7 +294,7 @@ 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, its connection did not end, and no abort write could be made, so every later plain unmount answers the same; `force` is refused too | Stop: the Sandbox holds a connection the engine cannot abort; replace the Sandbox | ```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..c24296d6d 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 +// connection of an entry that kept its custody (no usable abort control), and +// drops uncommitted changes. 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..2199e895b 100644 --- a/core/docs/architecture/76-native-mount-session.md +++ b/core/docs/architecture/76-native-mount-session.md @@ -189,7 +189,12 @@ 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. @@ -198,10 +203,11 @@ Any step that is not established leaves the entry `Retained` with its stage 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, and never sets the terminal +fence. ## Forced unmount diff --git a/core/docs/architecture/80-forced-teardown.md b/core/docs/architecture/80-forced-teardown.md index 9fe2d6f88..1ec895531 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,19 @@ 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`. + +Only when that write cannot be made — no abort control is bound, or the write +is short or refused — does the entry keep the undrained session as `Retained` +at `Join`, and every later normal `Unmount` returns that custody unchanged. +Such a custody is not forceable either: the guard admits only a stalled entry +whose control is bound and unused, which the normal path has then already +used. + +`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/engine/crates/layerfs-daemon/src/control/detach.rs b/engine/crates/layerfs-daemon/src/control/detach.rs index e84e879aa..f7549c885 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, @@ -84,20 +87,26 @@ 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 stopped = stage(undrained.stage); + let detail = format!( + "connection drain stopped at {:?}: loops {:?}, work {:?}", + undrained.stage, 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, None)); + } + }, }; // Drain barrier holds: one fixed logical mark, then indexed retirement. match self.job( diff --git a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs index 43da7ea76..4ef7cf40f 100644 --- a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs +++ b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs @@ -5,12 +5,12 @@ //! 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. @@ -30,10 +30,7 @@ mod mounted; #[allow(dead_code)] #[path = "support/mounted_commit.rs"] mod rig; -use layerfs_bridge::control::{ - AbortDisposition, CommitKnowledge, DetachDisposition, MountPhase, Reply, TeardownStage, -}; -use layerfs_daemon::control::Failure; +use layerfs_bridge::control::Reply; use mounted::{mount_entry, until}; use rig::{root, Rig}; use std::{ @@ -125,7 +122,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 +131,25 @@ 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"); - assert_eq!( - (work.loops_exited, work.active, work.retained), - (0, 0, 0), - "{work:?}" - ); + println!("STALLED plain unmount={:?}", done.reply); + assert_eq!(done.reply, Reply::Unmounted(token)); 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!( diff --git a/engine/crates/layerfs-fuse/src/session/force.rs b/engine/crates/layerfs-fuse/src/session/force.rs index 156878576..322ae7178 100644 --- a/engine/crates/layerfs-fuse/src/session/force.rs +++ b/engine/crates/layerfs-fuse/src/session/force.rs @@ -86,3 +86,22 @@ 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 custody + /// as it stood, and its facts then 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(_) => Err(Box::new(self)), + } + } +}