diff --git a/core/docs/architecture/76-native-mount-session.md b/core/docs/architecture/76-native-mount-session.md index 4932c3641..f636c508e 100644 --- a/core/docs/architecture/76-native-mount-session.md +++ b/core/docs/architecture/76-native-mount-session.md @@ -189,12 +189,15 @@ 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. 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. + lane. `drain_wait` is an observation deadline for the requests. Once they + are quiescent, the join of the receive loops is observed only for the + shorter `JOIN_WAIT` (1 s): a healthy join follows the detach in + milliseconds. A stop at `Join` means the connection outlived its mount + (another mount namespace holds a copy of it, #426 and #460): the unmount + then 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. diff --git a/core/docs/architecture/80-forced-teardown.md b/core/docs/architecture/80-forced-teardown.md index 6ede45548..0bd536e38 100644 --- a/core/docs/architecture/80-forced-teardown.md +++ b/core/docs/architecture/80-forced-teardown.md @@ -124,7 +124,9 @@ 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 unmount now makes one abort write itself +(issues #426 and #460). The join is observed for one second (`JOIN_WAIT`) +after the requests are quiescent, not for the whole drain wait, so the stall +is recognised promptly. The unmount now makes one abort write itself (`NativeSession::abort_detached`) and drains again, so the connection ends and the unmount completes with `Unmounted`. diff --git a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs index 6a74af6f4..5e697f77c 100644 --- a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs +++ b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs @@ -7,7 +7,8 @@ //! the kernel never ends the connection: the receive loops stay blocked and //! 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 +//! ends the connection itself; the join is bounded by a short grace, not the +//! whole drain wait. 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. @@ -138,11 +139,19 @@ fn a_normal_unmount_stalled_at_join_ends_itself_with_one_abort_write() { let mut copy = Copy::spawn(root(&ready)); // 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. + // stops at Join and the normal unmount ends it with one abort write. The + // join is observed for JOIN_WAIT (1 s), not the whole DRAIN_WAIT, so this + // returns in about a second; without the grace it would take about five. + let started = Instant::now(); let done = match rig.harness.try_unmount(token) { Ok(done) => done, Err(failure) => panic!("normal unmount of a copied mount: {failure:?}"), }; + let elapsed = started.elapsed(); + assert!( + elapsed < Duration::from_secs(3), + "the stalled unmount waited {elapsed:?}, not the 1 s join grace" + ); println!("STALLED plain unmount={:?}", done.reply); assert_eq!(done.reply, Reply::Unmounted(token)); let drained = done.kernel.as_ref().expect("drain receipt"); diff --git a/engine/crates/layerfs-fuse/src/session/drain.rs b/engine/crates/layerfs-fuse/src/session/drain.rs index 51d83d5fb..24ac43f24 100644 --- a/engine/crates/layerfs-fuse/src/session/drain.rs +++ b/engine/crates/layerfs-fuse/src/session/drain.rs @@ -5,6 +5,14 @@ use fuser::SessionPhase; use nix::errno::Errno; use std::{any::Any, fs, sync::atomic::Ordering, time::Instant}; +/// How long the join of the receive loops may take once every request +/// is quiescent. A healthy join follows the detach in milliseconds; a +/// connection kept alive by another mount namespace of the same +/// superblock never joins (#426, #460). Keeping this bound shorter +/// than `drain_wait` lets a stalled connection be aborted promptly, +/// while requests still get the whole `drain_wait`. +const JOIN_WAIT: std::time::Duration = std::time::Duration::from_secs(1); + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Detach { /// The kernel acknowledged the detach; kernel admission is now terminal. @@ -76,9 +84,13 @@ impl KernelSession { } } if let (Some(monitor), true) = (&self.monitor, self.owner.is_some()) { + // Requests are quiescent; from here the wait is only the loops' + // join, which a healthy connection finishes in milliseconds and a + // stalled one never does. Bound it by JOIN_WAIT, not drain_wait. + let join_deadline = deadline.min(Instant::now() + JOIN_WAIT); let mut loops = monitor.snapshot(); while loops.phase != SessionPhase::Joined { - let remaining = deadline.saturating_duration_since(Instant::now()); + let remaining = join_deadline.saturating_duration_since(Instant::now()); if remaining.is_zero() { return Err(stop(DrainStage::Join, None)); }