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)), + } + } +}