From 888ae70cd988934798e820799bad6b25a3073bd8 Mon Sep 17 00:00:00 2001 From: YifanXu1999 Date: Wed, 28 Oct 2026 03:50:16 +0800 Subject: [PATCH] fix(daemon): end a normal unmount stalled at Join with one forced unmount (#426, #460) A FUSE superblock outlives the daemon's plain detach when another mount namespace holds a copy of the Workspace mount (an engine or VM agent outside the Sandbox). The receive loops then stay blocked, the drain stops at Join, and the entry was kept as Retained with no way out: its owner was boxed as Debug-only evidence, ForceUnmount refused Retained entries, and the session's abort refused once detached. - Retained custody keeps a Join-stalled drain with its real type (Evidence::Stalled). Kept::forceable admits exactly that case: detached, abort control bound, not yet aborted, never forced. - ForceUnmount on such an entry makes one abort write (KernelSession::abort_detached), then reuses the existing force drain, revoke and close. Removing the mount directory also detaches the copy. A refused abort keeps the custody at Join. Plain unmount is unchanged. - The daemon mounts its own fusectl at /fusectl when /sys/fs/fuse/connections has none (Docker gives containers a read-only /sys without it). The mount is best effort: if it is refused (EPERM in a user namespace) the daemon still starts, logs LAYERFS_DAEMON_ABORT_UNBOUND, and the abort control stays unbound, so forced unmount is refused as before. - workspace.unmount(ws, discard?, force?) in the SDK, `layerfs workspace unmount --force` in the CLI, and the binding return ForceUnmounted. - Docs: forced-teardown and native-mount-session architecture, the agent docs table and the layerfs-js-sdk skill. Tests: - added engine/crates/layerfs-daemon/tests/stalled_unmount.rs: reproduces the stall with `unshare -m` and ends it with one forced unmount. - added engine/crates/layerfs-daemon/tests/control_root.rs: private fusectl mount, reuse, refusal of a leftover directory, and a refused mount in a user namespace leaves no directory. - touched support/mounted.rs, support/host_install.rs (shared_control, once per process), mounted_sqlite_full.rs, and the layerfs-api tests kernel_application.rs, observed_application.rs, r5_1_actual_daemon_commit.rs; the JS fixture adds a forced-unmount case. - deleted: none. Production LOC: 102588 -> 102728 (delta +140) core 38395 -> 38395 (0), engine 61534 -> 61674 (+140), api 2659 -> 2659 (0); reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Method: python3 tools/production_loc.py --root on `git archive` of the parent (08ded82541) and of the staged tree. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/layerfs-js-sdk/SKILL.md | 2 +- .../references/methods/workspace.md | 7 +- api/agent/src/workspace/unmount.js | 8 +- api/agent/test/fixture/cases.mjs | 2 + api/agent/test/fixture/fake-native.cjs | 2 +- api/cli/src/workspace/unmount.js | 8 +- api/docs/agent.md | 4 +- .../architecture/76-native-mount-session.md | 8 +- core/docs/architecture/80-forced-teardown.md | 53 ++++- .../crates/layerfs-api/src/sandbox/values.rs | 13 +- .../layerfs-api/tests/kernel_application.rs | 1 + .../layerfs-api/tests/observed_application.rs | 9 + .../tests/r5_1_actual_daemon_commit.rs | 17 ++ .../src/application/filesystem.rs | 18 +- .../layerfs-daemon/src/control/attach.rs | 9 +- .../layerfs-daemon/src/control/detach.rs | 40 ++-- .../layerfs-daemon/src/control/force.rs | 70 +++++- .../layerfs-daemon/src/control/kernel.rs | 2 + .../layerfs-daemon/src/control/serving.rs | 33 ++- .../layerfs-daemon/tests/control_root.rs | 126 +++++++++++ .../tests/mounted_sqlite_full.rs | 1 + .../layerfs-daemon/tests/stalled_unmount.rs | 208 ++++++++++++++++++ .../tests/support/host_install.rs | 26 +++ .../layerfs-daemon/tests/support/mounted.rs | 1 + engine/crates/layerfs-fuse/src/mount/mod.rs | 2 +- .../crates/layerfs-fuse/src/mount/syscalls.rs | 39 +++- .../crates/layerfs-fuse/src/session/force.rs | 22 +- .../layerfs-fuse/src/session/startup.rs | 2 +- .../crates/layerfs-fuse/src/session/state.rs | 3 + engine/crates/layerfs-node/src/workspace.rs | 23 +- .../src/backend/docker/topology.rs | 6 +- 31 files changed, 695 insertions(+), 70 deletions(-) create mode 100644 engine/crates/layerfs-daemon/tests/control_root.rs create mode 100644 engine/crates/layerfs-daemon/tests/stalled_unmount.rs diff --git a/.agents/skills/layerfs-js-sdk/SKILL.md b/.agents/skills/layerfs-js-sdk/SKILL.md index f0a5bdabba..0410b19e08 100644 --- a/.agents/skills/layerfs-js-sdk/SKILL.md +++ b/.agents/skills/layerfs-js-sdk/SKILL.md @@ -373,7 +373,7 @@ method, its arguments, result fields, refusals with what to do, limits and examp | [fs writes](references/methods/fs-write.md) | `write(branch, path, content, message?)`, `edit(branch, path, oldText, newText, message?)`, `apply(branch, changes, message?)`, `mkdir(branch, path, message?)`, `rm(branch, path, message?)`, `cp(branch, from, to, message?)`, `mv(branch, from, to, message?)` | | [branch](references/methods/branch.md) | `fork(name, fromRef?)`, `list_commits(branch, pageSize?, pagePos?)`, `diff_commits(from, to, pageSize?, pagePos?)`, `diff_branches(from, to, pageSize?, pagePos?)`, `merge(a, b, name, message?)`, `publish(branch, commit?)` | | [sandbox](references/methods/sandbox.md) | `create(options?)`, `list_workspaces(sandbox, pageSize?, pagePos?)`, `close(sandbox)` | -| [workspace](references/methods/workspace.md) | `mount(sandbox, branch)` → `{ sandbox, path, image }`, `exec(ws, command, message?)`, `commit(ws, message?, force?)`, `unmount(ws, discard?)` | +| [workspace](references/methods/workspace.md) | `mount(sandbox, branch)` → `{ sandbox, path, image }`, `exec(ws, command, message?)`, `commit(ws, message?, force?)`, `unmount(ws, discard?, force?)` | A `ref` is `main`, a Branch name, a Commit id or a Layer id. Paths are absolute in the view (`/src/index.js`). Pages are `{ items, hasMore }`; advance with diff --git a/.agents/skills/layerfs-js-sdk/references/methods/workspace.md b/.agents/skills/layerfs-js-sdk/references/methods/workspace.md index cdf67b6887..be8405a72c 100644 --- a/.agents/skills/layerfs-js-sdk/references/methods/workspace.md +++ b/.agents/skills/layerfs-js-sdk/references/methods/workspace.md @@ -274,7 +274,7 @@ conflict detail also exposes `conflictsTruncated`, and individual conflicts mark their text. Inspect those files with bounded `workspace.exec` commands and Commit each command. The omission flags never mean the unresolved source is safe to drop. -## `workspace.unmount(ws, discard?)` +## `workspace.unmount(ws, discard?, force?)` Unmounts a Workspace. A Sandbox left with no Workspace closes itself two minutes later. @@ -282,8 +282,10 @@ 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) | -**Returns:** `{ kind: "Unmounted" }`. +**Returns:** `{ kind: "Unmounted" }`, or with `force` +`{ kind: "ForceUnmounted", abort, detach, cleanup }`. **Refusals** @@ -292,6 +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 | ```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 eaf59482ae..62d881284f 100644 --- a/api/agent/src/workspace/unmount.js +++ b/api/agent/src/workspace/unmount.js @@ -1,6 +1,8 @@ // workspace.unmount: ends the Workspace; `discard` drops its uncommitted -// changes. A Sandbox left with no Workspace closes once it has been idle. +// 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. import { invokeClient, workspaceArgs } from "../client.js"; -export function unmount(client, ws, discard = false) { - return invokeClient(client, "workspace.unmount", { ...workspaceArgs(ws), discard }); +export function unmount(client, ws, discard = false, force = false) { + return invokeClient(client, "workspace.unmount", { ...workspaceArgs(ws), discard, force }); } diff --git a/api/agent/test/fixture/cases.mjs b/api/agent/test/fixture/cases.mjs index b8cd4baa58..ec70729391 100644 --- a/api/agent/test/fixture/cases.mjs +++ b/api/agent/test/fixture/cases.mjs @@ -107,6 +107,8 @@ export const CASES = [ op: "workspace.commit", sent: { ...WS, force: true }, result: committed }, { name: "workspace unmount", argv: ["workspace", "unmount", HANDLE, "--discard"], sdk: (lf) => lf.workspace.unmount(WS, true), op: "workspace.unmount", sent: { ...WS, discard: true }, result: { kind: "Unmounted" } }, + { name: "workspace unmount (force)", argv: ["workspace", "unmount", HANDLE, "--force"], sdk: (lf) => lf.workspace.unmount(WS, false, true), + op: "workspace.unmount", sent: { ...WS, force: true }, result: { kind: "ForceUnmounted", abort: "Written", detach: "Detached", cleanup: "Queued" } }, { name: "sandbox create", argv: ["sandbox", "create", "--image", "sha256:abc"], sdk: (lf) => lf.sandbox.create({ image: "sha256:abc" }), op: "sandbox.create", sent: { deploy: { image: "sha256:abc" } }, result: { sandbox: "2", image: "sha256:abc" } }, { name: "sandbox list_workspaces", argv: ["sandbox", "list_workspaces", "2"], sdk: (lf) => lf.sandbox.list_workspaces("2"), diff --git a/api/agent/test/fixture/fake-native.cjs b/api/agent/test/fixture/fake-native.cjs index c105e10331..19dbe47be4 100644 --- a/api/agent/test/fixture/fake-native.cjs +++ b/api/agent/test/fixture/fake-native.cjs @@ -60,7 +60,7 @@ function answer(op, args) { return { value: { kind: `exit:${code}`, exitCode: code, stdout: `out:${args.command}`, stderr: "err", truncated: false } }; } case "workspace.commit": return { value: committed }; - case "workspace.unmount": return { value: { kind: "Unmounted" } }; + case "workspace.unmount": return { value: args.force ? { kind: "ForceUnmounted", abort: "Written", detach: "Detached", cleanup: "Queued" } : { kind: "Unmounted" } }; case "backend.daemon.process.start": return { value: { kind: "running", process: "proc_1_ab", command: args.command } }; case "backend.store.project.get_history": return { value: exports.history ?? { kind: "ok", version: "h1", project: "website", branches: [], commits: [], more: {}, layers: [], workspaces: [] } }; case "backend.store.project.get_storage": return { value: { dedupe: "2", compression: null } }; diff --git a/api/cli/src/workspace/unmount.js b/api/cli/src/workspace/unmount.js index a136d28e27..400552e187 100644 --- a/api/cli/src/workspace/unmount.js +++ b/api/cli/src/workspace/unmount.js @@ -1,8 +1,8 @@ -// layerfs workspace unmount: workspace.unmount(ws, discard?). +// layerfs workspace unmount: workspace.unmount(ws, discard?, force?). export default { args: ["ws"], - options: { discard: "bool" }, - summary: "end the Workspace; --discard drops uncommitted changes", - run: (lf, a, o) => lf.workspace.unmount(a.ws, o.discard), + options: { discard: "bool", force: "bool" }, + summary: "end the Workspace; --discard drops uncommitted changes, --force aborts its connection", + run: (lf, a, o) => lf.workspace.unmount(a.ws, o.discard, o.force), }; diff --git a/api/docs/agent.md b/api/docs/agent.md index 90a6ccd96e..330fcab914 100644 --- a/api/docs/agent.md +++ b/api/docs/agent.md @@ -273,7 +273,7 @@ trace line covers the command's run. | fs writes (7) | `write(branch, path, content, message? \| { message?, mode? })`, `edit(branch, path, oldText, newText, message?)`, `apply(branch, changes, message?)`, `mkdir(branch, path, message? \| { message?, mode? })`, `rm(branch, path, message?)`, `cp(branch, from, to, message?)`, `mv(branch, from, to, message?)` | | branch (6) | `fork(name, fromRef?)`, `list_commits(branch, pageSize?, pagePos?)`, `diff_commits(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `diff_branches(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `merge(a, b, name, message?, units?)`, `publish(branch, commit?)` | | sandbox (3) | `create({ image }?)` → `{ sandbox, image }`, `list_workspaces(sandbox, pageSize?, pagePos?)`, `close(sandbox)` | -| workspace (4) | `mount(sandbox, branch)` → `{ sandbox, path, image }`, `exec(ws, command, message?, { stdout?, stderr? }?)`, `commit(ws, message?, force?)`, `unmount(ws, discard?)` | +| workspace (4) | `mount(sandbox, branch)` → `{ sandbox, path, image }`, `exec(ws, command, message?, { stdout?, stderr? }?)`, `commit(ws, message?, force?)`, `unmount(ws, discard?, force?)` | A Workspace's status is not an agent call: it is `backend.daemon.workspace.status(ws)` in [`@layerfs/backend`](backend.md). @@ -518,7 +518,7 @@ table differs, and when a family's files stop matching the SDK's. | `workspace.mount` | `layerfs workspace mount ` | | `workspace.exec` | `layerfs workspace exec [-m ] [--stream]` | | `workspace.commit` | `layerfs workspace commit [-m ] [--force]` | -| `workspace.unmount` | `layerfs workspace unmount [--discard]` | +| `workspace.unmount` | `layerfs workspace unmount [--discard] [--force]` | | `sandbox.create` | `layerfs sandbox create [--image ]` | | `sandbox.list_workspaces` | `layerfs sandbox list_workspaces [--page-size ] [--page-pos ]` | | `sandbox.close` | `layerfs sandbox close ` | diff --git a/core/docs/architecture/76-native-mount-session.md b/core/docs/architecture/76-native-mount-session.md index b5cadc9c0e..5621c8f873 100644 --- a/core/docs/architecture/76-native-mount-session.md +++ b/core/docs/architecture/76-native-mount-session.md @@ -197,9 +197,11 @@ Any step that is not established leaves the entry `Retained` with its stage (`Detach`, `Join`, `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, Unmount or ForceUnmount on that entry returns the same -custody. The normal path never uses the abort control and never sets the -terminal fence. +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. ## Forced unmount diff --git a/core/docs/architecture/80-forced-teardown.md b/core/docs/architecture/80-forced-teardown.md index 1b941a94da..9fe2d6f888 100644 --- a/core/docs/architecture/80-forced-teardown.md +++ b/core/docs/architecture/80-forced-teardown.md @@ -47,7 +47,8 @@ application takes it, although such a refusal carries no custody of its own. | Condition | Answer | | --- | --- | | The token names no entry, another namespace, or a binding still in progress | `Missing`, `Invalid` or `Busy` at `admission`, as for every control operation | -| The entry is `Retained` | The stored custody, as `Retained`. No second abort or detach ever follows it | +| The entry is `Retained` by a normal unmount stopped at `Join`, its abort control bound and unused | Admitted as stalled: native phase `Stopping`, Commit knowledge `Absent` (a normal unmount is admitted only idle). See [stalled normal unmount](#stalled-normal-unmount) | +| The entry is `Retained` otherwise | The stored custody, as `Retained`. No second abort or detach ever follows it | | Activity is `Committing`, `Attaching` or `Closing` | `Busy` at `force:admission` | | No native connection (`Unattached`) | `Invalid` at `force:admission` | | An Attach outcome is unknown (`Attaching`) | `Unknown` at `force:admission` | @@ -113,7 +114,29 @@ After admission, outside the registry lock, each step is made once: 6. **Routing removal and `ForceUnmounted`.** The token is `Missing` afterwards. No `MNT_FORCE`, lazy detach, second abort, second detach, retry, process -signal or wait on a caller's exit or output exists on this path. Revoke, Close +signal or wait on a caller's exit or output exists on this path. + +### Stalled normal unmount + +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 +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 +connection's one detach, so the facts report `detach: Detached` from the +start. A stop after the abort is ordinary forced custody and is never forced +again. After the abort the drain completes and the mount directory is removed, +and Linux then detaches the mounts on that removed mount point in every +namespace, so a copy at the same path goes with it and the connection ends. +A copy mounted anywhere else stays, and its calls get the aborted connection's +answer until it is unmounted. Revoke, Close and the observation are submitted without waiting for admission, as on the normal path, and at most one Lifecycle completion is held while another job is submitted. @@ -139,12 +162,26 @@ returned only by a later `Unmount`, `ForceUnmount` or `Attach`. ## Limits -- Forced teardown needs the abort control, which is bound at Attach only when - the control filesystem is mounted where the daemon runs. The Sandbox - container does not mount it, so in that topology every Force is refused at - `force:capability`. Mounting it there is an owner decision. -- There is no way out of `Retained`. A request that ended retained keeps the - drain at `Requests` with no detach; daemon stop is the exit. +- Forced teardown needs the abort control, which is bound at Attach from the + control filesystem. A daemon uses the shared `/sys/fs/fuse/connections` + when it is mounted there. Otherwise (a Sandbox container mounts none) it + makes one mount of it at startup in `fusectl/` beside its Overlay file, a + directory only the daemon can reach, because the tree lists every + connection of the kernel, other containers' included. The daemon still + binds only its own connection's row. The mount is best effort: where it is + refused, as inside a user namespace (rootless Docker or Podman), which + refuses it though not FUSE mounts, the daemon starts anyway with no abort + control bound, writes one `LAYERFS_DAEMON_ABORT_UNBOUND ` line to + stderr, and every Force is refused at `force:capability`, as before the + mount existed. A refused mount leaves no directory behind. An existing + `fusectl/` without the mount is not adopted, so it also leaves the control + unbound. +- The private mount lasts as long as the daemon's mount namespace. In a + Sandbox that ends with the container; a daemon run elsewhere leaves it + mounted after it exits. +- Apart from a [stalled normal unmount](#stalled-normal-unmount), there is no + way out of `Retained`. A request that ended retained keeps the drain at + `Requests` with no detach; daemon stop is the exit. - A Workspace that is `Unattached` with unknown Commit custody has no terminal exit: Unmount is `Unknown` and Force is `Invalid`. - A retained Commit failure can keep Lifecycle completions (its local diff --git a/engine/crates/layerfs-api/src/sandbox/values.rs b/engine/crates/layerfs-api/src/sandbox/values.rs index b044ab4f89..f4376a0ee3 100644 --- a/engine/crates/layerfs-api/src/sandbox/values.rs +++ b/engine/crates/layerfs-api/src/sandbox/values.rs @@ -6,7 +6,7 @@ //! refusals with stable codes. use super::{CommitReply, MountedWorkspace, Unmounted}; use crate::{OperationCause, OperationFailure}; -use layerfs_bridge::control::{SdkRefusal, SdkValue, WorkspaceStatus}; +use layerfs_bridge::control::{ForcedOutcome, SdkRefusal, SdkValue, WorkspaceStatus}; use layerfs_bridge::sdk_values::{ commit_value, conflicts, conflicts_truncated, hex, lww, lww_truncated, }; @@ -171,6 +171,17 @@ pub fn commit(reply: &CommitReply) -> Result { } } +/// `sandbox.unmount` with `force`: what the forced teardown established. +pub fn force_unmounted(outcome: &ForcedOutcome) -> SdkValue { + let text = |value: &dyn std::fmt::Debug| SdkValue::text(format!("{value:?}")); + SdkValue::map([ + ("kind", SdkValue::text("ForceUnmounted")), + ("abort", text(&outcome.facts.abort)), + ("detach", text(&outcome.facts.detach)), + ("cleanup", text(&outcome.cleanup)), + ]) +} + /// `sandbox.unmount`: Unmounted, or the `Dirty` refusal naming the paths. pub fn unmounted(answer: &Unmounted) -> Result { match answer { diff --git a/engine/crates/layerfs-api/tests/kernel_application.rs b/engine/crates/layerfs-api/tests/kernel_application.rs index 0d9b1c595e..614e0d80c3 100644 --- a/engine/crates/layerfs-api/tests/kernel_application.rs +++ b/engine/crates/layerfs-api/tests/kernel_application.rs @@ -39,6 +39,7 @@ fn connect(address: SocketAddr) -> duplex::Connection { } #[test] fn actual_daemon_install_hello_bind_status_no_constructor_and_session_end() { + support::shared_control(); let f = support::Fixture::new("application", None); fs::set_permissions(&f.directory, fs::Permissions::from_mode(0o700)).unwrap(); let setup = DaemonSetup { diff --git a/engine/crates/layerfs-api/tests/observed_application.rs b/engine/crates/layerfs-api/tests/observed_application.rs index 280904d3ec..5fe9ff34a9 100644 --- a/engine/crates/layerfs-api/tests/observed_application.rs +++ b/engine/crates/layerfs-api/tests/observed_application.rs @@ -234,6 +234,7 @@ fn task_ids(process: &Process) -> BTreeSet { } #[test] fn equal_calls_and_reused_slot_have_distinct_public_observation_identity() { + support::shared_control(); let f = support::Fixture::new("observed-correlation", None); let log = f.directory.join("stderr.log"); let (process, address, mut initial) = start(&f, File::create(&log).unwrap()); @@ -334,6 +335,7 @@ fn equal_calls_and_reused_slot_have_distinct_public_observation_identity() { } #[test] fn diagnostic_sink_failure_does_not_repeat_or_hide_successful_bind() { + support::shared_control(); let f = support::Fixture::new("observed-failed-sink", None); let (process, address, mut control) = start(&f, File::options().write(true).open("/dev/full").unwrap()); @@ -393,6 +395,7 @@ fn timing_names(telemetry: &layerfs_bridge::control::SdkValue) -> Vec { } #[test] fn traced_diagnostic_failure_is_reported_and_keeps_the_connection() { + support::shared_control(); let f = support::Fixture::new("observed-traced-sink", None); let (process, _address, mut control) = start(&f, File::options().write(true).open("/dev/full").unwrap()); @@ -441,6 +444,7 @@ fn traced_diagnostic_failure_is_reported_and_keeps_the_connection() { } #[test] fn traced_records_share_the_scope_and_untraced_replies_stay_plain() { + support::shared_control(); let f = support::Fixture::new("observed-traced-records", None); let log = f.directory.join("stderr.log"); let (process, _address, mut control) = start(&f, File::create(&log).unwrap()); @@ -503,6 +507,7 @@ fn width(telemetry: &layerfs_bridge::control::SdkValue, path: &[&str]) -> usize /// carries its delta selection work. #[test] fn traced_calls_carry_sql_work_and_status_unmount_carry_opcodes() { + support::shared_control(); let f = support::Fixture::new("observed-traced-work", None); let log = f.directory.join("stderr.log"); let (process, _address, mut control) = start(&f, File::create(&log).unwrap()); @@ -599,6 +604,7 @@ fn traced_calls_carry_sql_work_and_status_unmount_carry_opcodes() { } #[test] fn resource_and_output_failure_preserve_original_status_reply() { + support::shared_control(); let f = support::Fixture::new("observed-resource-and-sink", None); let (process, address, mut control) = start(&f, File::options().write(true).open("/dev/full").unwrap()); @@ -676,6 +682,7 @@ impl Drop for MountGuard { /// it makes none. Creates are ordinary processes of the command identity. #[test] fn configured_serial_low_water_reaches_a_mounted_create_on_both_startup_routes() { + support::shared_control(); use std::os::unix::{fs::MetadataExt, process::CommandExt}; const LOW: u64 = layerfs_workspace::SERIAL_REFILL - 1; let f = support::Fixture::new("observed-low-water", None); @@ -812,6 +819,7 @@ fn configured_serial_low_water_reaches_a_mounted_create_on_both_startup_routes() /// one the test above starts with. #[test] fn startup_refuses_a_serial_low_water_at_or_above_the_refill_window() { + support::shared_control(); let f = support::Fixture::new("observed-low-water-refused", None); let overlay = f.directory.join("overlay.sqlite"); for refused in [ @@ -881,6 +889,7 @@ fn startup_refuses_a_serial_low_water_at_or_above_the_refill_window() { /// it stays; an open connection does not. #[test] fn an_idle_daemon_exits_and_a_workspace_keeps_it() { + support::shared_control(); let f = support::Fixture::new("observed-idle-exit", None); let log = f.directory.join("stderr.log"); let mounts = f.directory.join("mounts"); diff --git a/engine/crates/layerfs-api/tests/r5_1_actual_daemon_commit.rs b/engine/crates/layerfs-api/tests/r5_1_actual_daemon_commit.rs index be1a431559..22108474eb 100644 --- a/engine/crates/layerfs-api/tests/r5_1_actual_daemon_commit.rs +++ b/engine/crates/layerfs-api/tests/r5_1_actual_daemon_commit.rs @@ -299,6 +299,9 @@ fn r5_1_the_actual_daemon_commits_a_change_by_external_bash_and_a_fresh_mount_is .attach(bound.token) .unwrap(); let guard = MountGuard(ready.directory.clone()); + // From the shared control tree, or else the daemon's own beside its + // Overlay file, as in a Sandbox container. + assert!(ready.receipt.abort_bound, "{:?}", ready.receipt); assert!(mounted_at(&ready.directory)); let mount = Path::new(&ready.directory); println!( @@ -446,5 +449,19 @@ fn r5_1_the_actual_daemon_commits_a_change_by_external_bash_and_a_fresh_mount_is drop(process); fs::remove_dir_all(&replayed).unwrap(); fs::remove_dir(&mounts).unwrap(); + // The daemon's own control tree outlives it in this namespace. + let private = f.directory.join("fusectl"); + let tree = if private.exists() { + let status = Command::new("umount") + .arg(&private) + .stdin(Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "umount {}: {status:?}", private.display()); + "private" + } else { + "shared" + }; + println!("{name} control_tree={tree} abort_bound=true"); f.cleanup(); } diff --git a/engine/crates/layerfs-daemon/src/application/filesystem.rs b/engine/crates/layerfs-daemon/src/application/filesystem.rs index 095cae93bb..a71a5e82b9 100644 --- a/engine/crates/layerfs-daemon/src/application/filesystem.rs +++ b/engine/crates/layerfs-daemon/src/application/filesystem.rs @@ -40,7 +40,7 @@ mod linux { }; use std::{ fs::{self, OpenOptions, Permissions}, - io, + io::{self, Write}, os::unix::fs::{DirBuilderExt, FileTypeExt, MetadataExt, OpenOptionsExt, PermissionsExt}, path::Path, sync::Arc, @@ -61,6 +61,21 @@ mod linux { let mounts = Path::new(&setup.mounts); device()?; home(mounts)?; + // Beside the Overlay file, in the directory configuration checked + // private, so no command identity can reach another connection. + // Best effort: where the mount is refused (a user namespace refuses + // it, though not FUSE mounts) the shared tree, unmounted there, binds + // nothing and forced unmount is refused before any effect, as it was + // before this mount existed. The refusal is written once to stderr. + let overlay = Path::new(&setup.overlay).parent(); + let private = overlay.ok_or_else(|| invalid("Overlay parent"))?; + let control = match layerfs_fuse::mount::control_root(&private.join("fusectl")) { + Ok(control) => control, + Err(cause) => { + let _ = writeln!(io::stderr().lock(), "LAYERFS_DAEMON_ABORT_UNBOUND {cause}"); + layerfs_fuse::mount::CONTROL.into() + } + }; KernelServing::start(KernelConfig { mounts: mounts.to_owned(), owner_uid: Uid::effective().as_raw(), @@ -69,6 +84,7 @@ mod linux { command_gid: setup.command_gid, ready_wait: READY_WAIT, drain_wait: DRAIN_WAIT, + control, }) .map(|serving| Some(Arc::new(serving))) .map_err(|failure| io::Error::other(failure.to_string())) diff --git a/engine/crates/layerfs-daemon/src/control/attach.rs b/engine/crates/layerfs-daemon/src/control/attach.rs index 2c300bd5b4..0478136630 100644 --- a/engine/crates/layerfs-daemon/src/control/attach.rs +++ b/engine/crates/layerfs-daemon/src/control/attach.rs @@ -2,7 +2,7 @@ use super::{ kernel::Kernel, registry::{bound_mut, idle}, - serving::{ready, stage, Attached, Kept, KernelServing}, + serving::{ready, stage, Attached, Evidence, Kept, KernelServing}, types::KernelFailure, Failure, Service, Success, }; @@ -164,6 +164,7 @@ impl Service { owner_gid: config.owner_gid, ready_wait: config.ready_wait, drain_wait: config.drain_wait, + control: config.control.clone(), }, queue, Arc::new(crate::service::Mounted(workspace.clone())), @@ -184,7 +185,7 @@ impl Service { stage: TeardownStage::Detach, detail: "serving session without negotiation receipt".into(), observer: Some(session.observer()), - evidence: Box::new(session), + evidence: Evidence::Other(Box::new(session)), forced: None, }))), }; @@ -205,7 +206,7 @@ impl Service { stage, detail, observer, - evidence: Box::new((failure, mount)), + evidence: Evidence::Other(Box::new((failure, mount))), forced: None, }))); } @@ -236,7 +237,7 @@ impl Service { stage: TeardownStage::Revoke, detail: failure.detail.clone(), observer: None, - evidence: Box::new((failure, mount, other)), + evidence: Evidence::Other(Box::new((failure, mount, other))), forced: None, })), } diff --git a/engine/crates/layerfs-daemon/src/control/detach.rs b/engine/crates/layerfs-daemon/src/control/detach.rs index 2e400e039b..e84e879aa7 100644 --- a/engine/crates/layerfs-daemon/src/control/detach.rs +++ b/engine/crates/layerfs-daemon/src/control/detach.rs @@ -2,14 +2,14 @@ use super::{ kernel::Kernel, registry::bound_mut, - serving::{stage, Attached, Kept, Leaving}, + serving::{stage, Attached, Evidence, Kept, Leaving}, Failure, Service, Success, }; use crate::store::BoundWorkspace; use layerfs_bridge::control::{ Activity, ControlCode, ForcedFacts, MountPhase, TeardownStage, WorkspaceToken, }; -use layerfs_fuse::session::{Detach, SessionObserver}; +use layerfs_fuse::session::{Detach, DrainStage, SessionObserver}; use std::{fmt, sync::Arc}; /// Terminal admission over a connection-owning entry. No daemon-side request @@ -85,17 +85,18 @@ impl Service { let drained = match session.drain() { Ok(drained) => drained, Err(undrained) => { - return Err(self.retain( - token, - stage(undrained.stage), - format!( - "connection drain stopped at {:?}: loops {:?}, work {:?}", - undrained.stage, undrained.loops, undrained.work - ), - Some(observer), - undrained, - None, - )) + 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)); } }; // Drain barrier holds: one fixed logical mark, then indexed retirement. @@ -155,6 +156,19 @@ impl Service { observer: Option, evidence: Box, forced: Option, + ) -> Failure { + let evidence = Evidence::Other(evidence); + self.keep(token, stage, detail, observer, evidence, forced) + } + /// The same, with the owner already sorted into its kind. + pub(super) fn keep( + &self, + token: WorkspaceToken, + stage: TeardownStage, + detail: String, + observer: Option, + evidence: Evidence, + forced: Option, ) -> Failure { let mut entries = self.entries.lock().unwrap_or_else(|e| e.into_inner()); let mut kept = Kept { diff --git a/engine/crates/layerfs-daemon/src/control/force.rs b/engine/crates/layerfs-daemon/src/control/force.rs index c7baa82537..6eb780c616 100644 --- a/engine/crates/layerfs-daemon/src/control/force.rs +++ b/engine/crates/layerfs-daemon/src/control/force.rs @@ -1,7 +1,9 @@ //! Forced terminal unmount: a before-effect guard, one abort write, the local //! drain with its one plain detach, then Revoke and Close. Nothing is repeated. +//! A normal unmount stalled at Join is forced the same way, its plain detach +//! counting as the one detach. #[cfg(target_os = "linux")] -use super::serving::{stage, work, Attached}; +use super::serving::{stage, work, Attached, Evidence, Leaving}; use super::{ kernel::Kernel, registry::{bound_mut, Binding}, @@ -48,9 +50,14 @@ fn guard( token: WorkspaceToken, relinquish_unknown: bool, ) -> Result { - // Stored custody answers first: no second abort or detach ever follows it. + // Stored custody answers first: no second abort or detach ever follows + // it. The one exception is a normal unmount stalled at Join, admitted + // idle with no Commit custody, whose abort control is still unused. #[cfg(target_os = "linux")] if let Kernel::Retained(kept) = &value.kernel { + if kept.forceable() { + return Ok(CommitKnowledge::Absent); + } return Err(Failure::Retained(Box::new(kept.custody(token)))); } #[cfg(not(target_os = "linux"))] @@ -149,6 +156,31 @@ struct Admitted { /// Restored only if the abort is refused before any effect. prior: Activity, commit: CommitKnowledge, + /// Taken from a normal unmount stalled at Join: already detached. + stalled: bool, +} +/// Moves a stalled normal unmount's session out of its retained custody, +/// which `guard` checked, into the forced teardown's own first phase. +#[cfg(target_os = "linux")] +fn take_stalled(kernel: &mut Kernel) -> Box { + let Kernel::Retained(kept) = std::mem::replace(kernel, Kernel::Attaching) else { + unreachable!("checked retained custody") + }; + let kept = *kept; + let (Some(ready), Some(observer), Evidence::Stalled(undrained)) = + (kept.ready, kept.observer, kept.evidence) + else { + unreachable!("checked stalled custody") + }; + *kernel = Kernel::Leaving(Box::new(Leaving { + ready: ready.clone(), + observer, + phase: MountPhase::Stopping, + })); + Box::new(Attached { + session: undrained.session, + ready, + }) } #[cfg(target_os = "linux")] fn detach(attempt: DetachAttempt) -> DetachDisposition { @@ -180,7 +212,12 @@ impl Service { let mut entries = self.entries.lock().map_err(|_| Failure::Poisoned)?; let value = bound_mut(&mut entries, token)?; let commit = guard(value, token, relinquish_unknown)?; - let attached = super::detach::take(&mut value.kernel, MountPhase::Stopping)?; + let stalled = matches!(value.kernel, Kernel::Retained(_)); + let attached = if stalled { + take_stalled(&mut value.kernel) + } else { + super::detach::take(&mut value.kernel, MountPhase::Stopping)? + }; let prior = value.activity; value.activity = Activity::Closing; value.epoch = value.epoch.saturating_add(1); @@ -189,6 +226,7 @@ impl Service { attached, prior, commit, + stalled, }) } /// Each step is made once. A step that is not established keeps the exact @@ -204,18 +242,40 @@ impl Service { attached, prior, commit, + stalled, } = admitted; let Attached { mut session, ready } = *attached; let mount = session.mount(); let observer = session.observer(); let mut facts = ForcedFacts { abort: AbortDisposition::Written, - detach: DetachDisposition::NotAttempted, + detach: if stalled { + DetachDisposition::Detached + } else { + DetachDisposition::NotAttempted + }, commit, fenced: 0, }; - let written = match session.abort() { + let aborted = if stalled { + session.abort_detached() + } else { + session.abort() + }; + let written = match aborted { Ok(written) => written, + // Admitted under the lock with its control bound and unused, so + // this cannot happen; the detached owner is kept, never released. + Err(refusal) if stalled => { + return Err(self.retain( + token, + TeardownStage::Join, + format!("abort refused before any effect: {refusal:?}"), + Some(observer), + Box::new(session), + None, + )) + } Err(refusal) => { // Refused before the write: the same connection returns with // the activity it had. diff --git a/engine/crates/layerfs-daemon/src/control/kernel.rs b/engine/crates/layerfs-daemon/src/control/kernel.rs index 14d266041f..92c07ef476 100644 --- a/engine/crates/layerfs-daemon/src/control/kernel.rs +++ b/engine/crates/layerfs-daemon/src/control/kernel.rs @@ -17,6 +17,8 @@ pub struct KernelConfig { pub ready_wait: Duration, /// Observation deadline for loop joins and request drain. pub drain_wait: Duration, + /// Control tree each connection's abort control is bound from. + pub control: PathBuf, } /// Kernel serving is a Linux capability; elsewhere it cannot be constructed. #[cfg(not(target_os = "linux"))] diff --git a/engine/crates/layerfs-daemon/src/control/serving.rs b/engine/crates/layerfs-daemon/src/control/serving.rs index 180f16a184..1385b560fd 100644 --- a/engine/crates/layerfs-daemon/src/control/serving.rs +++ b/engine/crates/layerfs-daemon/src/control/serving.rs @@ -5,7 +5,7 @@ use layerfs_bridge::control::{ WorkspaceToken, }; use layerfs_fuse::{ - session::{DrainStage, KernelSession, SessionFacts, SessionObserver}, + session::{DrainStage, KernelSession, SessionFacts, SessionObserver, Undrained}, DispatchError, MountRequests, }; use layerfs_overlay::KernelMount; @@ -56,9 +56,8 @@ pub(super) struct Kept { pub stage: TeardownStage, pub detail: String, pub observer: Option, - /// The original owner: an undrained session, a drained receipt or a - /// refused engine completion, exactly as the stopping boundary left it. - pub evidence: Box, + /// 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. pub forced: Option, @@ -75,7 +74,33 @@ impl fmt::Debug for Kept { .finish() } } +/// What a retained entry owns. Only `Stalled` can still be ended, by one +/// forced unmount; every other owner is kept as it is. +pub(super) enum Evidence { + /// A normal unmount's drain that stopped at `Join` after its plain detach. + Stalled(Box), + /// An undrained session, a drained receipt or a refused engine completion. + Other(Box), +} +/// Shows the owner itself, as the retained-owner observation always has. +impl fmt::Debug for Evidence { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Stalled(undrained) => undrained.fmt(f), + Self::Other(owner) => owner.fmt(f), + } + } +} impl Kept { + /// A stalled normal unmount whose abort control is bound and unused. + pub(super) fn forceable(&self) -> bool { + let facts = self.observer.as_ref().map(SessionObserver::facts); + matches!(self.evidence, Evidence::Stalled(_)) + && self.forced.is_none() + && self.ready.is_some() + && facts + .is_some_and(|facts| facts.detached && facts.abort_bound && facts.aborted.is_none()) + } pub(super) fn custody(&self, token: WorkspaceToken) -> TeardownCustody { let facts = self.observer.as_ref().map(SessionObserver::facts); TeardownCustody { diff --git a/engine/crates/layerfs-daemon/tests/control_root.rs b/engine/crates/layerfs-daemon/tests/control_root.rs new file mode 100644 index 0000000000..4f1ea9cda1 --- /dev/null +++ b/engine/crates/layerfs-daemon/tests/control_root.rs @@ -0,0 +1,126 @@ +//! The control tree a daemon binds abort controls from (`mount::control_root`). +//! +//! Needs root with `CAP_SYS_ADMIN`, as in the nightly mount lane. Where the +//! shared tree is mounted it is the answer and nothing else is checked here; +//! otherwise, as in a container, the private cases run: one mount at a new +//! directory, reuse of that mount on a later call, and refusal of an +//! existing directory that holds none. Each case removes what it mounted. +//! +//! A user namespace, as rootless engines give a container, refuses this mount +//! though not FUSE mounts. The refusal case runs this binary again inside +//! `unshare -Urm`: the mount's own error comes back and no directory is left, +//! so the daemon starts with its abort control unbound. +#![cfg(target_os = "linux")] +use layerfs_fuse::mount::{control_root, CONTROL}; +use std::{ + fs, io, + path::{Path, PathBuf}, + process::{Command, Stdio}, +}; + +/// The filesystem type mounted at exactly `path` in this namespace. +fn mounted_type(path: &Path) -> Option { + let table = fs::read_to_string("/proc/self/mountinfo").unwrap(); + table.lines().rev().find_map(|line| { + let (before, after) = line.split_once(" - ")?; + (before.split(' ').nth(4) == path.to_str()).then(|| after.split(' ').next().unwrap().into()) + }) +} +fn umount(path: &Path) { + let status = Command::new("umount") + .arg(path) + .stdin(Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "umount {}: {status:?}", path.display()); +} +fn scratch(label: &str) -> PathBuf { + let home = std::env::temp_dir().join(format!("layerfs-control-{}-{label}", std::process::id())); + fs::create_dir(&home).unwrap(); + home +} + +#[test] +fn a_private_control_tree_is_mounted_once_reused_and_never_adopted() { + if mounted_type(Path::new(CONTROL)).as_deref() == Some("fusectl") { + let home = scratch("shared"); + let root = control_root(&home.join("fusectl")).unwrap(); + assert_eq!(root, Path::new(CONTROL)); + assert!(!home.join("fusectl").exists(), "a private tree was made"); + fs::remove_dir(&home).unwrap(); + println!("CONTROL_ROOT shared tree mounted: private cases not run"); + return; + } + + // One mount at a new directory, private to its owner. + let home = scratch("private"); + let private = home.join("fusectl"); + let root = control_root(&private).unwrap(); + assert_eq!(root, private); + assert_eq!(mounted_type(&private).as_deref(), Some("fusectl")); + println!("CONTROL_ROOT private={}", private.display()); + + // A later call, as after a daemon restart in the same namespace, reuses + // the mount rather than making a second one. + let again = control_root(&private).unwrap(); + assert_eq!(again, private); + let count = fs::read_to_string("/proc/self/mountinfo") + .unwrap() + .lines() + .filter(|line| line.split(' ').nth(4) == private.to_str()) + .count(); + assert_eq!(count, 1, "the tree was mounted again"); + umount(&private); + + // An existing directory without the mount is refused, not adopted. + let refused = control_root(&private).unwrap_err(); + assert_eq!(refused.kind(), io::ErrorKind::AlreadyExists, "{refused}"); + assert_eq!(mounted_type(&private), None); + println!("CONTROL_ROOT leftover_directory={:?}", refused.kind()); + + fs::remove_dir(&private).unwrap(); + fs::remove_dir(&home).unwrap(); +} + +/// The refusal half, run only inside a new user namespace by the test below. +#[test] +#[ignore = "run inside `unshare -Urm` by a_refused_control_mount_leaves_no_directory"] +fn refused_inside_a_user_namespace() { + let home = scratch("refused"); + let private = home.join("fusectl"); + let refused = control_root(&private).unwrap_err(); + assert_eq!(refused.kind(), io::ErrorKind::PermissionDenied, "{refused}"); + assert!(!private.exists(), "a refused mount left its directory"); + fs::remove_dir(&home).unwrap(); + println!("CONTROL_ROOT_REFUSED {refused}"); +} + +#[test] +fn a_refused_control_mount_leaves_no_directory() { + let output = Command::new("unshare") + .arg("-Urm") + .arg(std::env::current_exe().unwrap()) + .args([ + "--exact", + "refused_inside_a_user_namespace", + "--ignored", + "--nocapture", + ]) + .stdin(Stdio::null()) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + output.status.success() && stdout.contains("CONTROL_ROOT_REFUSED"), + "{:?}\n{stdout}\n{}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + println!( + "{}", + stdout + .lines() + .find(|line| line.starts_with("CONTROL_ROOT_REFUSED")) + .unwrap() + ); +} diff --git a/engine/crates/layerfs-daemon/tests/mounted_sqlite_full.rs b/engine/crates/layerfs-daemon/tests/mounted_sqlite_full.rs index 24239accad..76b98ddb48 100644 --- a/engine/crates/layerfs-daemon/tests/mounted_sqlite_full.rs +++ b/engine/crates/layerfs-daemon/tests/mounted_sqlite_full.rs @@ -235,6 +235,7 @@ fn run(forced: bool, device: bool) { owner_gid: 0, ready_wait: Duration::from_secs(3), drain_wait: Duration::from_secs(3), + control: layerfs_fuse::mount::CONTROL.into(), }, queue.clone(), Arc::new(ObserveMount { diff --git a/engine/crates/layerfs-daemon/tests/stalled_unmount.rs b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs new file mode 100644 index 0000000000..43da7ea764 --- /dev/null +++ b/engine/crates/layerfs-daemon/tests/stalled_unmount.rs @@ -0,0 +1,208 @@ +//! A normal unmount whose connection outlives its mount (issues #426, #460). +//! +//! Real kernel mount, staging, hook-free. After Attach, `unshare -m` makes a +//! second mount namespace that holds a copy of the Workspace mount, the way an +//! 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 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")] +#[allow(dead_code)] +#[path = "support/fusectl.rs"] +mod fusectl; +#[allow(dead_code)] +#[path = "support/installed_store.rs"] +mod installed; +#[allow(dead_code)] +#[path = "support/namespace_model.rs"] +mod model; +#[allow(dead_code)] +#[path = "support/mounted.rs"] +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 mounted::{mount_entry, until}; +use rig::{root, Rig}; +use std::{ + io::{BufRead, BufReader, Write}, + path::Path, + process::{Child, ChildStdin, Command, Stdio}, + sync::mpsc::{self, Receiver}, + thread, + time::{Duration, Instant}, +}; + +/// Every bounded readiness or completion observation of the test. +const WAIT: Duration = Duration::from_secs(5); + +/// Runs in its own private copy of the mount namespace, so it holds a copy of +/// the Workspace mount without touching it. It answers one line for each line +/// it is sent: `probe` looks up a name never looked up before under the +/// copied mount; anything else ends it. +const COPY: &str = r#"echo held +n=0 +while read word; do + [ "$word" = probe ] || exit 0 + n=$((n + 1)) + if out=$(stat "$1/stall-probe-$n" 2>&1); then echo ok; else echo "$out"; fi +done"#; + +/// The process holding the copied mount. Dropped, it is killed and reaped. +struct Copy { + child: Child, + stdin: ChildStdin, + lines: Receiver, +} +impl Copy { + fn spawn(mount: &Path) -> Self { + let mut child = Command::new("unshare") + .args(["-m", "--propagation", "private", "sh", "-c", COPY, "copy"]) + .arg(mount) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .unwrap(); + let stdout = child.stdout.take().unwrap(); + let stdin = child.stdin.take().unwrap(); + let (send, lines) = mpsc::channel(); + // Ends when the process's output closes, whatever the test does. + thread::spawn(move || { + for line in BufReader::new(stdout).lines() { + if send.send(line.unwrap_or_default()).is_err() { + break; + } + } + }); + let mut copy = Self { + child, + stdin, + lines, + }; + assert_eq!(copy.line(), "held"); + copy + } + fn line(&mut self) -> String { + self.lines + .recv_timeout(WAIT) + .expect("the copy answered within its wait") + } + fn ask(&mut self, word: &str) -> String { + writeln!(self.stdin, "{word}").unwrap(); + self.line() + } + /// Asks it to leave and waits a bounded time for its own exit. + fn leave(&mut self) -> bool { + let _ = writeln!(self.stdin, "exit"); + let deadline = Instant::now() + WAIT; + while Instant::now() < deadline { + if self.child.try_wait().unwrap().is_some() { + return true; + } + thread::sleep(Duration::from_millis(10)); + } + false + } +} +impl Drop for Copy { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +#[test] +fn a_normal_unmount_stalled_at_join_is_ended_by_one_forced_unmount() { + fusectl::mount(); + let rig = Rig::new("stalled-join"); + let ready = rig.mount(1); + let token = ready.token; + assert!(ready.receipt.abort_bound, "{:?}", ready.receipt); + 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) + ), + }; + 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:?}" + ); + 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. + until("own connection entry removed", || !connection.exists()); + let answer = copy.ask("probe"); + assert!( + answer.contains("No such file or directory"), + "copy's call answered {answer:?}" + ); + assert!(copy.leave(), "the copy did not exit by itself"); + println!("COPY connection=absent_while_copy_ran probe={answer:?}"); + drop(copy); + rig.finish(); +} diff --git a/engine/crates/layerfs-daemon/tests/support/host_install.rs b/engine/crates/layerfs-daemon/tests/support/host_install.rs index ba400a0983..39856c0c71 100644 --- a/engine/crates/layerfs-daemon/tests/support/host_install.rs +++ b/engine/crates/layerfs-daemon/tests/support/host_install.rs @@ -21,6 +21,32 @@ pub struct Fixture { pub directory: PathBuf, pub project: SealedProject, } +/// The kernel's shared FUSE control tree, mounted once in this namespace. +/// An actual daemon binds abort controls from it; without it the daemon +/// mounts its own beside its Overlay file, inside this fixture, where it +/// outlives the daemon and blocks the fixture's removal. Needs root. +#[cfg(target_os = "linux")] +#[allow(dead_code)] +pub fn shared_control() { + use std::process::{Command, Stdio}; + // Once per test process: parallel tests would otherwise race to mount it. + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| { + let shared = layerfs_fuse::mount::CONTROL; + let table = fs::read_to_string("/proc/self/mountinfo").unwrap(); + let mounted = table + .lines() + .any(|line| line.split(' ').nth(4) == Some(shared) && line.contains(" - fusectl ")); + if !mounted { + let status = Command::new("mount") + .args(["-t", "fusectl", "none", shared]) + .stdin(Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "mount fusectl at {shared}: {status:?}"); + } + }); +} impl Fixture { pub fn new(label: &str, locator: Option<&str>) -> Self { let directory = std::env::temp_dir().join(format!( diff --git a/engine/crates/layerfs-daemon/tests/support/mounted.rs b/engine/crates/layerfs-daemon/tests/support/mounted.rs index f4c6e48cee..da97018438 100644 --- a/engine/crates/layerfs-daemon/tests/support/mounted.rs +++ b/engine/crates/layerfs-daemon/tests/support/mounted.rs @@ -89,6 +89,7 @@ impl Harness { command_gid: COMMAND, ready_wait: Duration::from_secs(3), drain_wait: Duration::from_secs(3), + control: layerfs_fuse::mount::CONTROL.into(), }) .unwrap(), ); diff --git a/engine/crates/layerfs-fuse/src/mount/mod.rs b/engine/crates/layerfs-fuse/src/mount/mod.rs index 9c691e62b0..278d5031e6 100644 --- a/engine/crates/layerfs-fuse/src/mount/mod.rs +++ b/engine/crates/layerfs-fuse/src/mount/mod.rs @@ -4,4 +4,4 @@ pub(crate) mod syscalls; pub(crate) use profile::negotiate; pub use profile::Negotiation; -pub use syscalls::{AbortWrite, MountEntry}; +pub use syscalls::{control_root, AbortWrite, MountEntry, CONTROL}; diff --git a/engine/crates/layerfs-fuse/src/mount/syscalls.rs b/engine/crates/layerfs-fuse/src/mount/syscalls.rs index 25c2c2c2aa..0d16e8517e 100644 --- a/engine/crates/layerfs-fuse/src/mount/syscalls.rs +++ b/engine/crates/layerfs-fuse/src/mount/syscalls.rs @@ -1,5 +1,6 @@ //! First-party device, direct mount, one plain detach and one abort write; no -//! helper or lazy path. +//! helper or lazy path. Also the one control-filesystem mount a daemon makes +//! when its container has none. use nix::{ errno::Errno, mount::{mount, umount2, MntFlags, MsFlags}, @@ -13,12 +14,14 @@ use std::{ fd::{AsRawFd, OwnedFd}, unix::{ffi::OsStrExt, fs::DirBuilderExt}, }, - path::Path, + path::{Path, PathBuf}, }; const DEVICE: &str = "/dev/fuse"; const MOUNT_TABLE: &str = "/proc/self/mountinfo"; -const CONTROL: &str = "/sys/fs/fuse/connections"; +/// The kernel's shared FUSE control tree, where a host mounts it. +pub const CONTROL: &str = "/sys/fs/fuse/connections"; +const CONTROL_TYPE: &str = "fusectl"; /// The type and source this product mounts with, and the only mount-table row /// whose connection it will ever bind an abort control for. const FILESYSTEM: &str = "fuse"; @@ -97,19 +100,41 @@ pub(crate) fn mount_entry(target: &Path) -> io::Result { /// product's own row names one: an anonymous device (major 0, whose minor is /// the connection number) of type `fuse` with this product's source. Any /// other row binds nothing and opens nothing. -pub(crate) fn abort_control(entry: &MountEntry) -> io::Result> { +pub(crate) fn abort_control(entry: &MountEntry, control: &Path) -> io::Result> { if entry.device.0 != 0 || entry.filesystem != FILESYSTEM || entry.source != SOURCE { return Ok(None); } - let path = Path::new(CONTROL) - .join(entry.device.1.to_string()) - .join("abort"); + let path = control.join(entry.device.1.to_string()).join("abort"); match OpenOptions::new().write(true).open(path) { Ok(file) => Ok(Some(file)), Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(None), Err(error) => Err(error), } } +/// The control tree this process binds its own abort controls from. The +/// shared tree is used when the control filesystem is mounted there. +/// Otherwise, as in a container, which mounts none, one mount of it is made +/// at `private`, a new directory inside one only this process can reach: +/// the tree lists every connection of the kernel, other containers' +/// included. A `private` that already holds it is used as it is; an +/// existing directory without it is never adopted. A refused mount (a user +/// namespace refuses it, though not FUSE mounts) removes the directory this +/// call made and returns the mount's own error. +pub fn control_root(private: &Path) -> io::Result { + for root in [Path::new(CONTROL), private] { + if mount_entry(root).is_ok_and(|entry| entry.filesystem == CONTROL_TYPE) { + return Ok(root.to_owned()); + } + } + DirBuilder::new().mode(0o700).create(private)?; + let flags = MsFlags::MS_NOSUID | MsFlags::MS_NODEV | MsFlags::MS_NOEXEC; + let source = Some(CONTROL_TYPE); + if let Err(errno) = mount(source, private, source, flags, None::<&str>) { + let _ = std::fs::remove_dir(private); + return Err(errno.into()); + } + Ok(private.to_owned()) +} /// Exactly one `write(2)` of one byte on the bound control: no second call /// after a short count, an interruption or any other error. pub(crate) fn abort(control: &File) -> AbortWrite { diff --git a/engine/crates/layerfs-fuse/src/session/force.rs b/engine/crates/layerfs-fuse/src/session/force.rs index ba1d07f96f..156878576a 100644 --- a/engine/crates/layerfs-fuse/src/session/force.rs +++ b/engine/crates/layerfs-fuse/src/session/force.rs @@ -18,6 +18,25 @@ impl KernelSession { if self.is_detached() || self.aborted.get().is_some() { return Err(ForceRefusal::NotServing); } + self.write_abort() + } + /// The single abort write for a connection 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. That detach is the + /// connection's one detach; `force_drain` makes none after this. Refused + /// before any effect unless detached and not yet aborted. + pub fn abort_detached(&mut self) -> Result { + if !self.is_detached() || self.aborted.get().is_some() { + return Err(ForceRefusal::NotServing); + } + if self.abort.is_none() { + return Err(ForceRefusal::AbortUnavailable); + } + self.forced_detach = DetachAttempt::Detached; + self.write_abort() + } + fn write_abort(&mut self) -> Result { let Some(control) = self.abort.take() else { return Err(ForceRefusal::AbortUnavailable); }; @@ -36,7 +55,8 @@ impl KernelSession { /// is attempted only once the local drain held, and never again: `EBUSY` /// or any other errno stops at `Detach` with the connection aborted and /// still mounted. Called again, it observes the same predicate; a - /// recorded failed detach is returned as it was, with no syscall. + /// recorded failed detach is returned as it was, with no syscall. After + /// `abort_detached` the earlier plain detach is that one detach. pub fn force_drain(mut self) -> Result, Box> { let attempted = match self.forced_detach { DetachAttempt::Failed(_) => return Err(self.undrained(DrainStage::Detach, None)), diff --git a/engine/crates/layerfs-fuse/src/session/startup.rs b/engine/crates/layerfs-fuse/src/session/startup.rs index 6314a30507..e5e0f6f9f7 100644 --- a/engine/crates/layerfs-fuse/src/session/startup.rs +++ b/engine/crates/layerfs-fuse/src/session/startup.rs @@ -132,7 +132,7 @@ impl KernelSession { Ok(entry) => entry, Err(cause) => return Err(session.abandon(AttachPhase::Receipt, cause)), }; - match syscalls::abort_control(&entry) { + match syscalls::abort_control(&entry, &config.control) { Ok(abort) => session.abort = abort, Err(cause) => return Err(session.abandon(AttachPhase::Receipt, cause)), } diff --git a/engine/crates/layerfs-fuse/src/session/state.rs b/engine/crates/layerfs-fuse/src/session/state.rs index fc23394c81..71f4793c6b 100644 --- a/engine/crates/layerfs-fuse/src/session/state.rs +++ b/engine/crates/layerfs-fuse/src/session/state.rs @@ -39,6 +39,9 @@ pub struct SessionConfig { pub ready_wait: Duration, /// Observation deadline for loop joins and request drain. pub drain_wait: Duration, + /// Control tree the connection's abort control is bound from; see + /// [`crate::mount::control_root`]. + pub control: PathBuf, } /// One mounted connection. Dropping it performs no detach, abort, join or diff --git a/engine/crates/layerfs-node/src/workspace.rs b/engine/crates/layerfs-node/src/workspace.rs index 991f74e4f3..829df67f71 100644 --- a/engine/crates/layerfs-node/src/workspace.rs +++ b/engine/crates/layerfs-node/src/workspace.rs @@ -13,7 +13,7 @@ use crate::registry::{ }; use crate::sandbox::{operation, WORKSPACES}; use layerfs_api::internal::workspace; -use layerfs_api::sandbox::values; +use layerfs_api::sandbox::{values, Unmounted}; use layerfs_bridge::control::{CommitControl, SdkRefusal, SdkValue}; use layerfs_history::WorkspaceId; use std::path::Path; @@ -249,7 +249,7 @@ pub(crate) fn commit(args: &SdkValue) -> Outcome { values::commit(&reply) } -/// `workspace.unmount(ws, { discard? })`: `Busy` while a background +/// `workspace.unmount(ws, { discard?, force? })`: `Busy` while a background /// process this binding started still runs there, then the daemon's answer. pub(crate) fn unmount(args: &SdkValue) -> Outcome { let state = sandbox_refreshed(args)?; @@ -273,10 +273,21 @@ pub(crate) fn unmount(args: &SdkValue) -> Outcome { )]), }); } - let answer = workspace(&mut *crate::trace::control(&state.controls)?) - .unmount_checked(token, args.get_bool("discard")) - .map_err(operation)?; - let value = values::unmounted(&answer)?; + let value = { + let mut control = crate::trace::control(&state.controls)?; + let mut api = workspace(&mut control); + // Forced: one abort write ends the connection, also after a normal + // unmount stalled at Join; uncommitted changes are dropped. + if args.get_bool("force") { + match api.unmount(token, true).map_err(operation)? { + Some(outcome) => values::force_unmounted(&outcome), + None => values::unmounted(&Unmounted::Closed)?, + } + } else { + let discard = args.get_bool("discard"); + values::unmounted(&api.unmount_checked(token, discard).map_err(operation)?)? + } + }; std::fs::remove_file(crate::discovery::workspace_file(&state, token)).map_err(failed)?; lock(&state.mounts)?.remove(path); Ok(value) diff --git a/engine/crates/layerfs-sandbox/src/backend/docker/topology.rs b/engine/crates/layerfs-sandbox/src/backend/docker/topology.rs index 7ffb2d8c09..9ee9ae628d 100644 --- a/engine/crates/layerfs-sandbox/src/backend/docker/topology.rs +++ b/engine/crates/layerfs-sandbox/src/backend/docker/topology.rs @@ -9,8 +9,10 @@ use std::io::Read; /// namespace (`CLONE_NEWUSER` to `unshare` or `clone`; `clone3` answers /// `ENOSYS`, which the C library follows with `clone`) and allows everything /// else. An unprivileged command can make a mount namespace only inside a -/// user namespace of its own, so with this no copy of a Workspace mount can -/// exist outside the daemon's namespace and a normal unmount stays truthful. +/// user namespace of its own, so with this no command can copy a Workspace +/// mount out of the daemon's namespace. Processes outside the container are +/// not filtered: a copy one makes keeps the connection alive after a plain +/// detach, which the daemon keeps at `Join` for one forced unmount to end. /// It replaces the Engine's default filter; the command identity holds no /// capability and, with no user namespace, cannot gain one. pub(super) const SECCOMP: &str = concat!(