From f6e68e82801b575774508e40de229a77dedb43f8 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 13:37:00 -0700 Subject: [PATCH 1/8] nvx-rs: lease eligible templates to clones, and retire them A generation of a template must stay in its store while a clone may still restore it, and no clone may restore one outside the restore window of its capture's time contract, which OpenVMM refuses only once the restore has begun. Add the eligibility of a capture at a host's time, apart from its compatibility: TimeContract::eligibility measures the downtime as OpenVMM's restore does, by the host's monotonic clock within the boot of the capture and by UTC otherwise, and refuses a negative downtime, and one that would advance the clone's VM time by more than 30 days. HostTime::now samples the identities and clocks of the host as OpenVMM does. Add the leases of a store's generations. Store::acquire leases an eligible generation that no tombstone quarantines to a provisioned sandbox that records no VM, launch, or lease, in one transaction under the store's lock and then the sandbox's: it writes the lease's record in the store, and the same record as the clone binding, clone.json, beside the sandbox's launch journal, and returns the sandbox's lock, under which the caller launches the clone. A lease never expires: Store::release ends it once the sandbox records no VM and no launch, and so once the clone's VM has exited, as the sandbox's store verifies through its recorded process. Store::select picks the latest eligible generation of a key. Store::tombstone quarantines a generation, which takes no new lease while its leases keep working, and Store::retire deletes it once its leases have drained: its payload, then its entry in the index, then its tombstone, and it removes the records of leases that no clone binding names any more. Each of these transactions makes its steps durable in order, syncing directories where the platform can, so that no clone binding outlasts, through a crash of the host, the record that keeps its generation in the store, even when the store and the sandbox are on different filesystems. The sandbox store counts the clone binding among its files, so that a removal of a sandbox removes it, and three error codes name the new refusals: template-ineligible, template-tombstoned, and sandbox-leased. The documentation of foreign-files now covers the template store too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- doc/usage.md | 3 + nvx-rs/Cargo.toml | 1 + nvx-rs/README.md | 15 +- nvx-rs/src/error.rs | 32 +- nvx-rs/src/host/mod.rs | 2 +- nvx-rs/src/host/platform/linux.rs | 29 + nvx-rs/src/host/platform/mod.rs | 59 + nvx-rs/src/host/platform/other.rs | 10 + nvx-rs/src/host/platform/windows.rs | 64 +- nvx-rs/src/host/store/files.rs | 29 +- nvx-rs/src/host/store/mod.rs | 15 +- nvx-rs/src/host/template/eligibility.rs | 224 +++ nvx-rs/src/host/template/eligibility/tests.rs | 276 ++++ nvx-rs/src/host/template/mod.rs | 14 +- nvx-rs/src/host/template/store.rs | 53 +- nvx-rs/src/host/template/store/leases.rs | 986 ++++++++++++ .../src/host/template/store/leases/tests.rs | 1338 +++++++++++++++++ nvx-rs/src/host/template/store/tests.rs | 24 +- nvx-rs/src/lib.rs | 2 +- 19 files changed, 3141 insertions(+), 35 deletions(-) create mode 100644 nvx-rs/src/host/template/eligibility.rs create mode 100644 nvx-rs/src/host/template/eligibility/tests.rs create mode 100644 nvx-rs/src/host/template/store/leases.rs create mode 100644 nvx-rs/src/host/template/store/leases/tests.rs diff --git a/doc/usage.md b/doc/usage.md index 0a4b769af..2a4ee581a 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -109,6 +109,9 @@ each code for callers of the `nvx` library. | `foreign-files` | 1 | `deprovision` found files that NVX does not own in the state directory, which the message names, and kept them with the directory. | | `outcome-report-exists` | 1 | A file exists at the `--outcome-report` path. | | `scratch-claimed` | 1 | Another sandbox claims the scratch image. | +| `sandbox-leased` | 1 | The sandbox holds a lease of a template's generation already, which a release ends once its VM has exited. | +| `template-ineligible` | 1 | The restore window of a template's generation, which the time contract of its capture sets, excludes the host's time, so no clone may restore it now. | +| `template-tombstoned` | 1 | A tombstone quarantines the template's generation, which takes no new clones. | | `exited` | 1 | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one. | | `exec-refused` | 1 | The guest agent refused the workload, as for a working directory that it cannot use. | | `endpoint-closed` | 1 | OpenVMM closed the control endpoint, as it does when the VM shuts down. | diff --git a/nvx-rs/Cargo.toml b/nvx-rs/Cargo.toml index 4f4c9271e..9750c94af 100644 --- a/nvx-rs/Cargo.toml +++ b/nvx-rs/Cargo.toml @@ -68,6 +68,7 @@ windows-sys = { version = "0.61", optional = true, features = [ "Win32_System_Pipes", "Win32_System_Registry", "Win32_System_Threading", + "Win32_System_WindowsProgramming", ] } # The host and the fake OpenVMM also build on Unix hosts other than Linux, such as macOS. diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 32c414eb6..2300007b0 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -47,10 +47,10 @@ nvx-rs/ │ │ sides, without I/O, threads, or features, for any framing and host │ ├── host/ feature `host` (default): artifacts, launch, network and egress policy files, │ │ filesystem, NVXS connection, session, process, platform, VM instances, -│ │ sandbox stores, templates' manifests, compatibility keys, and store, outcome -│ │ reports, file seals, and the sandbox launch profile of `nvx-rs sandbox`, its -│ │ distro and scratch bindings, and its prepared launches; with feature `cli`, -│ │ the `nvx-rs` CLI +│ │ sandbox stores, templates' manifests, compatibility keys, store, and leases, +│ │ outcome reports, file seals, and the sandbox launch profile of +│ │ `nvx-rs sandbox`, its distro and scratch bindings, and its prepared launches; +│ │ with feature `cli`, the `nvx-rs` CLI │ ├── testing/ feature `testing`: the fake OpenVMM │ ├── guest/ feature `guest`: the programs that run in Linux guests, the guest's NVXS │ │ connection, the libraries through which a guest init or agent opens the @@ -96,7 +96,7 @@ nvx-rs/ | `nvx::host::nvxs` | `HostConnection`: the host's role of the NVXS session over a platform transport, on which `nvx::host::session` and other host applications run their protocols. It attaches with the launch capability, waiting while the broker asks it to, sends application bytes in as many DATA records as they take, and returns each DATA payload from the guest, with deadlines; a receive that times out keeps a partial record | | `nvx::host::process` | Verified process references. A `ProcessRef` names one process by its ID, its start time, and the `BootId` of the host boot during which it started. `Process::open` checks a reference and holds its process through a pidfd on Linux or a process handle on Windows, whether or not the caller started it, so that waits sleep until the process exits and termination never reaches a process that reuses its ID. A reference from an earlier boot names an exited process, and a process whose identity this user cannot check, such as another user's, makes `Process::open` fail rather than report an exit. Linux hosts without pidfds poll the reference instead, and cannot terminate the process | | `nvx::host::supervisor` | The supervisor of a sandbox's VMM, beneath the CLI, `Instance`, and external orchestrators, which keep the configuration, the authorization, and the application protocol. `launch` runs the store's launch steps under the sandbox's lock: it writes the journal's marker, spawns OpenVMM with the claimed log, records its `ProcessRef` right after, and returns a `Starting` launch that the caller commits once the VMM serves its control endpoint, or abandons. A launch that fails never leaves an unrecorded VMM running. `VmmCommand::inherit` lists the descriptors (Linux) or handles (Windows) that the VMM inherits under their numbers, besides its standard streams, and it inherits nothing else of the caller's: on Linux, they must be close-on-exec, a flag that only the VMM's own process clears; on Windows, they join the VMM's explicit handle list. `Vmm::stop` runs one graceful step of the caller's and waits for the VMM's exit event until a deadline, and takes a bounded forced step through the VMM's verified identity only on request. `Vmm::terminate` takes that forced step alone, within the caller's bound, for a VM whose guest the caller must not ask anything, such as one of another runtime identity. `reattach` recovers an interrupted launch, and holds the recorded VMM again after the orchestrator restarts within the same host boot: `Live`, `Mismatched` with another runtime identity, `Exited`, or `Indeterminate` | -| `nvx::host::template` | The metadata of templates, captures of a microVM from which independent clones are restored. `TemplateManifest::read` reads the `manifest.bin` that OpenVMM wrote for a capture, at most `MAX_MANIFEST_LEN` bytes of a regular file that is not a link or other reparse point, and refuses another snapshot format version, an unknown, mistyped, or repeated field, truncated bytes, and a capture that is not a template's: a platform-tier capture for clones, with a read-only `distro` whose identity is unbound, a fresh scratch, and the dormant filesystem slot. `CaptureClass` holds the invariant inputs of a capture, known before it: the runtime, the guest's build, the backend, the requested CPU profile, the processors and RAM, the geometries of the disks, the network, and the adapter's other `Invariants`. `TemplateKey::new` checks that the capture made the machine that its class requested, and derives the template's compatibility key, from metadata alone: the runtime and contract versions through which its clones are restored, and the backend, CPU profile and effective CPUID, topology, rates, resources, kernel command line, disks, and devices of its machine. A capture's own metadata, such as its time anchor and host, stays out of the key. A key and a class each have one canonical, versioned encoding, whose domain-separated SHA-256 digest is its identity, a `KeyIdentity`, which converts to and from the `nvx::activation::Identity` by which a clone's activation record names its template's key, or a `ClassIdentity`; the [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/template-key-v1.txt`, pin the key's encoding and identity. `Store` keeps the generations of templates that a host published, apart from every sandbox's store, at a location that the adapter chooses: a private directory with an identity of its own, a `StoreId`, the immutable payload of each generation, its key's canonical encoding beside the snapshot that OpenVMM published, and the versioned `Index` of them, which lists each generation with the identities of its key and capture class and its own metadata, such as its time contract. `Store::create` creates a store, or opens the one that its directory holds, crash-safely and once between concurrent creations, and refuses a directory with foreign entries; `Store::open` opens one. Its records are JSON in one format, replaced atomically, and records of another format fail rather than being migrated. Its directories must be private to the current user, and it refuses links and, on Windows, other reparse points among its directories and files. Four namespaces stay apart: the `nvx::activation::StoreGeneration`, the host's publication ID, which the index assigns in the order of publication and never twice, and by which a clone's activation record names its template; the bound-block `StorageGeneration` of a capture, which belongs to the storage owner; the time lineage counter of the time contract; and the VM generation ID, which the store never records | +| `nvx::host::template` | The metadata of templates, captures of a microVM from which independent clones are restored. `TemplateManifest::read` reads the `manifest.bin` that OpenVMM wrote for a capture, at most `MAX_MANIFEST_LEN` bytes of a regular file that is not a link or other reparse point, and refuses another snapshot format version, an unknown, mistyped, or repeated field, truncated bytes, and a capture that is not a template's: a platform-tier capture for clones, with a read-only `distro` whose identity is unbound, a fresh scratch, and the dormant filesystem slot. `CaptureClass` holds the invariant inputs of a capture, known before it: the runtime, the guest's build, the backend, the requested CPU profile, the processors and RAM, the geometries of the disks, the network, and the adapter's other `Invariants`. `TemplateKey::new` checks that the capture made the machine that its class requested, and derives the template's compatibility key, from metadata alone: the runtime and contract versions through which its clones are restored, and the backend, CPU profile and effective CPUID, topology, rates, resources, kernel command line, disks, and devices of its machine. A capture's own metadata, such as its time anchor and host, stays out of the key. A key and a class each have one canonical, versioned encoding, whose domain-separated SHA-256 digest is its identity, a `KeyIdentity`, which converts to and from the `nvx::activation::Identity` by which a clone's activation record names its template's key, or a `ClassIdentity`; the [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/template-key-v1.txt`, pin the key's encoding and identity. `Store` keeps the generations of templates that a host published, apart from every sandbox's store, at a location that the adapter chooses: a private directory with an identity of its own, a `StoreId`, the immutable payload of each generation, its key's canonical encoding beside the snapshot that OpenVMM published, and the versioned `Index` of them, which lists each generation with the identities of its key and capture class and its own metadata, such as its time contract. `Store::create` creates a store, or opens the one that its directory holds, crash-safely and once between concurrent creations, and refuses a directory with foreign entries; `Store::open` opens one. Its records are JSON in one format, replaced atomically, and records of another format fail rather than being migrated. Its directories must be private to the current user, and it refuses links and, on Windows, other reparse points among its directories and files. Four namespaces stay apart: the `nvx::activation::StoreGeneration`, the host's publication ID, which the index assigns in the order of publication and never twice, and by which a clone's activation record names its template; the bound-block `StorageGeneration` of a capture, which belongs to the storage owner; the time lineage counter of the time contract; and the VM generation ID, which the store never records. `TimeContract::eligibility` decides, apart from compatibility, whether a clone may restore a capture at a host's time, a `HostTime`, which `HostTime::now` samples as OpenVMM does: within the boot of the capture, by the host's monotonic clock, and otherwise by UTC, the downtime must not be negative, and the interval by which a restore advances the clone's VM time at most `MAX_DOWNTIME_NS`, 30 days. `Store::select` selects the latest eligible generation of a key that no tombstone quarantines. `Store::acquire` leases a generation to a clone in one transaction, under the store's lock and then the sandbox's: it checks the generation's eligibility, writes the lease's record in the store and the clone binding, `clone.json`, beside the sandbox's launch journal, and returns the lease with the sandbox's lock, under which the caller launches the clone. A lease never expires: `Store::release` ends it once the sandbox records no VM and no launch. `Store::tombstone` quarantines a generation, which takes no new lease while its leases keep working, and `Store::retire` deletes it once its leases have drained, crash-safely, removing the records of leases that no clone binding names any more | | `nvx::host::seal` | `FileSeal`: what the host reports about an open file from its metadata alone, without reading it: the platform scheme, the device or volume serial number, the file ID, the kind, the length, the modification time, and on Linux the change time, with a versioned serde form. A seal detects accidental change, not tampering; see [Distro bindings](#distro-bindings) | | `nvx::host::session`, `platform`, `report` | The authenticated control session, which carries NVX's control protocol, `NVXC`, one frame per DATA record of a `HostConnection`; Linux and Windows primitives; and OpenVMM's outcome report | | `nvx::host::sandbox` | The launch profile of `nvx-rs sandbox`, with native input validation: `SandboxLaunch`, with the distro `SandboxLayer`, an EROFS layer or a GPT disk, the one layer that a sandbox takes, the ext4 scratch disk, the entrypoint, arguments, hostname, workload identity, and `SandboxSettings` resource limits (`Option` for memory and `Option` for processes), and the `SandboxPlan` that it makes of them. `plan`: `SandboxPlan`, the typed plan of a cold layered launch, which is persistable in a versioned serde form: one read-only `distro` block device, an EROFS filesystem or a GPT disk with the identity that the guest expects of it, if any, an upper, the ext4 scratch disk or RAM, the launch settings, the `ExportPlan` of its live shares, and optionally a `SandboxNetwork`, the guest network and its typed `NetworkPolicy`, whose proxy's URL as the guest sees it the plan reports. The plan of an adapter that boots its own guest, `SandboxPlan::adapter`, holds no launch settings and no export plan. A plan knows only the `distro` and `scratch` `BlockRole`s, and gives, for every combination, its `--microvm-sandbox-block` arguments, with `--microvm-workload-identity` when it has launch settings, with no scratch block device for a RAM upper, and those of its network, which `nvx::host::network` lowers, and its kernel command line, with the boot tokens of `nvx::protocol::root`, within the 1024-byte budget that OpenVMM leaves it, refusing one whose root tokens the guest's parser refuses. Both deliver the live shares as the managed lifecycle does: the arguments export them at `nvx::host::filesystem::GUEST_EXPORT`, the command line announces the number of their mappings with `nvx_maps=`, and `mapping_table` gives the ordered table that `start` sends once the guest agent is ready; the CLI's one-shot `run` still renders them as boot tokens. An adapter's plan renders the caller's tokens before those of its root, and refuses tokens that its root or OpenVMM's shares own. `mount`: `SandboxMount` requests of live shares of host directories and files, their guest targets and the reserved ones, their access policies and owner, and the checks of their host paths; `plan_mounts`, which plans them through `nvx::host::filesystem` for the container root, and `boot_shares`, which renders a plan as the OpenVMM arguments and boot tokens of one share or of the aggregate of several, as the one-shot lifecycle delivers them. `options`: the checks that `nvx-rs sandbox` and `nvx-rs run` apply to their options: the attribution of `--mount-deny`, `--mount-allow`, and `--mount-write` to shares, the systemd entrypoint check and the explicit verification of the distro layer, the targets of `nvx-rs run` shares, and the managed exec environment; see [Sandbox input contract](#sandbox-input-contract). `distro`: the `DistroBinding` of a distro layer, an EROFS layer or a GPT disk, which explicit verification makes or a staging caller imports, and its check from metadata alone, which returns the layer open; see [Distro bindings](#distro-bindings). `scratch`: the `ScratchBinding` of a scratch image, its identity, kind, and length, and its check from metadata alone, which claims the image for one sandbox at a time and returns it open. `prepared`: `PreparedLaunch`, the launch of a plan with the checked files of its block devices, which renders the plan's OpenVMM arguments and kernel command line, naming the checked files as `/proc/self/fd/N` on Linux and by their final paths on Windows, and lists the descriptors that the VMM inherits through `VmmCommand::inherit`; see [Checked handoff](#checked-handoff) | @@ -1002,11 +1002,14 @@ usage errors, which no library error has. | `runtime-mismatch` | `Other` | The sandbox's VM runs under another runtime identity than the artifacts', whose differing parts the message names, so only a stop, which terminates it, applies | | `vmm-indeterminate` | `Other` | Whether the OpenVMM process of the sandbox runs cannot be determined, so its records remain for a later attempt | | `format-unsupported` | `Io` | A record or report has a format or schema version that this build does not read, such as the state of a sandbox that another build provisioned or started | -| `foreign-files` | `Other` | The sandbox's state directory holds files that NVX does not own, which the message names and a removal keeps | +| `foreign-files` | `Other` | A directory of NVX's state, such as a sandbox's state directory or a template store, holds files that NVX does not own, which the message names and NVX keeps | | `outcome-report-exists` | `Other` | A file exists at the path of the outcome report to write | | `scratch-claimed` | `TimedOut` | Another sandbox claims the scratch image | | `exec-refused` | `Protocol` | The guest agent refused the workload, as for a working directory that it cannot use | | `endpoint-closed` | `Protocol` | OpenVMM closed the control endpoint, as it does when the VM shuts down | +| `template-ineligible` | `Unavailable` | The restore window of a template's generation, which the time contract of its capture sets, excludes the host's time, so no clone may restore it now: its capture is too old, or the host's clock is behind the capture's | +| `template-tombstoned` | `Unavailable` | A tombstone quarantines the template's generation, which takes no new clones, while the clones that lease it keep their leases | +| `sandbox-leased` | `Other` | The sandbox holds a lease of a template's generation already, which a release ends once its VM has exited | ## Bundled artifacts diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index ee49c9402..01d52b41e 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -134,8 +134,9 @@ pub enum ErrorCode { /// this build does not read, such as the state of a sandbox that another build provisioned or /// started. FormatUnsupported, - /// `foreign-files`: the sandbox's state directory holds files that NVX does not own, which a - /// removal keeps. + /// `foreign-files`: a directory of NVX's state, such as a sandbox's state directory or a + /// template store, holds files that NVX does not own, which the message names, and which NVX + /// keeps. ForeignFiles, /// `outcome-report-exists`: a file exists at the path of the outcome report to write. OutcomeReportExists, @@ -146,6 +147,16 @@ pub enum ErrorCode { ExecRefused, /// `endpoint-closed`: OpenVMM closed the control endpoint, as it does when the VM shuts down. EndpointClosed, + /// `template-ineligible`: the restore window of a template's generation, which the time + /// contract of its capture sets, excludes the host's time, so no clone may restore it now: its + /// capture is too old, or the host's clock is behind the capture's. + TemplateIneligible, + /// `template-tombstoned`: a tombstone quarantines the template's generation, which takes no + /// new clones, while the clones that lease it keep their leases. + TemplateTombstoned, + /// `sandbox-leased`: the sandbox holds a lease of a template's generation already, which a + /// release ends once its VM has exited. + SandboxLeased, } impl ErrorCode { @@ -178,6 +189,9 @@ impl ErrorCode { Self::ScratchClaimed, Self::ExecRefused, Self::EndpointClosed, + Self::TemplateIneligible, + Self::TemplateTombstoned, + Self::SandboxLeased, ]; /// Returns the stable spelling of this code. @@ -210,15 +224,20 @@ impl ErrorCode { Self::ScratchClaimed => "scratch-claimed", Self::ExecRefused => "exec-refused", Self::EndpointClosed => "endpoint-closed", + Self::TemplateIneligible => "template-ineligible", + Self::TemplateTombstoned => "template-tombstoned", + Self::SandboxLeased => "sandbox-leased", } } /// Returns the kind of the errors that have this code. pub const fn kind(self) -> ErrorKind { match self { - Self::Unavailable | Self::ArtifactMissing | Self::GuestFeaturesMissing => { - ErrorKind::Unavailable - } + Self::Unavailable + | Self::ArtifactMissing + | Self::GuestFeaturesMissing + | Self::TemplateIneligible + | Self::TemplateTombstoned => ErrorKind::Unavailable, Self::Unsupported | Self::ArgumentUnrecognized | Self::ArgumentMissing @@ -235,7 +254,8 @@ impl ErrorCode { | Self::RuntimeMismatch | Self::VmmIndeterminate | Self::ForeignFiles - | Self::OutcomeReportExists => ErrorKind::Other, + | Self::OutcomeReportExists + | Self::SandboxLeased => ErrorKind::Other, } } } diff --git a/nvx-rs/src/host/mod.rs b/nvx-rs/src/host/mod.rs index 46c687282..12bf21753 100644 --- a/nvx-rs/src/host/mod.rs +++ b/nvx-rs/src/host/mod.rs @@ -24,7 +24,7 @@ //! compatibility key of a template, [`TemplateKey`](template::TemplateKey), which it derives //! from the [`CaptureClass`](template::CaptureClass) of the capture and the capture's //! [`TemplateManifest`](template::TemplateManifest), and the [`Store`](template::Store) of the -//! generations of templates that a host published. +//! generations of templates that a host published, which leases them to clones. //! - [`session`] speaks NVX's control protocol, `NVXC`, over [`nvxs`], which runs the host's role //! of the NVXS session for it and for any other application protocol, over a platform //! transport. [`platform`] holds the primitives of Linux and Windows hosts. Other hosts compile diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index e00618520..384dab084 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -285,6 +285,35 @@ pub(crate) fn boot_id() -> io::Result<[u8; 16]> { }) } +/// The file of the host's machine ID. +const MACHINE_ID_PATH: &str = "/etc/machine-id"; + +/// Returns the identity of this host, as OpenVMM records it in a capture's time contract: its +/// machine ID, from [`MACHINE_ID_PATH`], as 16 bytes in the order in which it is written. +pub(crate) fn host_id() -> io::Result<[u8; 16]> { + let text = fs::read_to_string(MACHINE_ID_PATH)?; + super::parse_host_id(&text).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("{MACHINE_ID_PATH} has an unexpected format"), + ) + }) +} + +/// Returns the host's monotonic time, as OpenVMM measures the downtime of a capture within one +/// boot: `CLOCK_BOOTTIME`, in nanoseconds, which counts the time that the host was suspended. +pub(crate) fn monotonic_ns() -> io::Result { + /// Nanoseconds in a second. + const NANOS_PER_SECOND: u64 = 1_000_000_000; + let time = nix::time::clock_gettime(nix::time::ClockId::CLOCK_BOOTTIME)?; + u64::try_from(time.tv_sec()) + .ok() + .and_then(|seconds| seconds.checked_mul(NANOS_PER_SECOND)) + .zip(u64::try_from(time.tv_nsec()).ok()) + .and_then(|(seconds, nanos)| seconds.checked_add(nanos)) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "CLOCK_BOOTTIME is out of range")) +} + /// Starts the command in a new session so it outlives the caller and its terminal, with the /// descriptors `inherit` under their numbers in the caller, and no other descriptor of the caller /// beyond the standard streams that the command sets up. diff --git a/nvx-rs/src/host/platform/mod.rs b/nvx-rs/src/host/platform/mod.rs index e49ce765a..c9d91826a 100644 --- a/nvx-rs/src/host/platform/mod.rs +++ b/nvx-rs/src/host/platform/mod.rs @@ -99,3 +99,62 @@ pub(crate) fn parse_hex_id(digits: &str) -> Option<[u8; 16]> { } Some(id) } + +/// Parses the identity of a host as OpenVMM reads it: 32 hexadecimal digits, in either case, +/// between optional braces and surrounding whitespace, ignoring dashes, as in `/etc/machine-id` +/// and the Windows machine GUID. +#[cfg(any(target_os = "linux", windows))] +fn parse_host_id(text: &str) -> Option<[u8; 16]> { + let digits: String = text + .trim() + .trim_start_matches('{') + .trim_end_matches('}') + .chars() + .filter(|&character| character != '-') + .collect(); + parse_hex_id(&digits) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(any(target_os = "linux", windows))] + #[test] + fn host_identities_are_read_as_openvmm_reads_them() { + let id = [ + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, + 0xcd, 0xef, + ]; + for text in [ + "0123456789abcdef0123456789ABCDEF\n", + "01234567-89ab-cdef-0123-456789abcdef", + "{01234567-89AB-CDEF-0123-456789ABCDEF}", + " 0123456789abcdef0123456789abcdef ", + ] { + assert_eq!(parse_host_id(text), Some(id), "{text:?}"); + } + for text in [ + "0123", + "0123456789abcdef0123456789abcdeg", + "0123456789abcdef0123456789abcdef00", + "", + ] { + assert_eq!(parse_host_id(text), None, "{text:?}"); + } + } + + #[cfg(any(target_os = "linux", windows))] + #[test] + fn the_hosts_identity_and_monotonic_time_are_read() { + let first = monotonic_ns().unwrap(); + assert!(monotonic_ns().unwrap() >= first); + match host_id() { + Ok(id) => assert_eq!(host_id().unwrap(), id), + Err(error) if error.kind() == io::ErrorKind::NotFound => { + eprintln!("skipped: this host has no machine identity: {error}"); + } + Err(error) => panic!("{error}"), + } + } +} diff --git a/nvx-rs/src/host/platform/other.rs b/nvx-rs/src/host/platform/other.rs index f046788e3..ddc2e9fb8 100644 --- a/nvx-rs/src/host/platform/other.rs +++ b/nvx-rs/src/host/platform/other.rs @@ -73,6 +73,16 @@ pub(crate) fn boot_id() -> io::Result<[u8; 16]> { Err(unsupported()) } +/// Fails: this host has no host identity that NVX reads. +pub(crate) fn host_id() -> io::Result<[u8; 16]> { + Err(unsupported()) +} + +/// Fails: this host has no monotonic clock that NVX reads. +pub(crate) fn monotonic_ns() -> io::Result { + Err(unsupported()) +} + /// Fails: this host cannot export host paths. pub fn file_identity(_path: &Path) -> io::Result<(u64, u64)> { Err(unsupported()) diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index 4ac98774a..b57574c1b 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -46,7 +46,9 @@ use windows_sys::Win32::System::LibraryLoader::{ GetProcAddress, LOAD_LIBRARY_SEARCH_SYSTEM32, LoadLibraryExW, }; use windows_sys::Win32::System::Pipes::{GetNamedPipeServerProcessId, WaitNamedPipeW}; -use windows_sys::Win32::System::Registry::{HKEY_LOCAL_MACHINE, RRF_RT_REG_DWORD, RegGetValueW}; +use windows_sys::Win32::System::Registry::{ + HKEY_LOCAL_MACHINE, RRF_RT_REG_DWORD, RRF_RT_REG_SZ, RegGetValueW, +}; #[cfg(feature = "cli")] use windows_sys::Win32::System::Threading::GetExitCodeProcess; use windows_sys::Win32::System::Threading::{ @@ -59,6 +61,7 @@ use windows_sys::Win32::System::Threading::{ STARTF_USESTDHANDLES, STARTUPINFOEXW, TerminateProcess, UpdateProcThreadAttribute, WaitForSingleObject, }; +use windows_sys::Win32::System::WindowsProgramming::QueryInterruptTimePrecise; use super::Transport; use super::inherit::Inherited; @@ -347,6 +350,65 @@ pub(crate) fn boot_id() -> io::Result<[u8; 16]> { const BOOT_ID_KEY: &str = r"SYSTEM\CurrentControlSet\Control\Session Manager\Memory Management\PrefetchParameters"; +/// The registry key of the machine GUID, under `HKEY_LOCAL_MACHINE`. +const MACHINE_GUID_KEY: &str = r"SOFTWARE\Microsoft\Cryptography"; + +/// Returns the identity of this host, as OpenVMM records it in a capture's time contract: the +/// machine GUID that Windows keeps in the registry value `MachineGuid` of [`MACHINE_GUID_KEY`], as +/// 16 bytes in the order of its text. +pub(crate) fn host_id() -> io::Result<[u8; 16]> { + let key = wide(MACHINE_GUID_KEY); + let name = wide("MachineGuid"); + let mut value = [0u16; 64]; + let mut size = mem::size_of_val(&value) as u32; + // SAFETY: the names are NUL-terminated, the type flag admits only a REG_SZ, and the output + // buffer holds `size` bytes. + let status = unsafe { + RegGetValueW( + HKEY_LOCAL_MACHINE, + key.as_ptr(), + name.as_ptr(), + RRF_RT_REG_SZ, + ptr::null_mut(), + value.as_mut_ptr().cast(), + &mut size, + ) + }; + if status != ERROR_SUCCESS { + let error = io::Error::from_raw_os_error(status as i32); + return Err(io::Error::new( + error.kind(), + format!( + "cannot read the registry value HKLM\\{MACHINE_GUID_KEY}\\MachineGuid: {error}" + ), + )); + } + let units = &value[..(size as usize / 2).min(value.len())]; + let end = units + .iter() + .position(|&unit| unit == 0) + .unwrap_or(units.len()); + super::parse_host_id(&String::from_utf16_lossy(&units[..end])).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!( + "the registry value HKLM\\{MACHINE_GUID_KEY}\\MachineGuid has an unexpected format" + ), + ) + }) +} + +/// Returns the host's monotonic time, as OpenVMM measures the downtime of a capture within one +/// boot: the interrupt time, which counts the time that the host was asleep, in nanoseconds. +pub(crate) fn monotonic_ns() -> io::Result { + let mut intervals = 0u64; + // SAFETY: the output pointer is valid. + unsafe { QueryInterruptTimePrecise(&mut intervals) }; + intervals + .checked_mul(100) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "the interrupt time overflows")) +} + /// An OpenVMM process launched by this caller, held through its process handle. pub(crate) struct LaunchedProcess { /// Process handle retained for termination and waiting. diff --git a/nvx-rs/src/host/store/files.rs b/nvx-rs/src/host/store/files.rs index 1e9a34be6..31ab4ce2a 100644 --- a/nvx-rs/src/host/store/files.rs +++ b/nvx-rs/src/host/store/files.rs @@ -1,7 +1,8 @@ -//! The private files of the store: their creation, atomic replacement, reading, and removal. +//! The private files of the store: their creation, atomic replacement, reading, and removal, and +//! the syncs of their directories. //! //! The template store, [`template::Store`](crate::host::template::Store), keeps its own records -//! with the same lock files, atomic replacements, and removals. +//! with the same lock files, atomic replacements, removals, and syncs. use std::fs::{self, File, OpenOptions}; use std::io::{self, Read, Write}; @@ -200,6 +201,30 @@ pub(super) fn read_private(path: &Path, description: &str) -> Result Result<()> { + #[cfg(unix)] + { + File::open(dir) + .and_then(|directory| directory.sync_all()) + .map_err(io_error(format!( + "cannot sync the directory {}", + dir.display() + ))) + } + #[cfg(not(unix))] + { + let _ = dir; + Ok(()) + } +} + /// Removes the file at `path` if it exists. pub(crate) fn remove_if_present(path: &Path) -> Result<()> { match fs::remove_file(path) { diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 2aac7346f..a28ca22b9 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -14,6 +14,7 @@ //! caller captures it //! outcome.json the VMM's outcome report of the latest launch //! .tmpXXXXXX the outcome report while the VMM writes it, which it then publishes +//! clone.json the binding of a clone to the generation of a template that it leases //! ... the caller's configuration record and other files, which it declares //! ``` //! @@ -265,11 +266,16 @@ pub const LOG_NAME: &str = "openvmm.log"; pub const CONSOLE_NAME: &str = "console.log"; /// Name of the VMM's outcome report. pub const OUTCOME_NAME: &str = "outcome.json"; +/// Name of the binding of the sandbox's clone to the generation of a template that it leases, +/// which [`template::Store::acquire`](crate::host::template::Store::acquire) writes, durably, +/// before the clone's launch, and [`template::Store::release`](crate::host::template::Store::release) +/// removes once its VM has exited. A [removal](self#removal) of the sandbox removes it too. +pub const CLONE_BINDING_NAME: &str = "clone.json"; /// Every file that the store keeps in a sandbox's directory. Replacing a record leaves a /// temporary file named `.NAME.NUMBER.tmp` beside it only if the replacement was interrupted, and /// a VMM that is killed while it writes its outcome report leaves the file in which it stages it; /// see [the report staging file](self#the-report-staging-file). -pub const FILE_NAMES: [&str; 8] = [ +pub const FILE_NAMES: [&str; 9] = [ LOCK_NAME, JOURNAL_NAME, RUNTIME_NAME, @@ -278,6 +284,7 @@ pub const FILE_NAMES: [&str; 8] = [ LOG_NAME, CONSOLE_NAME, OUTCOME_NAME, + CLONE_BINDING_NAME, ]; /// Interval at which a lock that another holder has is tried again: the sandbox's lock, by a @@ -464,6 +471,12 @@ impl SandboxStore { self.path(SOCKET_NAME) } + /// Returns the path of the binding of the sandbox's clone to the generation of a template that + /// it leases, [`CLONE_BINDING_NAME`]. + pub fn binding_path(&self) -> PathBuf { + self.path(CLONE_BINDING_NAME) + } + /// Reads the runtime record, which names the sandbox's VM from the commit of its launch until /// its runtime is cleared. pub fn runtime(&self) -> Result> { diff --git a/nvx-rs/src/host/template/eligibility.rs b/nvx-rs/src/host/template/eligibility.rs new file mode 100644 index 000000000..f81994d75 --- /dev/null +++ b/nvx-rs/src/host/template/eligibility.rs @@ -0,0 +1,224 @@ +//! Eligibility: whether a clone may restore a generation of a template at a host's time, by the +//! restore window that the time contract of its capture sets, apart from its compatibility. + +#[cfg(test)] +mod tests; + +use std::fmt; +use std::io; +use std::time::SystemTime; + +use super::manifest::{HostClock, TimeContract}; +use super::store::Generation; +use crate::error::{Error, ErrorKind, Result}; +use crate::host::platform; +use crate::host::process::BootId; + +/// The longest interval by which a restore advances a clone's VM time, from the cut of the +/// captured VM's time to the restoring host's time, in nanoseconds: 30 days, as version +/// [`TIME_ABI_VERSION`](crate::contract::TIME_ABI_VERSION) of OpenVMM's time ABI selects +/// downtimes. +pub const MAX_DOWNTIME_NS: u64 = 30 * 24 * 60 * 60 * 1_000_000_000; + +/// The time of a host as a restore samples it: the identities of the host and of its boot, its +/// monotonic clock with that clock's reading, and its UTC. +/// +/// [`HostTime::now`] samples this host's. The fields are public, so that eligibility can also be +/// checked at other times, such as with a margin before the window of a generation closes, and in +/// tests. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct HostTime { + /// The identity of the host, as a time contract records it: its machine ID on Linux, and its + /// machine GUID on Windows. + pub host_id: [u8; 16], + /// The host's boot. + pub boot_id: BootId, + /// The host's monotonic clock. + pub clock: HostClock, + /// The reading of [`clock`](Self::clock), in nanoseconds. + pub monotonic_ns: u64, + /// The host's UTC, in nanoseconds since the Unix epoch. + pub utc_ns: u64, +} + +impl HostTime { + /// Samples the time of this host as OpenVMM samples it at a capture and at a restore: the + /// identities of the host and of its boot, and its UTC between two readings of its monotonic + /// clock, Linux's `CLOCK_BOOTTIME` or Windows' interrupt time, whose midpoint it keeps. + /// + /// Fails with [`ErrorKind::Unavailable`] on hosts other than Linux and Windows, whose clocks + /// no capture records, and with [`ErrorKind::Io`] if an identity or a clock of this host + /// cannot be read. + pub fn now() -> Result { + let clock = if cfg!(target_os = "linux") { + HostClock::LinuxBoottime + } else if cfg!(windows) { + HostClock::WindowsInterruptTime + } else { + return Err(Error::new( + ErrorKind::Unavailable, + "this host has no clock that a capture records, so it restores no template", + )); + }; + let failed = |what: &'static str| { + move |error: io::Error| Error::io(format!("cannot read {what}")).with_source(error) + }; + let host_id = platform::host_id().map_err(failed("the identity of this host"))?; + let boot_id = BootId::current().map_err(failed("the identity of this host's boot"))?; + let monotonic = || platform::monotonic_ns().map_err(failed("this host's monotonic clock")); + let before = monotonic()?; + let utc_ns = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .ok() + .and_then(|since| u64::try_from(since.as_nanos()).ok()) + .ok_or_else(|| Error::io("this host's UTC is outside the range of a time contract"))?; + let after = monotonic()?; + let Some(elapsed) = after.checked_sub(before) else { + return Err(Error::io("this host's monotonic clock went backwards")); + }; + Ok(Self { + host_id, + boot_id, + clock, + monotonic_ns: before + elapsed / 2, + utc_ns, + }) + } +} + +/// The source of the downtime of a capture, which a restore chooses from the host's time. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DowntimeSource { + /// The host's monotonic clock, on the host, in the boot, and by the clock of the capture. + HostMonotonic, + /// UTC, after a reboot, on another host, or by another clock. + Utc, +} + +impl fmt::Display for DowntimeSource { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::HostMonotonic => "the host's monotonic clock", + Self::Utc => "UTC", + }) + } +} + +/// The downtime of a capture at a host's time. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct Downtime { + /// The downtime from the anchor of the capture's time contract, in nanoseconds. + pub nanos: u64, + /// The interval by which a restore advances the clone's VM time: the downtime and the + /// interval from the cut of the captured VM's time to the anchor, in nanoseconds, or + /// `u64::MAX` if it is longer. + pub vm_time_nanos: u64, + /// The source of the downtime. + pub source: DowntimeSource, +} + +/// Whether a clone may restore a capture at a host's time, by the restore window of its time +/// contract, as [`TimeContract::eligibility`] decides it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Eligibility { + /// A clone may restore it: its downtime is not negative, and the interval by which the + /// restore advances the clone's VM time is at most [`MAX_DOWNTIME_NS`]. + Eligible(Downtime), + /// Not yet: the host's time is before the anchor of the capture, so that its downtime is + /// negative, as on a host whose UTC is behind the capturing host's. + Early { + /// The source of the downtime. + source: DowntimeSource, + /// How long before the anchor the host's time is, in nanoseconds. + before_ns: u64, + }, + /// No longer: the interval by which a restore would advance the clone's VM time is longer + /// than [`MAX_DOWNTIME_NS`]. + Expired(Downtime), +} + +impl Eligibility { + /// Returns whether a clone may restore the capture. + pub fn is_eligible(&self) -> bool { + matches!(self, Self::Eligible(_)) + } +} + +impl fmt::Display for Eligibility { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Eligible(downtime) => write!( + formatter, + "eligible, after a downtime of {} ns by {}", + downtime.nanos, downtime.source + ), + Self::Early { source, before_ns } => write!( + formatter, + "not eligible yet: by {source}, the host's time is {before_ns} ns before the \ + anchor of the capture" + ), + Self::Expired(downtime) => write!( + formatter, + "no longer eligible: after a downtime of {} ns by {}, a restore would advance the \ + clone's VM time by {} ns, more than the {MAX_DOWNTIME_NS} ns that it accepts", + downtime.nanos, downtime.source, downtime.vm_time_nanos + ), + } + } +} + +impl TimeContract { + /// Returns whether a clone may restore the capture of this time contract at the host's time + /// `now`, as a restore decides it. + /// + /// A restore measures the downtime of the capture from its anchor: by the host's monotonic + /// clock if `now` is of the host, the boot, and the clock that captured it, a clock that + /// counts the time that the host was suspended, and that no change of the host's wall clock + /// affects; and by UTC otherwise, after a reboot, on another host, or by another clock. It + /// refuses a negative downtime, and a capture whose VM time it would advance, by the downtime + /// and the interval from the cut of the VM's time to the anchor, by more than + /// [`MAX_DOWNTIME_NS`]. + pub fn eligibility(&self, now: &HostTime) -> Eligibility { + let same_boot = self.host_id == now.host_id + && self.boot_id == now.boot_id + && self.host_clock == now.clock; + let (source, measured) = if same_boot { + ( + DowntimeSource::HostMonotonic, + i128::from(now.monotonic_ns) - i128::from(self.capture_monotonic_ns), + ) + } else { + ( + DowntimeSource::Utc, + i128::from(now.utc_ns) - i128::from(self.capture_utc_ns), + ) + }; + // Both readings are 64-bit, so a downtime that is not negative is one too. + let Ok(nanos) = u64::try_from(measured) else { + return Eligibility::Early { + source, + before_ns: u64::try_from(measured.unsigned_abs()).unwrap_or(u64::MAX), + }; + }; + let vm_time_nanos = nanos.saturating_add(self.cut_to_anchor_ns.unwrap_or(0)); + let downtime = Downtime { + nanos, + vm_time_nanos, + source, + }; + if vm_time_nanos <= MAX_DOWNTIME_NS { + Eligibility::Eligible(downtime) + } else { + Eligibility::Expired(downtime) + } + } +} + +impl Generation { + /// Returns whether a clone may restore the generation at the host's time `now`, by the + /// restore window of its capture's time contract, as [`TimeContract::eligibility`] decides + /// it. + pub fn eligibility(&self, now: &HostTime) -> Eligibility { + self.time().eligibility(now) + } +} diff --git a/nvx-rs/src/host/template/eligibility/tests.rs b/nvx-rs/src/host/template/eligibility/tests.rs new file mode 100644 index 000000000..81f83ab6c --- /dev/null +++ b/nvx-rs/src/host/template/eligibility/tests.rs @@ -0,0 +1,276 @@ +//! Tests of eligibility: the restore window of a capture's time contract at a host's time, by the +//! clock that a restore chooses within one boot, and across a reboot. + +use super::*; + +/// Nanoseconds in a second. +const SECOND: u64 = 1_000_000_000; + +/// Returns the time contract of a capture by the host whose identity's bytes are all `host`, +/// during its boot whose bytes are all `boot`, by Linux's boot-time clock, with no interval from +/// the cut of its VM time to its anchor. +fn contract(host: u8, boot: u8) -> TimeContract { + TimeContract { + tsc_frequency_hz: 2_000_000_000, + tsc_tolerance_ppm: 100, + apic_frequency_hz: 1_000_000_000, + capture_tsc: 1_000, + capture_utc_ns: 1_700_000_000 * SECOND, + capture_monotonic_ns: 100 * SECOND, + host_clock: HostClock::LinuxBoottime, + host_id: [host; 16], + boot_id: BootId::from_bytes([boot; 16]), + lineage: 1, + cut_to_anchor_ns: None, + } +} + +/// Returns the time of the host whose identity's bytes are all `host`, during its boot whose bytes +/// are all `boot`, by Linux's boot-time clock: `utc` seconds after the anchor of [`contract`] by +/// UTC, and `monotonic` seconds after it by the monotonic clock. +fn at(host: u8, boot: u8, utc: i64, monotonic: i64) -> HostTime { + let capture = contract(host, boot); + let offset = |seconds: i64| seconds * SECOND as i64; + HostTime { + host_id: [host; 16], + boot_id: BootId::from_bytes([boot; 16]), + clock: HostClock::LinuxBoottime, + monotonic_ns: capture + .capture_monotonic_ns + .checked_add_signed(offset(monotonic)) + .unwrap(), + utc_ns: capture + .capture_utc_ns + .checked_add_signed(offset(utc)) + .unwrap(), + } +} + +/// Returns `time` with both of its clocks `nanos` later. +fn later(time: HostTime, nanos: u64) -> HostTime { + HostTime { + monotonic_ns: time.monotonic_ns + nanos, + utc_ns: time.utc_ns + nanos, + ..time + } +} + +/// Returns a downtime of `nanos` by `source`, which advances VM time by as much. +fn downtime(nanos: u64, source: DowntimeSource) -> Downtime { + Downtime { + nanos, + vm_time_nanos: nanos, + source, + } +} + +#[test] +fn within_one_boot_the_downtime_is_the_host_monotonic_clocks() { + let capture = contract(1, 1); + // Steps of the host's wall clock, forward or back, change nothing within one boot. + for utc in [5, 7, -3_600, 365 * 86_400] { + assert_eq!( + capture.eligibility(&at(1, 1, utc, 5)), + Eligibility::Eligible(downtime(5 * SECOND, DowntimeSource::HostMonotonic)), + "{utc}" + ); + } +} + +#[test] +fn across_a_reboot_on_another_host_or_by_another_clock_the_downtime_is_utcs() { + let capture = contract(1, 1); + let mut other_clock = at(1, 1, 7, 0); + other_clock.clock = HostClock::WindowsInterruptTime; + // A reboot restarts the monotonic clock, whose reading then tells nothing. + let rebooted = HostTime { + monotonic_ns: 1, + ..at(1, 2, 7, 0) + }; + for now in [at(1, 2, 7, 0), rebooted, at(2, 1, 7, 0), other_clock] { + assert_eq!( + capture.eligibility(&now), + Eligibility::Eligible(downtime(7 * SECOND, DowntimeSource::Utc)), + "{now:?}" + ); + } +} + +#[test] +fn the_window_closes_after_the_longest_downtime() { + let capture = contract(1, 1); + for (start, source) in [ + (at(1, 1, 0, 0), DowntimeSource::HostMonotonic), + (at(1, 2, 0, 0), DowntimeSource::Utc), + ] { + for nanos in [0, 1, MAX_DOWNTIME_NS - 1, MAX_DOWNTIME_NS] { + assert_eq!( + capture.eligibility(&later(start, nanos)), + Eligibility::Eligible(downtime(nanos, source)), + "{source:?} {nanos}" + ); + } + for nanos in [MAX_DOWNTIME_NS + 1, 2 * MAX_DOWNTIME_NS] { + let eligibility = capture.eligibility(&later(start, nanos)); + assert_eq!( + eligibility, + Eligibility::Expired(downtime(nanos, source)), + "{source:?} {nanos}" + ); + assert!(!eligibility.is_eligible()); + } + } +} + +#[test] +fn the_window_counts_the_interval_from_the_cut_of_the_vm_time() { + let mut capture = contract(1, 1); + capture.cut_to_anchor_ns = Some(200); + let start = at(1, 1, 0, 0); + let advanced = |nanos: u64, vm_time_nanos: u64| Downtime { + nanos, + vm_time_nanos, + source: DowntimeSource::HostMonotonic, + }; + assert_eq!( + capture.eligibility(&start), + Eligibility::Eligible(advanced(0, 200)) + ); + assert_eq!( + capture.eligibility(&later(start, MAX_DOWNTIME_NS - 200)), + Eligibility::Eligible(advanced(MAX_DOWNTIME_NS - 200, MAX_DOWNTIME_NS)) + ); + assert_eq!( + capture.eligibility(&later(start, MAX_DOWNTIME_NS - 199)), + Eligibility::Expired(advanced(MAX_DOWNTIME_NS - 199, MAX_DOWNTIME_NS + 1)) + ); + // An interval that is longer than the window by itself leaves no time to restore in. + capture.cut_to_anchor_ns = Some(MAX_DOWNTIME_NS + 1); + assert_eq!( + capture.eligibility(&start), + Eligibility::Expired(advanced(0, MAX_DOWNTIME_NS + 1)) + ); + capture.cut_to_anchor_ns = Some(u64::MAX); + assert_eq!( + capture.eligibility(&later(start, 1)), + Eligibility::Expired(advanced(1, u64::MAX)) + ); +} + +#[test] +fn a_host_time_before_the_anchor_is_early() { + let capture = contract(1, 1); + let mut same_boot = at(1, 1, 0, 0); + same_boot.monotonic_ns -= 1; + let eligibility = capture.eligibility(&same_boot); + assert_eq!( + eligibility, + Eligibility::Early { + source: DowntimeSource::HostMonotonic, + before_ns: 1, + } + ); + assert!(!eligibility.is_eligible()); + // Another host whose UTC is behind the capturing host's. + assert_eq!( + capture.eligibility(&at(2, 2, -10, 50)), + Eligibility::Early { + source: DowntimeSource::Utc, + before_ns: 10 * SECOND, + } + ); + // The earliest reading of each clock. + assert_eq!( + capture.eligibility(&HostTime { + monotonic_ns: 0, + ..at(1, 1, 0, 0) + }), + Eligibility::Early { + source: DowntimeSource::HostMonotonic, + before_ns: capture.capture_monotonic_ns, + } + ); + assert_eq!( + capture.eligibility(&HostTime { + utc_ns: 0, + ..at(2, 2, 0, 0) + }), + Eligibility::Early { + source: DowntimeSource::Utc, + before_ns: capture.capture_utc_ns, + } + ); +} + +#[test] +fn eligibilities_say_why() { + let capture = contract(1, 1); + assert_eq!( + capture.eligibility(&at(1, 1, 0, 5)).to_string(), + "eligible, after a downtime of 5000000000 ns by the host's monotonic clock" + ); + assert_eq!( + capture.eligibility(&at(2, 2, -10, 0)).to_string(), + "not eligible yet: by UTC, the host's time is 10000000000 ns before the anchor of the \ + capture" + ); + assert_eq!( + capture + .eligibility(&later(at(2, 2, 0, 0), MAX_DOWNTIME_NS + 1)) + .to_string(), + format!( + "no longer eligible: after a downtime of {} ns by UTC, a restore would advance the \ + clone's VM time by {} ns, more than the {MAX_DOWNTIME_NS} ns that it accepts", + MAX_DOWNTIME_NS + 1, + MAX_DOWNTIME_NS + 1 + ) + ); +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn this_host_samples_its_time_as_a_capture_records_it() { + let host_id = match platform::host_id() { + Ok(host_id) => host_id, + Err(error) if error.kind() == io::ErrorKind::NotFound => { + eprintln!("skipped: this host has no machine identity: {error}"); + return; + } + Err(error) => panic!("{error}"), + }; + let first = HostTime::now().unwrap(); + assert_eq!(first.host_id, host_id); + assert_eq!(first.boot_id, BootId::current().unwrap()); + let clock = if cfg!(windows) { + HostClock::WindowsInterruptTime + } else { + HostClock::LinuxBoottime + }; + assert_eq!(first.clock, clock); + // After 2020-01-01. + assert!(first.utc_ns > 1_577_836_800 * SECOND); + let second = HostTime::now().unwrap(); + assert!(second.monotonic_ns >= first.monotonic_ns); + // A capture at the first sample is eligible at the second, by the host's monotonic clock. + let capture = TimeContract { + capture_utc_ns: first.utc_ns, + capture_monotonic_ns: first.monotonic_ns, + host_clock: first.clock, + host_id: first.host_id, + boot_id: first.boot_id, + ..contract(0, 0) + }; + assert_eq!( + capture.eligibility(&second), + Eligibility::Eligible(downtime( + second.monotonic_ns - first.monotonic_ns, + DowntimeSource::HostMonotonic + )) + ); +} + +#[cfg(not(any(target_os = "linux", windows)))] +#[test] +fn other_hosts_sample_no_time() { + assert_eq!(HostTime::now().unwrap_err().kind(), ErrorKind::Unavailable); +} diff --git a/nvx-rs/src/host/template/mod.rs b/nvx-rs/src/host/template/mod.rs index c7eceaa42..c8f6661b3 100644 --- a/nvx-rs/src/host/template/mod.rs +++ b/nvx-rs/src/host/template/mod.rs @@ -20,6 +20,13 @@ //! sandbox's store: their payloads, which never change, and the versioned [`Index`] of them, //! which names each by its [`StoreGeneration`](crate::activation::StoreGeneration), at a //! location that the adapter chooses. +//! - [`TimeContract::eligibility`] decides whether a clone may restore a capture at a host's +//! time, [`HostTime`], by the restore window of its time contract, apart from its +//! compatibility. +//! - [`Store::acquire`] leases an eligible generation to a clone, with a binding in the clone's +//! sandbox, until [`Store::release`] ends the lease once the clone's VM has exited. A tombstone +//! quarantines a generation, which takes no new lease, and [`Store::retire`] deletes it once its +//! leases have drained. //! //! # Canonical encoding //! @@ -41,6 +48,7 @@ //! `tests/vectors/template-key-v1.txt` fix version 1 of a key's encoding. mod canonical; +mod eligibility; mod files; mod key; mod manifest; @@ -49,11 +57,13 @@ mod store; mod test_support; mod wire; +pub use self::eligibility::{Downtime, DowntimeSource, Eligibility, HostTime, MAX_DOWNTIME_NS}; pub use self::key::{CaptureClass, ClassIdentity, Invariants, KeyIdentity, TemplateKey}; pub use self::manifest::{ BlockGeometry, HostClock, MANIFEST_NAME, MAX_MANIFEST_LEN, TemplateManifest, TimeContract, }; pub use self::store::{ - GENERATIONS_NAME, Generation, INDEX_LOCK_NAME, INDEX_NAME, Index, KEY_NAME, MAX_GENERATIONS, - SNAPSHOT_NAME, STORE_FORMAT, STORE_RECORD_NAME, StorageGeneration, Store, StoreId, + Acquired, GENERATIONS_NAME, Generation, INDEX_LOCK_NAME, INDEX_NAME, Index, KEY_NAME, + LEASES_NAME, Lease, LeaseId, MAX_GENERATIONS, Retirement, SNAPSHOT_NAME, STORE_FORMAT, + STORE_RECORD_NAME, StorageGeneration, Store, StoreId, TOMBSTONES_NAME, }; diff --git a/nvx-rs/src/host/template/store.rs b/nvx-rs/src/host/template/store.rs index 4272e0ac4..0e5cfd958 100644 --- a/nvx-rs/src/host/template/store.rs +++ b/nvx-rs/src/host/template/store.rs @@ -1,6 +1,7 @@ -//! The store of templates, [`Store`]: its directory, its records, and the namespaces of its -//! generations. +//! The store of templates, [`Store`]: its directory, its records, the namespaces of its +//! generations, and their leases, tombstones, and retirement. +mod leases; #[cfg(test)] mod tests; @@ -25,6 +26,8 @@ use crate::host::platform::{self, parse_hex_id}; use crate::host::process::BootId; use crate::host::store::files::{open_lock, write_private}; +pub use self::leases::{Acquired, LEASES_NAME, Lease, LeaseId, Retirement, TOMBSTONES_NAME}; + /// Version of the store's records: its identity record and its index. pub const STORE_FORMAT: u32 = 1; /// Name of the store's identity record. @@ -297,6 +300,10 @@ impl Index { /// / the payload of the store generation G /// key.bin the canonical encoding of its compatibility key /// snapshot/ its snapshot, as OpenVMM published it +/// leases/ the records of the leases that the store granted +/// .json the record of the lease L +/// tombstones/ the tombstones of the generations that take no new leases +/// .json the tombstone of the store generation G /// ``` /// /// # Identity @@ -313,10 +320,12 @@ impl Index { /// of its key and of its capture class, its storage generation, and what its manifest records of /// the capture itself, its creation time, its time contract, and the lengths of its artifacts. /// The payload of a generation never changes once the index lists it: the canonical encoding of -/// its key, which [`Store::read_key`] reads, and its snapshot. Only the index changes, as -/// generations are published and removed. +/// its key, which [`Store::read_key`] reads, and its snapshot. The index changes as generations +/// are published and retired, and the records of leases and tombstones as clones lease +/// generations, and as generations are quarantined and retired. /// -/// The store's records, its identity record and its index, are JSON in format [`STORE_FORMAT`]. +/// The store's records, its identity record, its index, and the records of its leases and +/// tombstones, are JSON in format [`STORE_FORMAT`]. /// A record in any other format fails with a message that names its format, and is neither read /// nor migrated: a store of another format is replaced by a new one, whose templates are captured /// again. Records are replaced atomically: written to a temporary file beside them, flushed to @@ -340,6 +349,40 @@ impl Index { /// - The VM generation ID is 16 random bytes that each VM process draws, which tell the clones of /// one generation apart. Their guests read it, and the store records none. /// +/// # Leases, tombstones, and retirement +/// +/// A clone that restores a generation leases it first, so that the generation stays in the store +/// until the clone's VM has exited. [`Store::select`] selects the generation of a key from which +/// a clone restores: the latest that is neither quarantined nor ineligible at the host's time, +/// as [`Generation::eligibility`] decides it. [`Store::acquire`] leases a generation in one +/// transaction, under the store's lock and then the sandbox's: it checks the generation's +/// eligibility, and writes the record of the lease in the store and the clone binding, +/// [`CLONE_BINDING_NAME`](crate::host::store::CLONE_BINDING_NAME), beside the sandbox's launch +/// journal, before any VMM of the clone is spawned. It returns the lease with the sandbox's lock, +/// under which the caller launches the clone, and holds no lock of the store for the VM's +/// lifetime. A lease never expires: it lasts until [`Store::release`] ends it, which it does only +/// once the sandbox records no VM and no launch, and so once its VM has exited, as its store +/// verifies through the VM's recorded process, also after a reattach. A caller that reads a +/// generation's payload, as a restore does, holds a lease of it. +/// +/// A tombstone, which [`Store::tombstone`] writes, quarantines a generation: it takes no new +/// lease, and selection skips it, while the leases that it granted keep working. +/// [`Store::retire`] tombstones a generation, and deletes it once its leases have drained: its +/// payload, then its entry in the index, and then its tombstone, so that an interrupted +/// retirement leaves a quarantined generation, which retiring it again deletes. A release, or a +/// removal of a sandbox, that is interrupted leaves the record of a lease that no clone binding +/// names, which a retirement removes. +/// +/// These transactions make each step durable before the next, so that their order holds through +/// a crash of the host too, even when a sandbox and the store are on different filesystems: an +/// acquisition makes the record of a lease durable before it writes the clone binding, and the +/// binding before it returns, and a release, or a retirement that finds a binding gone, makes the +/// binding's absence durable before it removes the record, so that no binding outlasts the record +/// that keeps its generation; a retirement makes the tombstone durable before it deletes +/// anything, and each deletion before the next. On Windows, which has no portable way to flush a +/// directory, a rename or a removal is as durable as the filesystem makes it, as OpenVMM's +/// publication of a snapshot assumes. +/// /// # Privacy /// /// The store's directories are private to the current user, the orchestrator's account, so that diff --git a/nvx-rs/src/host/template/store/leases.rs b/nvx-rs/src/host/template/store/leases.rs new file mode 100644 index 000000000..3f5a10fe2 --- /dev/null +++ b/nvx-rs/src/host/template/store/leases.rs @@ -0,0 +1,986 @@ +//! The leases of a store's generations, the bindings of clones to them, the tombstones that +//! quarantine generations, and the retirement of generations whose leases have drained; see +//! [the store's documentation](Store#leases-tombstones-and-retirement). + +#[cfg(test)] +mod tests; + +use std::fmt; +use std::fs; +use std::io; +use std::num::NonZeroU64; +use std::path::{Path, PathBuf}; +use std::str::FromStr; +use std::sync::{Mutex, MutexGuard, PoisonError}; +use std::time::Instant; + +use serde::{Deserialize, Serialize}; + +use super::{ + GENERATIONS_NAME, Generation, ID_LEN, STORE_FORMAT, Store, StoreId, entries, hex, random_id, + read_record, refuse_problem, store_error, take_lock, write_record, +}; +use crate::activation::{Identity, StoreGeneration, TemplateReference}; +use crate::error::{Error, ErrorCode, ErrorKind, Result}; +use crate::host::platform::{self, parse_hex_id}; +use crate::host::store::files::{remove_if_present, sync_dir}; +use crate::host::store::{CLONE_BINDING_NAME, CallerFiles, LockedStore, SandboxStore}; +use crate::host::template::eligibility::HostTime; +use crate::host::template::key::KeyIdentity; + +/// Name of the directory of the records of the leases that a store granted. +pub const LEASES_NAME: &str = "leases"; +/// Name of the directory of the tombstones of a store's generations. +pub const TOMBSTONES_NAME: &str = "tombstones"; + +/// Syncs a directory, as [`sync_dir`] does. The transactions of leases take it as a parameter, so +/// that tests observe the order of their syncs, and fail them. +type SyncDir<'a> = &'a dyn Fn(&Path) -> Result<()>; + +/// The directories of leases and tombstones whose entries in their stores this process made +/// durable, by the directory and the identity of the store, and the name of the directory. A store +/// creates each once, and never removes it, so one sync of the store's directory, once a process +/// sees one, keeps its entry durable, even if the process that created it did not finish its own +/// sync. +static DURABLE_RECORD_DIRS: Mutex> = Mutex::new(Vec::new()); + +/// An entry of [`DURABLE_RECORD_DIRS`]: the directory and the identity of a store, and the name +/// of one of its directories. +type DurableRecordDir = (PathBuf, StoreId, &'static str); + +/// Returns [`DURABLE_RECORD_DIRS`], locked. +fn durable_record_dirs() -> MutexGuard<'static, Vec> { + DURABLE_RECORD_DIRS + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + +/// Longest record of a lease, of a clone binding, or of a tombstone, in bytes: more than a record +/// that names the longest path of a sandbox takes. +const MAX_LEASE_RECORD_LEN: u64 = 256 * 1024; + +/// The directory of a store's leases, in messages. +const LEASES_WHAT: &str = "the directory of the leases of the template store"; +/// The directory of a store's tombstones, in messages. +const TOMBSTONES_WHAT: &str = "the directory of the tombstones of the template store"; +/// The record of a lease in its store, in messages. +const LEASE_DESCRIPTION: &str = "lease of the template store"; +/// The binding of a clone in its sandbox, in messages. +const BINDING_DESCRIPTION: &str = "clone binding"; +/// A tombstone, in messages. +const TOMBSTONE_DESCRIPTION: &str = "tombstone of the template store"; + +/// The identity of a lease: 16 random bytes that its acquisition draws. +/// +/// Its text form, which `Display` writes and `FromStr` parses, is 32 lowercase hexadecimal +/// digits; parsing also accepts uppercase ones. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct LeaseId([u8; ID_LEN]); + +impl LeaseId { + /// Returns the identity whose bytes are `bytes`. + pub const fn from_bytes(bytes: [u8; ID_LEN]) -> Self { + Self(bytes) + } + + /// Returns the bytes of the identity. + pub const fn as_bytes(&self) -> &[u8; ID_LEN] { + &self.0 + } +} + +impl fmt::Display for LeaseId { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(&hex(&self.0)) + } +} + +impl fmt::Debug for LeaseId { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "LeaseId({self})") + } +} + +impl FromStr for LeaseId { + type Err = Error; + + /// Parses the text form of a lease's identity, failing with [`ErrorKind::Unsupported`] for + /// anything else. + fn from_str(text: &str) -> Result { + parse_hex_id(text).map(Self).ok_or_else(|| { + Error::unsupported(format!( + "{text:?} is not the identity of a lease, which is 32 hexadecimal digits" + )) + }) + } +} + +/// A lease of a generation of a template, which keeps the generation in its store for the clone +/// of one sandbox, whose VM restores it. +/// +/// The store keeps the record of each lease that it granted, and the sandbox keeps the same +/// record as its clone binding, [`CLONE_BINDING_NAME`], beside its launch journal. A lease names +/// its sandbox by the absolute path of its directory, so a sandbox that holds a lease must not +/// move. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Lease { + /// The lease's identity. + id: LeaseId, + /// The identity of the store that granted it. + store: StoreId, + /// The store generation of the leased generation. + generation: StoreGeneration, + /// The identity of the leased generation's key. + key: KeyIdentity, + /// The directory of the sandbox that holds the lease, as an absolute path, which is valid + /// Unicode. + sandbox: PathBuf, +} + +impl Lease { + /// Reads the clone binding of the sandbox of `sandbox`: the lease that it holds, if it holds + /// one. + /// + /// A read that holds no lock sees one whole binding, as the latest transition left it. Fails + /// with [`ErrorKind::Io`] for a binding that cannot be read, or is malformed, and with + /// [`ErrorCode::FormatUnsupported`] for one of another format. + pub fn of(sandbox: &SandboxStore) -> Result> { + read_binding(sandbox.dir()) + } + + /// Returns the lease's identity. + pub fn id(&self) -> LeaseId { + self.id + } + + /// Returns the identity of the store that granted the lease. + pub fn store(&self) -> StoreId { + self.store + } + + /// Returns the store generation of the leased generation. + pub fn generation(&self) -> StoreGeneration { + self.generation + } + + /// Returns the identity of the leased generation's key. + pub fn key(&self) -> KeyIdentity { + self.key + } + + /// Returns the directory of the sandbox that holds the lease, as an absolute path. + pub fn sandbox(&self) -> &Path { + &self.sandbox + } + + /// Returns the reference by which the activation record of the clone names the template that + /// it restores: the leased generation's store generation, and its key's identity. + pub fn template(&self) -> TemplateReference { + TemplateReference { + generation: self.generation, + key: Identity::from(self.key), + } + } +} + +/// A lease that [`Store::acquire`] granted, with the lock of the sandbox that holds it. +#[derive(Debug)] +pub struct Acquired { + /// The lease. + pub lease: Lease, + /// The lock of the sandbox, under which the caller launches its clone. Dropping it releases + /// the lock, and keeps the lease. + pub locked: LockedStore, +} + +/// What [`Store::retire`] found of a generation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Retirement { + /// The generation is gone, with its payload, its entry in the index, and its tombstone, or was + /// gone already. + Retired, + /// A tombstone quarantines the generation, which keeps its payload and its entry in the index + /// for its leases, which this lists in the order of their identities, until their clones + /// release them. + Draining(Vec), +} + +impl Store { + /// Selects the generation of the key `key` from which a clone restores at the host's time + /// `now`: the latest that the store indexes, that no tombstone quarantines, and that is + /// eligible at `now`, as [`Generation::eligibility`] decides it, if one is. + /// + /// The selection reads the index and the tombstones without the store's lock, as + /// [`index`](Self::index) does, so a selected generation may be tombstoned or retired before + /// the caller leases it, which [`acquire`](Self::acquire) refuses. + pub fn select(&self, key: KeyIdentity, now: &HostTime) -> Result> { + let index = self.index()?; + let tombstoned = self.tombstones()?; + Ok(index + .generations + .iter() + .rev() + .find(|generation| { + generation.key == key + && !tombstoned.contains(&generation.generation) + && generation.eligibility(now).is_eligible() + }) + .cloned()) + } + + /// Leases the generation `generation` to the clone of the sandbox in the directory `sandbox`, + /// whose caller keeps `files` there, at the host's time `now`, as [`HostTime::now`] samples + /// it, waiting for the store's lock and the sandbox's until `deadline`, or for as long as it + /// takes without one. + /// + /// In one transaction, under the store's lock and then the sandbox's + /// [lifecycle lock](crate::host::store#lifecycle-locks), it checks that the store indexes the + /// generation, that no tombstone quarantines it, that it is eligible at `now`, as + /// [`Generation::eligibility`] decides it, and that the sandbox is provisioned, records no VM + /// and no launch, and holds no lease; and it writes the lease's record in the store, and then + /// the same record as the sandbox's clone binding, beside its launch journal, each durable + /// before the next step, so that no binding outlasts, through a crash of the host, the record + /// that keeps its generation in the store. It returns the lease, with the sandbox's lock, + /// under which the caller launches the clone, and releases the store's lock. + /// + /// Fails, and leases nothing: + /// + /// - with [`ErrorKind::NotFound`] if the store does not index the generation; + /// - with [`ErrorCode::TemplateTombstoned`] if a tombstone quarantines it; + /// - with [`ErrorCode::TemplateIneligible`] if it is not eligible at `now`; + /// - with [`ErrorCode::SandboxNotProvisioned`] if the directory does not exist, or holds no + /// configuration record of the caller, as `files` declares it; + /// - with [`ErrorCode::SandboxRunning`] if the sandbox records a VM, or a launch, which may + /// still run; + /// - with [`ErrorCode::SandboxLeased`] if the sandbox holds a lease already; + /// - with [`ErrorKind::Unsupported`] if the sandbox's path is not valid Unicode, which its + /// record cannot hold; + /// - with [`ErrorKind::TimedOut`] if the deadline passes before it takes the store's lock, and + /// as [`SandboxStore::lock_existing`] fails before it takes the sandbox's. + /// + /// A failure to write the binding, or to make it durable, leases nothing either, unless the + /// binding's removal cannot be made durable: the lease's record then stays, and the error says + /// that the sandbox may hold the lease still, which a [release](Self::release) ends. + pub fn acquire( + &self, + generation: StoreGeneration, + sandbox: impl Into, + files: &CallerFiles, + now: &HostTime, + deadline: Option, + ) -> Result { + self.acquire_with(generation, sandbox.into(), files, now, deadline, &sync_dir) + } + + /// Leases a generation as [`acquire`](Self::acquire) does, making its records durable with + /// `sync`. + fn acquire_with( + &self, + generation: StoreGeneration, + sandbox: PathBuf, + files: &CallerFiles, + now: &HostTime, + deadline: Option, + sync: SyncDir<'_>, + ) -> Result { + let lock = take_lock(&self.dir, deadline)?; + let index = self.index()?; + let indexed = index + .get(generation) + .ok_or_else(|| self.not_indexed(generation))?; + if self.is_tombstoned(generation)? { + return Err(Error::new( + ErrorCode::TemplateTombstoned, + format!( + "a tombstone quarantines the generation {generation} of the template store \ + {}, which takes no new clones", + self.dir.display() + ), + )); + } + let eligibility = indexed.eligibility(now); + if !eligibility.is_eligible() { + return Err(Error::new( + ErrorCode::TemplateIneligible, + format!( + "the generation {generation} of the template store {} is {eligibility}", + self.dir.display() + ), + )); + } + let not_provisioned = || { + Error::new( + ErrorCode::SandboxNotProvisioned, + format!("no sandbox is provisioned in {}", sandbox.display()), + ) + }; + let locked = match SandboxStore::lock_existing(&sandbox, files, deadline) { + Ok(locked) => locked, + Err(error) if error.kind() == ErrorKind::NotFound => { + return Err(not_provisioned().with_source(error)); + } + Err(error) => return Err(error), + }; + if let Some(config) = files.config() { + let path = locked.dir().join(config); + match fs::symlink_metadata(&path) { + Ok(_) => {} + Err(error) if error.kind() == io::ErrorKind::NotFound => { + return Err(not_provisioned()); + } + Err(error) => { + return Err(store_error(format!("cannot examine {}", path.display()))( + error, + )); + } + } + } + if locked.journal()?.is_some() || locked.runtime()?.is_some() { + return Err(may_run(locked.dir())); + } + if let Some(held) = read_binding(locked.dir())? { + return Err(Error::new( + ErrorCode::SandboxLeased, + format!( + "the sandbox in {} holds the lease {} of the generation {} of the template \ + store {} already", + locked.dir().display(), + held.id, + held.generation, + held.store + ), + )); + } + if locked.dir().to_str().is_none() { + return Err(Error::unsupported(format!( + "the path of the sandbox {} is not valid Unicode, which the record of a lease \ + cannot hold", + locked.dir().display() + ))); + } + let lease = Lease { + id: LeaseId(random_id("a lease's identity")?), + store: self.id, + generation, + key: indexed.key, + sandbox: locked.dir().to_owned(), + }; + let record = LeaseRecord::new(&lease); + self.create_records_dir(LEASES_NAME, LEASES_WHAT, sync)?; + let path = self.lease_path(lease.id); + write_record(&path, &record, "the lease of the template store")?; + // A retirement deletes a generation that no record of the store leases, so the record + // must be durable before a binding names it. + if let Err(error) = sync(&self.dir.join(LEASES_NAME)) { + let _ = remove_if_present(&path); + return Err(error); + } + if let Err(error) = bind(locked.dir(), &record, sync) { + // The record goes only once the binding's removal is durable. Until then, it keeps + // the generation for a binding that may outlast a crash, and a retirement removes it + // once no binding names it. + if unbind(locked.dir(), sync).is_err() { + return Err(held_still(error, locked.dir(), lease.id)); + } + let _ = remove_if_present(&path); + return Err(error); + } + drop(lock); + Ok(Acquired { lease, locked }) + } + + /// Releases the lease that the sandbox of `locked` holds, and returns it, or returns `None` if + /// the sandbox holds none. + /// + /// A lease lasts until its clone's VM has exited: the release refuses with + /// [`ErrorCode::SandboxRunning`] while the sandbox records a VM or a launch, which may still + /// run. The sandbox's store clears those records only once it has verified that the VM's + /// process exited, through its recorded [`ProcessRef`](crate::host::process::ProcessRef), as + /// a stop or a [`reattach`](crate::host::supervisor::reattach) does. The release removes the + /// clone binding, and once its removal is durable, the lease's record in the store, so that an + /// interrupted release leaves a record that no binding names, which a + /// [retirement](Self::retire) removes, and never a binding without its record. It takes no + /// lock of the store. + /// + /// Fails with [`ErrorKind::Other`] if the sandbox holds a lease of another store. + pub fn release(&self, locked: &mut LockedStore) -> Result> { + self.release_with(locked, &sync_dir) + } + + /// Releases a lease as [`release`](Self::release) does, making the removal of its binding + /// durable with `sync`. + fn release_with(&self, locked: &mut LockedStore, sync: SyncDir<'_>) -> Result> { + if locked.journal()?.is_some() || locked.runtime()?.is_some() { + return Err(may_run(locked.dir())); + } + let Some(lease) = read_binding(locked.dir())? else { + return Ok(None); + }; + if lease.store != self.id { + return Err(Error::other(format!( + "the sandbox in {} holds a lease of the template store {}, not of this one, {}", + locked.dir().display(), + lease.store, + self.id + ))); + } + let recorded = self.check_records_dir(LEASES_NAME, LEASES_WHAT)?; + unbind(locked.dir(), sync)?; + if recorded { + remove_if_present(&self.lease_path(lease.id))?; + } + Ok(Some(lease)) + } + + /// Returns the leases of the generation `generation` that the store's records list, in the + /// order of their identities, including those that an interrupted release or removal of + /// their sandboxes left, which a [retirement](Self::retire) removes. + /// + /// Fails with [`ErrorCode::ForeignFiles`] if the directory of the leases holds entries that + /// are not the store's. + pub fn leases(&self, generation: StoreGeneration) -> Result> { + let mut leases = Vec::new(); + for id in self.records(LEASES_NAME, LEASES_WHAT, lease_name)?.records { + // A release may remove the record after the listing. + if let Some(lease) = self.read_lease(id)? + && lease.generation == generation + { + leases.push(lease); + } + } + leases.sort_by_key(|lease| lease.id); + Ok(leases) + } + + /// Quarantines the generation `generation` with a tombstone, waiting for the store's lock + /// until `deadline`, or for as long as it takes without one: the generation takes no new + /// lease, and [`select`](Self::select) skips it, while its leases keep working. A tombstone + /// is durable once this returns, and lasts until the generation's + /// [retirement](Self::retire). + /// + /// Tombstoning a generation again does nothing. Fails with [`ErrorKind::NotFound`] if the + /// store does not index the generation. + pub fn tombstone(&self, generation: StoreGeneration, deadline: Option) -> Result<()> { + let _lock = take_lock(&self.dir, deadline)?; + if self.index()?.get(generation).is_none() { + return Err(self.not_indexed(generation)); + } + self.write_tombstone(generation, &sync_dir) + } + + /// Returns the store generations that tombstones quarantine, in order. + /// + /// Fails with [`ErrorCode::ForeignFiles`] if the directory of the tombstones holds entries + /// that are not the store's. + pub fn tombstones(&self) -> Result> { + let mut tombstoned = self + .records(TOMBSTONES_NAME, TOMBSTONES_WHAT, tombstone_name)? + .records; + tombstoned.sort(); + Ok(tombstoned) + } + + /// Retires the generation `generation`, waiting for the store's lock until `deadline`, or for + /// as long as it takes without one: it tombstones the generation, and deletes it once its + /// leases have drained. + /// + /// Under the store's lock, it tombstones the generation, and removes the records of its + /// leases whose sandboxes no longer hold them: a sandbox that is gone, that holds no clone + /// binding, or one of another lease, as an interrupted release, or a removal of the sandbox, + /// leaves it. It makes the binding's absence durable first, by syncing the sandbox's + /// directory, or the nearest of its ancestors that remains, through the links that the + /// sandbox's path names, as a release that a crash or a failed sync interrupted may not have. + /// If any lease remains, the generation keeps its + /// payload and its entry in the index, and the retirement returns [`Retirement::Draining`] + /// with the leases, whose clones [release](Self::release) them; a later retirement deletes it. + /// Otherwise it removes the generation's payload, then its entry in the index, and then its + /// tombstone, each durable before the next, as the tombstone is before the payload goes, so + /// that a retirement that a crash interrupts, even one of the host, leaves a tombstoned + /// generation, which retiring it again deletes, and returns [`Retirement::Retired`]. A + /// generation that the store indexed once, and no longer indexes, is retired already. Its + /// store generation is never assigned again. + /// + /// It also removes the temporary files that interrupted replacements of the records of leases + /// and tombstones left, since the store writes those only under its lock. + /// + /// Fails with [`ErrorKind::NotFound`] for a store generation that the store never assigned, + /// and with [`ErrorCode::ForeignFiles`] if the directory of the leases, or of the tombstones, + /// holds entries that are not the store's. A clone binding that cannot be read, or whose + /// absence cannot be made durable, fails the retirement, which then deletes no generation. + pub fn retire( + &self, + generation: StoreGeneration, + deadline: Option, + ) -> Result { + self.retire_with(generation, deadline, &sync_dir) + } + + /// Retires a generation as [`retire`](Self::retire) does, making each step durable with `sync` + /// before the next. + fn retire_with( + &self, + generation: StoreGeneration, + deadline: Option, + sync: SyncDir<'_>, + ) -> Result { + let _lock = take_lock(&self.dir, deadline)?; + let mut index = self.index()?; + if index.get(generation).is_none() { + if generation >= index.next { + return Err(self.not_indexed(generation)); + } + // A retirement that was interrupted once it had written the index may not have made + // it durable, which must be before the tombstone goes. + sync(&self.dir)?; + self.remove_tombstone(generation)?; + return Ok(Retirement::Retired); + } + let leases = self.records(LEASES_NAME, LEASES_WHAT, lease_name)?; + let tombstones = self.records(TOMBSTONES_NAME, TOMBSTONES_WHAT, tombstone_name)?; + self.write_tombstone(generation, sync)?; + for temporary in leases.temporaries.iter().chain(&tombstones.temporaries) { + remove_if_present(temporary)?; + } + let mut live = Vec::new(); + for id in leases.records { + let Some(lease) = self.read_lease(id)? else { + continue; + }; + if lease.generation != generation { + continue; + } + if is_bound(&lease)? { + live.push(lease); + } else { + // A release may have removed the binding without making that durable yet, so the + // binding's absence must be durable before its record goes. + sync(binding_dir(&lease.sandbox)?)?; + remove_if_present(&self.lease_path(id))?; + } + } + if !live.is_empty() { + live.sort_by_key(|lease| lease.id); + return Ok(Retirement::Draining(live)); + } + self.remove_payload(generation)?; + sync(&self.dir.join(GENERATIONS_NAME))?; + index + .generations + .retain(|indexed| indexed.generation != generation); + self.write_index(&index)?; + sync(&self.dir)?; + self.remove_tombstone(generation)?; + Ok(Retirement::Retired) + } + + /// Returns the error of the generation `generation`, which the store does not index. + fn not_indexed(&self, generation: StoreGeneration) -> Error { + Error::new( + ErrorKind::NotFound, + format!( + "the template store {} holds no generation {generation}", + self.dir.display() + ), + ) + } + + /// Returns the path of the record of the lease `id`. + fn lease_path(&self, id: LeaseId) -> PathBuf { + self.dir.join(LEASES_NAME).join(format!("{id}.json")) + } + + /// Returns the path of the tombstone of the store generation `generation`. + fn tombstone_path(&self, generation: StoreGeneration) -> PathBuf { + self.dir + .join(TOMBSTONES_NAME) + .join(format!("{generation}.json")) + } + + /// Creates the store's directory `name`, which `what` names, private to the current user, + /// unless it exists, under the store's lock, and makes its entry in the store durable with + /// `sync`: it syncs the store's directory once it creates the directory, and once it finds + /// one that this process has not synced yet, whose creation may not have finished its sync. + fn create_records_dir(&self, name: &'static str, what: &str, sync: SyncDir<'_>) -> Result<()> { + let key = (self.dir.clone(), self.id, name); + if !self.check_records_dir(name, what)? { + let dir = self.dir.join(name); + let problem = platform::create_owner_only_dir(&dir).map_err(store_error(format!( + "cannot create {what} {}", + dir.display() + )))?; + refuse_problem(&dir, what, problem)?; + } else if durable_record_dirs().contains(&key) { + return Ok(()); + } + sync(&self.dir)?; + let mut durable = durable_record_dirs(); + if !durable.contains(&key) { + durable.push(key); + } + Ok(()) + } + + /// Checks that the store's directory `name`, which `what` names, is private to the current + /// user, and not a link, if it exists, and returns whether it exists. + fn check_records_dir(&self, name: &str, what: &str) -> Result { + let dir = self.dir.join(name); + match platform::check_owner_only_dir(&dir) { + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(false), + checked => { + let problem = checked + .map_err(store_error(format!("cannot open {what} {}", dir.display())))?; + refuse_problem(&dir, what, problem)?; + Ok(true) + } + } + } + + /// Lists the store's directory `name`, which `what` names, whose records `parse` names: the + /// records, and the temporary files that interrupted replacements of them left. A directory + /// that does not exist holds none. + /// + /// Fails with [`ErrorCode::ForeignFiles`] for any other entry. + fn records( + &self, + name: &str, + what: &str, + parse: impl Fn(&str) -> Option, + ) -> Result> { + let mut listed = Records { + records: Vec::new(), + temporaries: Vec::new(), + }; + if !self.check_records_dir(name, what)? { + return Ok(listed); + } + let dir = self.dir.join(name); + let mut foreign = Vec::new(); + for entry in entries(&dir)? { + if let Some(record) = parse(&entry) { + listed.records.push(record); + } else if is_temporary_of(&entry, |file| parse(file).is_some()) { + listed.temporaries.push(dir.join(entry)); + } else { + foreign.push(entry); + } + } + if foreign.is_empty() { + return Ok(listed); + } + foreign.sort(); + Err(Error::new( + ErrorCode::ForeignFiles, + format!( + "{what} {} holds entries that are not the store's: {}", + dir.display(), + foreign.join(", ") + ), + )) + } + + /// Reads the record of the lease `id`, or returns `None` if it does not exist. + fn read_lease(&self, id: LeaseId) -> Result> { + let path = self.lease_path(id); + let Some(lease) = read_lease(&path, LEASE_DESCRIPTION)? else { + return Ok(None); + }; + if lease.id != id || lease.store != self.id { + return Err(Error::io(format!( + "the {LEASE_DESCRIPTION} {} is malformed: it is the lease {} of the template \ + store {}", + path.display(), + lease.id, + lease.store + ))); + } + Ok(Some(lease)) + } + + /// Returns whether a tombstone quarantines the generation `generation`. + fn is_tombstoned(&self, generation: StoreGeneration) -> Result { + if !self.check_records_dir(TOMBSTONES_NAME, TOMBSTONES_WHAT)? { + return Ok(false); + } + let path = self.tombstone_path(generation); + let Some(record) = + read_record::(&path, TOMBSTONE_DESCRIPTION, MAX_LEASE_RECORD_LEN)? + else { + return Ok(false); + }; + if record.store != self.id.to_string() || record.generation != generation.get().get() { + return Err(Error::io(format!( + "the {TOMBSTONE_DESCRIPTION} {} is malformed: it is the tombstone of the \ + generation {} of the template store {}", + path.display(), + record.generation, + record.store + ))); + } + Ok(true) + } + + /// Writes the tombstone of the generation `generation`, unless it has one, under the store's + /// lock, and makes it durable with `sync`, even one that an earlier transaction wrote, whose + /// sync may have failed. + fn write_tombstone(&self, generation: StoreGeneration, sync: SyncDir<'_>) -> Result<()> { + if !self.is_tombstoned(generation)? { + self.create_records_dir(TOMBSTONES_NAME, TOMBSTONES_WHAT, sync)?; + write_record( + &self.tombstone_path(generation), + &TombstoneRecord { + format: STORE_FORMAT, + store: self.id.to_string(), + generation: generation.get().get(), + }, + "the tombstone of the template store", + )?; + } + sync(&self.dir.join(TOMBSTONES_NAME)) + } + + /// Removes the tombstone of the generation `generation`, if it has one, under the store's + /// lock. + fn remove_tombstone(&self, generation: StoreGeneration) -> Result<()> { + if self.check_records_dir(TOMBSTONES_NAME, TOMBSTONES_WHAT)? { + remove_if_present(&self.tombstone_path(generation))?; + } + Ok(()) + } + + /// Removes the payload of the generation `generation`, if it exists, under the store's lock. + /// A payload that is a link, or not private to the current user, is refused, and kept. + fn remove_payload(&self, generation: StoreGeneration) -> Result<()> { + self.check_generations_dir()?; + let payload = self.payload_dir(generation); + let what = "the payload of the generation"; + match platform::check_owner_only_dir(&payload) { + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + checked => { + let problem = checked.map_err(store_error(format!( + "cannot open {what} {}", + payload.display() + )))?; + refuse_problem(&payload, what, problem)?; + } + } + match fs::remove_dir_all(&payload) { + Err(error) if error.kind() != io::ErrorKind::NotFound => Err(store_error(format!( + "cannot remove {what} {}", + payload.display() + ))(error)), + _ => Ok(()), + } + } +} + +/// Returns the error of a lease that the sandbox in `dir` cannot take or end while it records a +/// VM or a launch, [`ErrorCode::SandboxRunning`]. +fn may_run(dir: &Path) -> Error { + Error::new( + ErrorCode::SandboxRunning, + format!( + "a VM of the sandbox in {}, or a launch of it, may still run", + dir.display() + ), + ) +} + +/// Returns whether the clone binding of the sandbox of `lease` names it still: unless the sandbox +/// is gone, holds no clone binding, or holds one of another lease. +fn is_bound(lease: &Lease) -> Result { + Ok(read_binding(&lease.sandbox)? + .is_some_and(|binding| binding.id == lease.id && binding.store == lease.store)) +} + +/// Returns the directory whose entries changed last as the clone binding of the sandbox in +/// `sandbox` went: the sandbox's own directory, if it remains one, and otherwise the nearest of +/// its ancestors that is a directory, following the links that its path names, as the removal of +/// the sandbox followed them, where the sandbox's entry went. +/// +/// Fails for an ancestor that cannot be examined, or that is a link to nothing, whose target, the +/// directory that held the sandbox, cannot be known. +fn binding_dir(sandbox: &Path) -> Result<&Path> { + if fs::symlink_metadata(sandbox).is_ok_and(|metadata| metadata.is_dir()) { + return Ok(sandbox); + } + for ancestor in sandbox.ancestors().skip(1) { + let unknown = |error: io::Error| { + Error::io(format!( + "cannot find the directory from which the sandbox in {} went, through {}", + sandbox.display(), + ancestor.display() + )) + .with_source(error) + }; + match fs::metadata(ancestor) { + Ok(metadata) if metadata.is_dir() => return Ok(ancestor), + // A file replaced the directory, in the entries of its parent. + Ok(_) => {} + Err(error) if error.kind() == io::ErrorKind::NotFound => { + if fs::symlink_metadata(ancestor).is_ok() { + return Err(unknown(error)); + } + } + Err(error) => return Err(unknown(error)), + } + } + Err(Error::io(format!( + "no directory holds the sandbox in {}", + sandbox.display() + ))) +} + +/// Writes `record` as the clone binding of the sandbox in `dir`, and makes it durable with `sync`. +fn bind(dir: &Path, record: &LeaseRecord, sync: SyncDir<'_>) -> Result<()> { + write_record(&dir.join(CLONE_BINDING_NAME), record, "the clone binding")?; + sync(dir) +} + +/// Removes the clone binding of the sandbox in `dir`, if it has one, and makes its removal durable +/// with `sync`, before a record of its lease goes: a binding that outlasted the record through a +/// crash would let a retirement delete the generation that it names. +fn unbind(dir: &Path, sync: SyncDir<'_>) -> Result<()> { + remove_if_present(&dir.join(CLONE_BINDING_NAME))?; + sync(dir) +} + +/// Returns `error`, which ended an acquisition of the lease `id` whose binding the sandbox in +/// `dir` may hold still, with the error's code and source, saying so. +fn held_still(error: Error, dir: &Path, id: LeaseId) -> Error { + let code = error.code(); + let (_, message, source) = error.into_parts(); + let error = Error::new( + code, + format!( + "{message}; the sandbox in {} may hold the lease {id} still, which a release ends", + dir.display() + ), + ); + match source { + Some(source) => error.with_source(source), + None => error, + } +} + +/// Reads the clone binding of the sandbox in `dir`, or returns `None` if it holds none, or if the +/// directory does not exist. +fn read_binding(dir: &Path) -> Result> { + read_lease(&dir.join(CLONE_BINDING_NAME), BINDING_DESCRIPTION) +} + +/// Reads the record of a lease at `path`, its `description`, or returns `None` if it does not +/// exist. +fn read_lease(path: &Path, description: &str) -> Result> { + let Some(record) = read_record::(path, description, MAX_LEASE_RECORD_LEN)? else { + return Ok(None); + }; + record.into_lease().map(Some).map_err(|problem| { + Error::io(format!( + "the {description} {} is malformed: {problem}", + path.display() + )) + }) +} + +/// Parses the name of the record of a lease, `ID.json`, in the form in which the store writes +/// it. +fn lease_name(name: &str) -> Option { + let stem = name.strip_suffix(".json")?; + let id: LeaseId = stem.parse().ok()?; + (id.to_string() == stem).then_some(id) +} + +/// Parses the name of a tombstone, `G.json`, in the form in which the store writes it. +fn tombstone_name(name: &str) -> Option { + let stem = name.strip_suffix(".json")?; + let generation = NonZeroU64::new(stem.parse().ok()?).map(StoreGeneration::new)?; + (generation.to_string() == stem).then_some(generation) +} + +/// Returns whether `name` is a temporary file that an interrupted replacement of a record whose +/// name `record` accepts leaves beside it: `.NAME.NUMBER.tmp`. +fn is_temporary_of(name: &str, record: impl Fn(&str) -> bool) -> bool { + name.strip_prefix('.') + .and_then(|name| name.strip_suffix(".tmp")) + .and_then(|name| name.rsplit_once('.')) + .is_some_and(|(file, number)| { + record(file) && !number.is_empty() && number.bytes().all(|byte| byte.is_ascii_digit()) + }) +} + +/// The entries of the directory of a store's leases or tombstones. +struct Records { + /// What the names of the records name. + records: Vec, + /// The temporary files that interrupted replacements of records left. + temporaries: Vec, +} + +/// The record of a lease, as JSON: in its store, and as the clone binding of its sandbox. +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct LeaseRecord { + /// [`STORE_FORMAT`]. + format: u32, + /// The identity of the store that granted the lease. + store: String, + /// The identity of the lease. + lease: String, + /// The store generation of the leased generation. + generation: u64, + /// The identity of the leased generation's key. + key: String, + /// The directory of the sandbox that holds the lease, as an absolute path. + sandbox: String, +} + +impl LeaseRecord { + /// Returns the record of `lease`. + fn new(lease: &Lease) -> Self { + Self { + format: STORE_FORMAT, + store: lease.store.to_string(), + lease: lease.id.to_string(), + generation: lease.generation.get().get(), + key: lease.key.to_string(), + sandbox: lease.sandbox.to_string_lossy().into_owned(), + } + } + + /// Checks the record, and returns its lease, or says why it is malformed. + fn into_lease(self) -> std::result::Result { + let field = + |what: &str, error: Error| format!("its {what} is invalid: {}", error.message()); + let sandbox = PathBuf::from(self.sandbox); + if !sandbox.is_absolute() { + return Err(format!( + "its sandbox, {}, is not an absolute path", + sandbox.display() + )); + } + Ok(Lease { + id: self + .lease + .parse() + .map_err(|error| field("identity", error))?, + store: self.store.parse().map_err(|error| field("store", error))?, + generation: NonZeroU64::new(self.generation) + .map(StoreGeneration::new) + .ok_or("it leases store generation 0, which no generation has")?, + key: self.key.parse().map_err(|error| field("key", error))?, + sandbox, + }) + } +} + +/// A tombstone, as JSON. +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct TombstoneRecord { + /// [`STORE_FORMAT`]. + format: u32, + /// The identity of the store. + store: String, + /// The store generation of the tombstoned generation. + generation: u64, +} diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs new file mode 100644 index 000000000..04550b17a --- /dev/null +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -0,0 +1,1338 @@ +//! Tests of leases, clone bindings, tombstones, and retirement, by one process, and by +//! transactions in processes of their own, which [`transaction_role`] runs. + +use std::cell::{Cell, RefCell}; +use std::env; +use std::io::{BufRead, BufReader, Read, Write}; +use std::process::{Child, ChildStdin, ChildStdout, Command, Stdio}; +use std::time::Duration; + +use serde_json::{Value, json}; + +use super::*; +use crate::host::store::files::write_private; +use crate::host::store::{FORMAT, RUNTIME_NAME, Recovery, RuntimeIdentity}; +use crate::host::template::eligibility::MAX_DOWNTIME_NS; +use crate::host::template::store::tests::{ + link_dir, publish, publish_with, remove, storage, store as new_store, unlink_dir, +}; +use crate::host::template::test_support::{ManifestSpec, artifact, network_class}; +use crate::protocol::CAPABILITY_LEN; + +/// A change to a record, as JSON. +type Change = fn(&mut Value); + +/// Nanoseconds in a second. +const SECOND: u64 = 1_000_000_000; +/// Nanoseconds in a day. +const DAY: u64 = 86_400 * SECOND; +/// The configuration record of these tests' sandboxes. +const CONFIG: &str = "config.json"; +/// The directory of the store in which [`transaction_role`] runs its transaction. +const ROLE_STORE: &str = "NVX_TEMPLATE_LEASE_TEST_STORE"; +/// The transaction that [`transaction_role`] runs: `acquire`, `release`, `tombstone`, or +/// `retire`. +const ROLE_ACTION: &str = "NVX_TEMPLATE_LEASE_TEST_ACTION"; +/// The store generation of the transaction of [`transaction_role`]. +const ROLE_GENERATION: &str = "NVX_TEMPLATE_LEASE_TEST_GENERATION"; +/// The directory of the sandbox of the transaction of [`transaction_role`], if it has one. +const ROLE_SANDBOX: &str = "NVX_TEMPLATE_LEASE_TEST_SANDBOX"; +/// The line that [`transaction_role`] prints once it is ready, before it waits for a line on its +/// standard input. +const READY: &str = "nvx-template-lease-test-ready"; +/// The start of the line in which [`transaction_role`] prints how its transaction ended. +const OUTCOME: &str = "nvx-template-lease-test-outcome "; + +/// Returns the files that these tests' callers keep in their sandboxes. +fn files() -> CallerFiles { + CallerFiles::new(CONFIG).unwrap() +} + +/// Provisions the sandbox `name` in `parent`, and returns its directory. +fn provision(parent: &Path, name: &str) -> PathBuf { + let dir = parent.join(name); + let locked = SandboxStore::lock_for_provision(&dir, &files(), None).unwrap(); + fs::write(dir.join(CONFIG), b"{}").unwrap(); + drop(locked); + dir +} + +/// Returns the time of the host that captured `generation`, during the boot of the capture, +/// `seconds` after it. +fn after(generation: &Generation, seconds: u64) -> HostTime { + let time = generation.time(); + HostTime { + host_id: time.host_id, + boot_id: time.boot_id, + clock: time.host_clock, + monotonic_ns: time.capture_monotonic_ns + seconds * SECOND, + utc_ns: time.capture_utc_ns + seconds * SECOND, + } +} + +/// Leases `generation` of `store` to the sandbox in `sandbox`, a second after its capture, and +/// releases the sandbox's lock. +fn leased(store: &Store, generation: &Generation, sandbox: &Path) -> Lease { + store + .acquire( + generation.generation(), + sandbox, + &files(), + &after(generation, 1), + None, + ) + .unwrap() + .lease +} + +/// Returns the error with which leasing `generation` of `store` to the sandbox in `sandbox` at +/// `now` fails. +fn refusal(store: &Store, generation: StoreGeneration, sandbox: &Path, now: &HostTime) -> Error { + store + .acquire(generation, sandbox, &files(), now, None) + .unwrap_err() +} + +/// Releases the lease of the sandbox in `sandbox` from `store`, under the sandbox's lock. +fn release(store: &Store, sandbox: &Path) -> Result> { + let mut locked = SandboxStore::lock_existing(sandbox, &files(), None).unwrap(); + store.release(&mut locked) +} + +/// Returns the lease that the clone binding of the sandbox in `sandbox` records, if any. +fn binding(sandbox: &Path) -> Option { + Lease::of(&SandboxStore::open(sandbox).unwrap()).unwrap() +} + +/// Returns the store generation `generation`, which must not be 0. +fn store_generation(generation: u64) -> StoreGeneration { + StoreGeneration::new(NonZeroU64::new(generation).unwrap()) +} + +/// Returns a runtime identity under this crate's contracts. +fn runtime() -> RuntimeIdentity { + RuntimeIdentity::new(artifact('a'), artifact('b'), artifact('c')) +} + +/// Records a VM of the sandbox in `sandbox`, as the commit of its launch does. +fn record_vm(sandbox: &Path) { + let record = json!({ + "format": FORMAT, + "launch": "0123456789abcdef0123456789abcdef", + "process": { "pid": 42, "startTime": 4242, "boot": "ab".repeat(16) }, + "endpoint": "/state/sandbox/control.sock", + "runtime": serde_json::to_value(runtime()).unwrap(), + }); + write_private(&sandbox.join(RUNTIME_NAME), record.to_string().as_bytes()).unwrap(); +} + +/// Returns the file at `path` as JSON. +fn json_of(path: &Path) -> Value { + serde_json::from_slice(&fs::read(path).unwrap()).unwrap() +} + +/// Returns the identities of `leases`, sorted, and separated by spaces. +fn ids(leases: &[Lease]) -> String { + let mut ids: Vec = leases.iter().map(|lease| lease.id().to_string()).collect(); + ids.sort(); + ids.join(" ") +} + +#[test] +fn leases_bind_clones_to_the_generations_that_they_restore() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let first = provision(directory.path(), "first"); + let acquired = store + .acquire( + published.generation(), + &first, + &files(), + &after(&published, 1), + None, + ) + .unwrap(); + let lease = acquired.lease.clone(); + assert_eq!(lease.store(), store.id()); + assert_eq!(lease.generation(), published.generation()); + assert_eq!(lease.key(), published.key()); + assert_eq!(lease.sandbox(), first); + assert_eq!( + lease.template(), + TemplateReference { + generation: published.generation(), + key: Identity::from(published.key()), + } + ); + // The caller keeps the sandbox's lock, under which it launches the clone, until it drops it. + assert_eq!(acquired.locked.dir(), first); + let busy = SandboxStore::lock_existing(&first, &files(), Some(Instant::now())).unwrap_err(); + assert_eq!(busy.code(), ErrorCode::SandboxBusy, "{busy}"); + drop(acquired); + // The clone binding in the sandbox and the record in the store are the same lease. + assert_eq!(binding(&first), Some(lease.clone())); + assert_eq!( + store.leases(published.generation()).unwrap(), + std::slice::from_ref(&lease) + ); + // A sandbox holds one lease at a time. + let error = refusal( + &store, + published.generation(), + &first, + &after(&published, 2), + ); + assert_eq!(error.code(), ErrorCode::SandboxLeased, "{error}"); + assert!(error.message().contains(&lease.id().to_string()), "{error}"); + // Other sandboxes lease the generation too, and other generations. + let second = provision(directory.path(), "second"); + let mut leases = vec![lease, leased(&store, &published, &second)]; + leases.sort_by_key(Lease::id); + let other = publish(&store, &ManifestSpec::template(), storage(2)); + let third = provision(directory.path(), "third"); + let other_lease = leased(&store, &other, &third); + assert_eq!(store.leases(published.generation()).unwrap(), leases); + assert_eq!(store.leases(other.generation()).unwrap(), [other_lease]); +} + +#[test] +fn acquisitions_refuse_what_they_cannot_lease_and_lease_nothing() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let now = after(&published, 1); + // A generation that the store does not index. + let error = refusal(&store, store_generation(9), &sandbox, &now); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + // One that is not eligible at the host's time: no longer, or not yet. + let expired = after(&published, MAX_DOWNTIME_NS / SECOND + 1); + let mut early = after(&published, 0); + early.monotonic_ns -= 1; + for (now, condition) in [ + (expired, "is no longer eligible"), + (early, "is not eligible yet"), + ] { + let error = refusal(&store, generation, &sandbox, &now); + assert_eq!(error.code(), ErrorCode::TemplateIneligible, "{error}"); + assert!(error.message().contains(condition), "{error}"); + } + // A sandbox that is not provisioned: its directory is missing, or holds no configuration. + let unprovisioned = directory.path().join("unprovisioned"); + drop(SandboxStore::lock_for_provision(&unprovisioned, &files(), None).unwrap()); + for dir in [directory.path().join("missing"), unprovisioned] { + let error = refusal(&store, generation, &dir, &now); + assert_eq!(error.code(), ErrorCode::SandboxNotProvisioned, "{error}"); + } + // A sandbox that records a launch, or a VM, which may still run. + let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); + drop( + locked + .begin_launch(&runtime(), &[7; CAPABILITY_LEN]) + .unwrap(), + ); + drop(locked); + let error = refusal(&store, generation, &sandbox, &now); + assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); + let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); + assert_eq!(locked.recover().unwrap(), Recovery::Exited); + record_vm(&sandbox); + drop(locked); + let error = refusal(&store, generation, &sandbox, &now); + assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); + let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); + locked.clear_runtime().unwrap(); + drop(locked); + // A generation that a tombstone quarantines. + store.tombstone(generation, None).unwrap(); + let error = refusal(&store, generation, &sandbox, &now); + assert_eq!(error.code(), ErrorCode::TemplateTombstoned, "{error}"); + // None of them leased anything. + assert!(store.leases(generation).unwrap().is_empty()); + assert_eq!(binding(&sandbox), None); + assert!(!store.dir().join(LEASES_NAME).exists()); +} + +#[test] +fn releases_end_leases_once_their_clones_have_exited() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let mut acquired = store + .acquire(generation, &sandbox, &files(), &after(&published, 1), None) + .unwrap(); + let lease = acquired.lease.clone(); + // While a launch of the clone, or its VM, may run, its lease stays. + drop( + acquired + .locked + .begin_launch(&runtime(), &[7; CAPABILITY_LEN]) + .unwrap(), + ); + let error = store.release(&mut acquired.locked).unwrap_err(); + assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); + assert_eq!(acquired.locked.recover().unwrap(), Recovery::Exited); + record_vm(&sandbox); + let error = store.release(&mut acquired.locked).unwrap_err(); + assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); + assert_eq!(binding(&sandbox), Some(lease.clone())); + assert_eq!( + store.leases(generation).unwrap(), + std::slice::from_ref(&lease) + ); + // Once its VM has exited, the release removes the binding, and then the store's record. + acquired.locked.clear_runtime().unwrap(); + assert_eq!( + store.release(&mut acquired.locked).unwrap(), + Some(lease.clone()) + ); + drop(acquired); + assert_eq!(binding(&sandbox), None); + assert!(store.leases(generation).unwrap().is_empty()); + assert_eq!(release(&store, &sandbox).unwrap(), None); + // The sandbox may lease the generation again, under another identity. + let again = leased(&store, &published, &sandbox); + assert_ne!(again.id(), lease.id()); + // Another store's release keeps the lease. + let (_other_directory, other) = new_store(); + let error = release(&other, &sandbox).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Other, "{error}"); + assert_eq!(binding(&sandbox), Some(again.clone())); + assert_eq!(store.leases(generation).unwrap(), [again]); +} + +#[test] +fn tombstones_quarantine_generations_and_keep_their_leases() { + let (directory, store) = new_store(); + let first = publish(&store, &ManifestSpec::template(), storage(1)); + let second = publish(&store, &ManifestSpec::template(), storage(2)); + let leasing = provision(directory.path(), "leasing"); + let lease = leased(&store, &second, &leasing); + let now = after(&second, 1); + assert_eq!( + store.select(second.key(), &now).unwrap(), + Some(second.clone()) + ); + assert!(store.tombstones().unwrap().is_empty()); + // A tombstone stops new leases, and selection skips the generation, whose leases stay. + store.tombstone(second.generation(), None).unwrap(); + store.tombstone(second.generation(), None).unwrap(); + assert_eq!(store.tombstones().unwrap(), [second.generation()]); + let sandbox = provision(directory.path(), "sandbox"); + let error = refusal(&store, second.generation(), &sandbox, &now); + assert_eq!(error.code(), ErrorCode::TemplateTombstoned, "{error}"); + assert_eq!(binding(&leasing), Some(lease.clone())); + assert_eq!(store.leases(second.generation()).unwrap(), [lease]); + assert_eq!( + store.select(second.key(), &now).unwrap(), + Some(first.clone()) + ); + leased(&store, &first, &sandbox); + // A tombstone needs a generation that the store indexes. + let error = store.tombstone(store_generation(9), None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + assert_eq!(store.tombstones().unwrap(), [second.generation()]); +} + +#[test] +fn selection_takes_the_latest_eligible_generation_of_a_key() { + let (_directory, store) = new_store(); + let first = publish(&store, &ManifestSpec::template(), storage(1)); + let mut spec = ManifestSpec::template(); + spec.time().capture_utc_ns += 10 * DAY; + spec.time().capture_monotonic_ns += 10 * DAY; + let second = publish(&store, &spec, storage(2)); + let other = publish_with( + &store, + &network_class(), + &ManifestSpec::with_network(), + storage(3), + ); + assert_eq!(second.key(), first.key()); + assert_ne!(other.key(), first.key()); + // The host's time `days` after the first capture, during its boot. + let at = |days: u64| HostTime { + monotonic_ns: first.time().capture_monotonic_ns + days * DAY, + utc_ns: first.time().capture_utc_ns + days * DAY, + ..after(&first, 0) + }; + let select = |now: &HostTime| store.select(first.key(), now).unwrap(); + // Before the second capture, only the first generation is eligible; then both are, and the + // latest is selected; after the first's window, only the second is, and after both, none. + assert_eq!(select(&at(1)), Some(first.clone())); + assert_eq!(select(&at(11)), Some(second.clone())); + assert_eq!(select(&at(35)), Some(second.clone())); + assert_eq!(select(&at(45)), None); + // A quarantined generation is skipped. + store.tombstone(second.generation(), None).unwrap(); + assert_eq!(select(&at(11)), Some(first.clone())); + assert_eq!(select(&at(35)), None); + // Each key selects among its own generations. + assert_eq!(store.select(other.key(), &at(1)).unwrap(), Some(other)); + assert_eq!( + store + .select(KeyIdentity::from_bytes([0; 32]), &at(1)) + .unwrap(), + None + ); +} + +#[test] +fn retirement_deletes_a_generation_once_its_leases_drain() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + // The generation stays, quarantined, while its lease lasts. + for _ in 0..2 { + assert_eq!( + store.retire(generation, None).unwrap(), + Retirement::Draining(vec![lease.clone()]) + ); + } + assert_eq!(store.tombstones().unwrap(), [generation]); + assert_eq!(store.index().unwrap().get(generation), Some(&published)); + assert!(store.snapshot_dir(&published).is_ok()); + let other = provision(directory.path(), "other"); + let error = refusal(&store, generation, &other, &after(&published, 1)); + assert_eq!(error.code(), ErrorCode::TemplateTombstoned, "{error}"); + // Once its clone releases it, the generation goes: its payload, its entry, and its tombstone. + assert_eq!(release(&store, &sandbox).unwrap(), Some(lease)); + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); + assert!(!store.payload_dir(generation).exists()); + assert_eq!(store.index().unwrap().get(generation), None); + assert!(store.tombstones().unwrap().is_empty()); + assert!(store.leases(generation).unwrap().is_empty()); + // Retiring it again finds it retired, and its store generation is never assigned again. + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); + let next = publish(&store, &ManifestSpec::template(), storage(1)); + assert!(next.generation() > generation); + let unassigned = store_generation(next.generation().get().get() + 1); + let error = store.retire(unassigned, None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); + let error = refusal(&store, generation, &other, &after(&published, 1)); + assert_eq!(error.kind(), ErrorKind::NotFound, "{error}"); +} + +#[test] +fn retirement_removes_the_leases_that_no_clone_binds() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandboxes: Vec = ["released", "removed", "leased-again", "bound"] + .into_iter() + .map(|name| provision(directory.path(), name)) + .collect(); + let leases: Vec = sandboxes + .iter() + .map(|sandbox| leased(&store, &published, sandbox)) + .collect(); + // A release that was interrupted once it had removed the clone binding. + fs::remove_file(sandboxes[0].join(CLONE_BINDING_NAME)).unwrap(); + // A sandbox whose removal removed its binding with the rest. + let locked = SandboxStore::lock_existing(&sandboxes[1], &files(), None).unwrap(); + assert!(locked.remove().unwrap().is_none()); + assert!(!sandboxes[1].exists()); + // A sandbox that leased the generation again after such a release. + fs::remove_file(sandboxes[2].join(CLONE_BINDING_NAME)).unwrap(); + let again = leased(&store, &published, &sandboxes[2]); + let mut listed = leases.clone(); + listed.push(again.clone()); + listed.sort_by_key(Lease::id); + assert_eq!(store.leases(generation).unwrap(), listed); + // The retirement removes the records of the first three, and drains the bound ones. + let mut live = vec![again, leases[3].clone()]; + live.sort_by_key(Lease::id); + assert_eq!( + store.retire(generation, None).unwrap(), + Retirement::Draining(live.clone()) + ); + assert_eq!(store.leases(generation).unwrap(), live); + for sandbox in &sandboxes[2..] { + assert!(release(&store, sandbox).unwrap().is_some()); + } + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); +} + +#[test] +fn interrupted_retirements_finish() { + let (directory, store) = new_store(); + let first = publish(&store, &ManifestSpec::template(), storage(1)); + let second = publish(&store, &ManifestSpec::template(), storage(2)); + let third = publish(&store, &ManifestSpec::template(), storage(3)); + // One that was interrupted once it had removed the payload leaves the generation indexed and + // quarantined. + store.tombstone(first.generation(), None).unwrap(); + fs::remove_dir_all(store.payload_dir(first.generation())).unwrap(); + assert_eq!( + store.retire(first.generation(), None).unwrap(), + Retirement::Retired + ); + assert_eq!(store.index().unwrap().get(first.generation()), None); + // One that was interrupted once it had removed the entry from the index leaves the tombstone. + store.tombstone(second.generation(), None).unwrap(); + fs::remove_dir_all(store.payload_dir(second.generation())).unwrap(); + remove(&store, second.generation()); + assert_eq!(store.tombstones().unwrap(), [second.generation()]); + assert_eq!( + store.retire(second.generation(), None).unwrap(), + Retirement::Retired + ); + assert!(store.tombstones().unwrap().is_empty()); + // Interrupted replacements of records leave temporary files, which listings skip, and which + // a retirement removes under the store's lock. + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &third, &sandbox); + store.tombstone(third.generation(), None).unwrap(); + let temporaries = [ + store + .dir() + .join(LEASES_NAME) + .join(format!(".{}.json.42.tmp", lease.id())), + store.dir().join(TOMBSTONES_NAME).join(".7.json.1.tmp"), + ]; + for temporary in &temporaries { + fs::write(temporary, b"{").unwrap(); + } + assert_eq!( + store.leases(third.generation()).unwrap(), + std::slice::from_ref(&lease) + ); + assert_eq!(store.tombstones().unwrap(), [third.generation()]); + assert_eq!( + store.retire(third.generation(), None).unwrap(), + Retirement::Draining(vec![lease]) + ); + for temporary in &temporaries { + assert!(!temporary.exists(), "{}", temporary.display()); + } +} + +#[test] +fn acquisitions_make_their_records_durable_before_their_bindings_and_both_before_they_return() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let leases_dir = store.dir().join(LEASES_NAME); + let first = provision(directory.path(), "first"); + let second = provision(directory.path(), "second"); + // Each sync notes its directory, the records of leases in the store, and the bindings of the + // sandboxes. + let synced = RefCell::new(Vec::new()); + let sync = |dir: &Path| -> Result<()> { + let records = fs::read_dir(&leases_dir).map_or(0, Iterator::count); + let bindings = [&first, &second] + .into_iter() + .filter(|sandbox| sandbox.join(CLONE_BINDING_NAME).exists()) + .count(); + synced + .borrow_mut() + .push((dir.to_owned(), records, bindings)); + Ok(()) + }; + // An acquisition that creates the directory of the leases, but cannot make its entry in the + // store durable, leases nothing, and the next acquisition syncs the store's directory again. + let now = after(&published, 1); + let unsynced_store = |dir: &Path| { + if dir == store.dir() { + Err(Error::io("the disk failed")) + } else { + Ok(()) + } + }; + let error = store + .acquire_with( + published.generation(), + first.clone(), + &files(), + &now, + None, + &unsynced_store, + ) + .unwrap_err(); + assert_eq!(error.message(), "the disk failed"); + assert!(leases_dir.is_dir()); + assert_eq!(binding(&first), None); + for sandbox in [&first, &second] { + let acquired = store + .acquire_with( + published.generation(), + sandbox.clone(), + &files(), + &now, + None, + &sync, + ) + .unwrap(); + drop(acquired); + } + assert_eq!( + *synced.borrow(), + [ + // The first makes the entry of the directory of the leases durable, which no + // acquisition of this process did before, ahead of any record in it. + (store.dir().to_owned(), 0, 0), + // Each makes its record durable before its binding, and its binding before it + // returns. + (leases_dir.clone(), 1, 0), + (first.clone(), 1, 1), + (leases_dir.clone(), 2, 1), + (second.clone(), 2, 2), + ] + ); +} + +#[test] +fn an_acquisition_keeps_its_record_while_its_binding_may_outlast_a_crash() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let now = after(&published, 1); + let sandbox = provision(directory.path(), "sandbox"); + let leases_dir = store.dir().join(LEASES_NAME); + let failed = || Err(Error::io("the disk failed")); + let acquire = |sync: SyncDir<'_>| { + store + .acquire_with(generation, sandbox.clone(), &files(), &now, None, sync) + .unwrap_err() + }; + // A record that cannot be made durable goes before any binding names it. + let error = acquire(&|dir: &Path| if dir == leases_dir { failed() } else { Ok(()) }); + assert_eq!(error.message(), "the disk failed"); + assert_eq!(binding(&sandbox), None); + assert!(store.leases(generation).unwrap().is_empty()); + // A binding that cannot be made durable goes, and its record once the binding's removal is + // durable. + let first = Cell::new(true); + let error = acquire(&|dir: &Path| { + if dir == sandbox && first.replace(false) { + failed() + } else { + Ok(()) + } + }); + assert_eq!(error.message(), "the disk failed"); + assert_eq!(binding(&sandbox), None); + assert!(store.leases(generation).unwrap().is_empty()); + // A binding whose removal cannot be made durable either may outlast a crash, so its record + // stays, and keeps the generation, until a retirement finds that no binding names it. + let error = acquire(&|dir: &Path| if dir == sandbox { failed() } else { Ok(()) }); + let [kept]: [Lease; 1] = store.leases(generation).unwrap().try_into().unwrap(); + assert_eq!( + error.message(), + format!( + "the disk failed; the sandbox in {} may hold the lease {} still, which a release ends", + sandbox.display(), + kept.id() + ) + ); + assert_eq!(binding(&sandbox), None); + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); +} + +#[test] +fn a_release_makes_the_removal_of_its_binding_durable_before_it_removes_its_record() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + // Each sync notes its directory, and whether the binding and the record existed then. + let synced = RefCell::new(Vec::new()); + let sync = |dir: &Path| -> Result<()> { + let bound = sandbox.join(CLONE_BINDING_NAME).exists(); + synced + .borrow_mut() + .push((dir.to_owned(), bound, record.exists())); + Ok(()) + }; + let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); + assert_eq!(store.release_with(&mut locked, &sync).unwrap(), Some(lease)); + drop(locked); + assert_eq!(*synced.borrow(), [(sandbox.clone(), false, true)]); + assert!(!record.exists()); + // A release that cannot make the binding's removal durable keeps the record, which a + // retirement removes once no binding names it. + let again = leased(&store, &published, &sandbox); + let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); + let error = store + .release_with(&mut locked, &|_: &Path| Err(Error::io("the disk failed"))) + .unwrap_err(); + drop(locked); + assert_eq!(error.message(), "the disk failed"); + assert_eq!(binding(&sandbox), None); + assert_eq!(store.leases(generation).unwrap(), [again]); + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); +} + +#[test] +fn a_retirement_makes_each_step_durable_before_the_next() { + let (_directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let tombstones = store.dir().join(TOMBSTONES_NAME); + let tombstone = tombstones.join(format!("{generation}.json")); + let payload = store.payload_dir(generation); + // Each sync notes its directory, and whether the tombstone, the payload, and the + // generation's entry in the index existed then. + let synced = RefCell::new(Vec::new()); + let sync = |dir: &Path| -> Result<()> { + let indexed = store.index()?.get(generation).is_some(); + synced.borrow_mut().push(( + dir.to_owned(), + tombstone.exists(), + payload.exists(), + indexed, + )); + Ok(()) + }; + assert_eq!( + store.retire_with(generation, None, &sync).unwrap(), + Retirement::Retired + ); + assert_eq!( + *synced.borrow(), + [ + // The new directory of the tombstones, before the tombstone in it. + (store.dir().to_owned(), false, true, true), + // The tombstone, before anything goes. + (tombstones.clone(), true, true, true), + // The payload's removal, before the index changes. + (store.dir().join(GENERATIONS_NAME), true, false, true), + // The index, before the tombstone goes. + (store.dir().to_owned(), true, false, false), + ] + ); + assert!(!tombstone.exists()); +} + +#[test] +fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_record() { + let (directory, store) = new_store(); + let first = publish(&store, &ManifestSpec::template(), storage(1)); + let second = publish(&store, &ManifestSpec::template(), storage(2)); + let third = publish(&store, &ManifestSpec::template(), storage(3)); + let released = provision(directory.path(), "released"); + let removed = provision(directory.path(), "removed"); + // A sandbox whose parent the path names through a link. + let sandboxes = directory.path().join("sandboxes"); + fs::create_dir(&sandboxes).unwrap(); + let linked = directory.path().join("linked"); + link_dir(&sandboxes, &linked); + let removed_through_link = provision(&linked, "removed"); + let released_lease = leased(&store, &first, &released); + let removed_lease = leased(&store, &second, &removed); + let linked_lease = leased(&store, &third, &removed_through_link); + // A release that removed its binding, but did not make that durable, and removals of + // sandboxes. + fs::remove_file(released.join(CLONE_BINDING_NAME)).unwrap(); + for sandbox in [&removed, &removed_through_link] { + let locked = SandboxStore::lock_existing(sandbox, &files(), None).unwrap(); + assert!(locked.remove().unwrap().is_none()); + } + let record = |lease: &Lease| { + store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())) + }; + // A retirement that cannot make the binding's absence durable keeps its record, and the + // generation. + let unsynced_sandbox = |dir: &Path| { + if dir == released { + Err(Error::io("the disk failed")) + } else { + Ok(()) + } + }; + let error = store + .retire_with(first.generation(), None, &unsynced_sandbox) + .unwrap_err(); + assert_eq!(error.message(), "the disk failed"); + assert!(record(&released_lease).exists()); + assert!(store.snapshot_dir(&first).is_ok()); + // A record goes once the binding's absence is durable, in the sandbox's directory, or in the + // nearest of its ancestors that remains, through the links that its path names. + for (generation, lease, dir) in [ + (&first, &released_lease, released.as_path()), + (&second, &removed_lease, directory.path()), + (&third, &linked_lease, linked.as_path()), + ] { + let path = record(lease); + let synced = RefCell::new(Vec::new()); + let sync = |synced_dir: &Path| -> Result<()> { + synced + .borrow_mut() + .push((synced_dir.to_owned(), path.exists())); + Ok(()) + }; + assert_eq!( + store + .retire_with(generation.generation(), None, &sync) + .unwrap(), + Retirement::Retired + ); + assert_eq!( + *synced.borrow(), + [ + (store.dir().join(TOMBSTONES_NAME), true), + (dir.to_owned(), true), + (store.dir().join(GENERATIONS_NAME), false), + (store.dir().to_owned(), false), + ] + ); + } + // A link to nothing hides the directory that held a sandbox, so the record stays. + let fourth = publish(&store, &ManifestSpec::template(), storage(4)); + let dangling = provision(&linked, "dangling"); + let dangling_lease = leased(&store, &fourth, &dangling); + let locked = SandboxStore::lock_existing(&dangling, &files(), None).unwrap(); + assert!(locked.remove().unwrap().is_none()); + fs::remove_dir(&sandboxes).unwrap(); + let error = store.retire(fourth.generation(), None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io, "{error}"); + assert!( + error.message().contains("cannot find the directory"), + "{error}" + ); + assert!(record(&dangling_lease).exists()); + assert!(store.snapshot_dir(&fourth).is_ok()); +} + +#[test] +fn a_retirement_that_finds_its_generation_unindexed_makes_the_index_durable_first() { + let (_directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + // A retirement that a crash interrupted once it had written the index, which it may not have + // made durable, leaves the tombstone. + store.tombstone(generation, None).unwrap(); + fs::remove_dir_all(store.payload_dir(generation)).unwrap(); + remove(&store, generation); + let tombstone = store + .dir() + .join(TOMBSTONES_NAME) + .join(format!("{generation}.json")); + let synced = RefCell::new(Vec::new()); + let sync = |dir: &Path| -> Result<()> { + synced + .borrow_mut() + .push((dir.to_owned(), tombstone.exists())); + Ok(()) + }; + assert_eq!( + store.retire_with(generation, None, &sync).unwrap(), + Retirement::Retired + ); + assert_eq!(*synced.borrow(), [(store.dir().to_owned(), true)]); + assert!(!tombstone.exists()); +} + +#[test] +fn directories_are_synced_where_the_platform_can() { + let directory = tempfile::tempdir().unwrap(); + sync_dir(directory.path()).unwrap(); + // On Unix, a sync opens the directory, which must exist. + #[cfg(unix)] + { + let error = sync_dir(&directory.path().join("missing")).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io, "{error}"); + } +} + +#[test] +fn records_keep_their_json_form() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let expected = json!({ + "format": 1, + "store": store.id().to_string(), + "lease": lease.id().to_string(), + "generation": generation.get().get(), + "key": published.key().to_string(), + "sandbox": sandbox.to_str().unwrap(), + }); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + assert_eq!(json_of(&record), expected); + assert_eq!(json_of(&sandbox.join(CLONE_BINDING_NAME)), expected); + store.tombstone(generation, None).unwrap(); + assert_eq!( + json_of( + &store + .dir() + .join(TOMBSTONES_NAME) + .join(format!("{generation}.json")) + ), + json!({ + "format": 1, + "store": store.id().to_string(), + "generation": generation.get().get(), + }) + ); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + for name in [LEASES_NAME, TOMBSTONES_NAME] { + let mode = fs::metadata(store.dir().join(name)) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o700, "{name}"); + } + } + // A lease's identity is its text form. + let text = lease.id().to_string(); + assert_eq!(text.len(), 32); + assert_eq!(text.parse::().unwrap(), lease.id()); + assert_eq!(text.to_uppercase().parse::().unwrap(), lease.id()); + assert_eq!(LeaseId::from_bytes(*lease.id().as_bytes()), lease.id()); + for invalid in ["", "0123", &"g".repeat(32)] { + let error = invalid.parse::().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported, "{invalid}"); + } +} + +#[test] +fn malformed_records_and_foreign_entries_fail_clearly() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + let bound = sandbox.join(CLONE_BINDING_NAME); + let valid = json_of(&record); + let rewrite = |path: &Path, change: Change| { + let mut value = valid.clone(); + change(&mut value); + fs::write(path, value.to_string()).unwrap(); + }; + // A record of another format is neither read nor migrated. + rewrite(&record, |value| value["format"] = json!(2)); + let error = store.leases(generation).unwrap_err(); + assert_eq!(error.code(), ErrorCode::FormatUnsupported, "{error}"); + // Records that are not leases. + let malformed: [(Change, &str); 5] = [ + ( + |value| value["generation"] = json!(0), + "it leases store generation 0", + ), + ( + |value| value["sandbox"] = json!("relative"), + "is not an absolute path", + ), + (|value| value["key"] = json!("00"), "its key is invalid"), + ( + |value| value["lease"] = json!("00"), + "its identity is invalid", + ), + (|value| value["extra"] = json!(1), "is malformed"), + ]; + for (change, problem) in malformed { + rewrite(&bound, change); + let error = Lease::of(&SandboxStore::open(&sandbox).unwrap()).unwrap_err(); + assert_eq!(error.code(), ErrorCode::Io, "{error}"); + assert!(error.message().contains("is malformed"), "{error}"); + assert!(error.message().contains(problem), "{error}"); + } + fs::write(&bound, valid.to_string()).unwrap(); + // A record of another lease than its name names, or of another store. + let other_lease: Change = |value| value["lease"] = json!("0".repeat(32)); + let other_store: Change = |value| value["store"] = json!("0".repeat(32)); + for change in [other_lease, other_store] { + rewrite(&record, change); + let error = store.leases(generation).unwrap_err(); + assert_eq!(error.code(), ErrorCode::Io, "{error}"); + assert!(error.message().contains("is malformed"), "{error}"); + } + fs::write(&record, valid.to_string()).unwrap(); + assert_eq!(store.leases(generation).unwrap(), [lease]); + // Entries that are not the store's, which a retirement refuses before it changes anything. + let notes = store.dir().join(LEASES_NAME).join("notes.txt"); + fs::write(¬es, b"").unwrap(); + for error in [ + store.leases(generation).unwrap_err(), + store.retire(generation, None).unwrap_err(), + ] { + assert_eq!(error.code(), ErrorCode::ForeignFiles, "{error}"); + assert!( + error + .message() + .ends_with("holds entries that are not the store's: notes.txt"), + "{error}" + ); + } + assert!(store.tombstones().unwrap().is_empty()); + fs::remove_file(¬es).unwrap(); + // A tombstone of another store, and entries that are not tombstones. + store.tombstone(generation, None).unwrap(); + let tombstones = store.dir().join(TOMBSTONES_NAME); + let tombstone = json!({ + "format": 1, + "store": "0".repeat(32), + "generation": generation.get().get(), + }); + fs::write( + tombstones.join(format!("{generation}.json")), + tombstone.to_string(), + ) + .unwrap(); + let other = provision(directory.path(), "other"); + let error = refusal(&store, generation, &other, &after(&published, 1)); + assert_eq!(error.code(), ErrorCode::Io, "{error}"); + assert!(error.message().contains("is malformed"), "{error}"); + fs::write(tombstones.join("+1.json"), b"").unwrap(); + for error in [ + store.tombstones().unwrap_err(), + store + .select(published.key(), &after(&published, 1)) + .unwrap_err(), + ] { + assert_eq!(error.code(), ErrorCode::ForeignFiles, "{error}"); + assert!(error.message().ends_with(": +1.json"), "{error}"); + } +} + +#[test] +fn the_directories_of_leases_and_tombstones_refuse_links() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + store.tombstone(generation, None).unwrap(); + for name in [LEASES_NAME, TOMBSTONES_NAME] { + let path = store.dir().join(name); + let moved = path.with_extension("moved"); + fs::rename(&path, &moved).unwrap(); + link_dir(&moved, &path); + let mut errors = vec![store.retire(generation, None).unwrap_err()]; + if name == LEASES_NAME { + errors.push(store.leases(generation).unwrap_err()); + errors.push(release(&store, &sandbox).unwrap_err()); + } else { + errors.push(store.tombstones().unwrap_err()); + errors.push( + store + .select(published.key(), &after(&published, 1)) + .unwrap_err(), + ); + } + for error in errors { + assert!( + error.message().ends_with("is not a plain directory"), + "{name}: {error}" + ); + } + unlink_dir(&path); + fs::rename(&moved, &path).unwrap(); + } + // Nothing changed meanwhile. + assert_eq!(binding(&sandbox), Some(lease.clone())); + assert_eq!(store.leases(generation).unwrap(), [lease]); + assert_eq!(store.tombstones().unwrap(), [generation]); +} + +/// A transaction in a process of its own, which [`transaction_role`] runs. +struct Transaction { + /// The process. + child: Child, + /// Its standard input, a line on which starts the transaction. + input: ChildStdin, + /// Its standard output. + output: BufReader, +} + +impl Transaction { + /// Starts the transaction `action` of the store generation `generation` of the store in + /// `store`, for the sandbox in `sandbox`, if any, and returns once it is ready to run. + fn start( + store: &Path, + action: &str, + generation: StoreGeneration, + sandbox: Option<&Path>, + ) -> Self { + let mut command = Command::new(env::current_exe().unwrap()); + command + .args([ + "--exact", + "host::template::store::leases::tests::transaction_role", + "--ignored", + "--nocapture", + "--test-threads=1", + ]) + .env(ROLE_STORE, store) + .env(ROLE_ACTION, action) + .env(ROLE_GENERATION, generation.to_string()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()); + if let Some(sandbox) = sandbox { + command.env(ROLE_SANDBOX, sandbox); + } + let mut child = command.spawn().unwrap(); + let input = child.stdin.take().unwrap(); + let mut output = BufReader::new(child.stdout.take().unwrap()); + let mut line = String::new(); + while line.trim_end() != READY { + line.clear(); + let read = output.read_line(&mut line).unwrap(); + assert_ne!(read, 0, "the transaction ended before it was ready"); + } + Self { + child, + input, + output, + } + } + + /// Lets the transaction run. + fn run(&mut self) { + self.input.write_all(b"run\n").unwrap(); + self.input.flush().unwrap(); + } + + /// Waits for the transaction to end, and returns how it ended, as [`transaction_role`] + /// prints it. + fn outcome(mut self) -> String { + let mut rest = String::new(); + self.output.read_to_string(&mut rest).unwrap(); + assert!(self.child.wait().unwrap().success(), "{rest}"); + rest.lines() + .find_map(|line| line.strip_prefix(OUTCOME)) + .expect("the transaction printed its outcome") + .to_owned() + } +} + +impl Drop for Transaction { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +/// Runs `transactions` at once, with `other` amid them, and returns how each of `transactions` +/// ended, in their order, and how `other` ended. +fn race(mut transactions: Vec, mut other: Transaction) -> (Vec, String) { + let middle = transactions.len() / 2; + for (index, transaction) in transactions.iter_mut().enumerate() { + if index == middle { + other.run(); + } + transaction.run(); + } + let outcomes = transactions.into_iter().map(Transaction::outcome).collect(); + (outcomes, other.outcome()) +} + +/// Returns the identities of the leases that `outcomes` report as acquired, sorted, and separated +/// by spaces. +fn acquired(outcomes: &[String]) -> String { + let mut ids: Vec<&str> = outcomes + .iter() + .filter_map(|outcome| outcome.strip_prefix("acquired ")) + .collect(); + ids.sort_unstable(); + ids.join(" ") +} + +/// A transaction in a process of its own, which [`Transaction::start`] runs: once a line arrives on +/// its standard input, it runs the transaction that [`ROLE_ACTION`] names, of the store generation +/// [`ROLE_GENERATION`] of the store in [`ROLE_STORE`], for the sandbox in [`ROLE_SANDBOX`], and +/// prints how it ended: `acquired ID`, `released ID` or `none`, `tombstoned`, `retired` or +/// `draining ID...`, or the code of its error. +#[test] +#[ignore = "a transaction that the lease tests run in a process of its own"] +fn transaction_role() { + let (Some(dir), Ok(action), Ok(generation)) = ( + env::var_os(ROLE_STORE), + env::var(ROLE_ACTION), + env::var(ROLE_GENERATION), + ) else { + return; + }; + let store = Store::open(dir).unwrap(); + let generation = StoreGeneration::new(generation.parse().unwrap()); + let sandbox = env::var_os(ROLE_SANDBOX).map(PathBuf::from); + // A second after the capture of the generation, on the host that captured it. + let now = store + .index() + .unwrap() + .get(generation) + .map(|indexed| after(indexed, 1)); + // The test harness has printed the test's name without ending the line. + println!("\n{READY}"); + let mut line = String::new(); + std::io::stdin().read_line(&mut line).unwrap(); + let deadline = Some(Instant::now() + Duration::from_secs(30)); + let code = |error: Error| error.code().to_string(); + let outcome = match action.as_str() { + "acquire" => store + .acquire( + generation, + sandbox.unwrap(), + &files(), + &now.unwrap(), + deadline, + ) + .map_or_else(code, |acquired| format!("acquired {}", acquired.lease.id())), + "release" => { + let mut locked = + SandboxStore::lock_existing(sandbox.unwrap(), &files(), deadline).unwrap(); + match store.release(&mut locked) { + Ok(Some(lease)) => format!("released {}", lease.id()), + Ok(None) => "none".to_owned(), + Err(error) => code(error), + } + } + "tombstone" => store + .tombstone(generation, deadline) + .map_or_else(code, |()| "tombstoned".to_owned()), + "retire" => match store.retire(generation, deadline) { + Ok(Retirement::Retired) => "retired".to_owned(), + Ok(Retirement::Draining(leases)) => format!("draining {}", ids(&leases)), + Err(error) => code(error), + }, + _ => panic!("unknown transaction {action}"), + }; + println!("{OUTCOME}{outcome}"); +} + +#[test] +fn processes_that_race_to_lease_one_sandbox_lease_it_once() { + /// The processes that race. + const PROCESSES: usize = 6; + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let mut acquisitions: Vec = (0..PROCESSES) + .map(|_| Transaction::start(store.dir(), "acquire", generation, Some(&sandbox))) + .collect(); + for acquisition in &mut acquisitions { + acquisition.run(); + } + let outcomes: Vec = acquisitions.into_iter().map(Transaction::outcome).collect(); + let leases = store.leases(generation).unwrap(); + assert_eq!(leases.len(), 1, "{outcomes:?}"); + assert_eq!(acquired(&outcomes), leases[0].id().to_string()); + assert_eq!( + outcomes + .iter() + .filter(|outcome| *outcome == "sandbox-leased") + .count(), + PROCESSES - 1, + "{outcomes:?}" + ); + assert_eq!(binding(&sandbox), Some(leases[0].clone())); +} + +#[test] +fn a_tombstone_that_races_acquisitions_in_other_processes_keeps_the_leases_before_it() { + /// The clones whose acquisitions race the tombstone. + const CLONES: usize = 6; + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandboxes: Vec = (0..CLONES) + .map(|clone| provision(directory.path(), &format!("clone-{clone}"))) + .collect(); + let acquisitions = sandboxes + .iter() + .map(|sandbox| Transaction::start(store.dir(), "acquire", generation, Some(sandbox))) + .collect(); + let tombstone = Transaction::start(store.dir(), "tombstone", generation, None); + let (outcomes, tombstoned) = race(acquisitions, tombstone); + assert_eq!(tombstoned, "tombstoned"); + // Each acquisition leased the generation before the tombstone, or found it quarantined after. + for (sandbox, outcome) in sandboxes.iter().zip(&outcomes) { + match outcome.strip_prefix("acquired ") { + Some(id) => assert_eq!(binding(sandbox).unwrap().id().to_string(), id), + None => { + assert_eq!(outcome, "template-tombstoned"); + assert_eq!(binding(sandbox), None); + let error = refusal(&store, generation, sandbox, &after(&published, 1)); + assert_eq!(error.code(), ErrorCode::TemplateTombstoned, "{error}"); + } + } + } + // The leases granted before it keep working. + assert_eq!(ids(&store.leases(generation).unwrap()), acquired(&outcomes)); + assert_eq!(store.tombstones().unwrap(), [generation]); +} + +#[test] +fn a_retirement_that_races_acquisitions_and_releases_in_other_processes_waits_for_its_leases() { + /// The clones whose acquisitions and releases race the retirements. + const CLONES: usize = 6; + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandboxes: Vec = (0..CLONES) + .map(|clone| provision(directory.path(), &format!("clone-{clone}"))) + .collect(); + let start = |action: &str, sandbox: Option<&Path>| { + Transaction::start(store.dir(), action, generation, sandbox) + }; + let acquisitions = sandboxes + .iter() + .map(|sandbox| start("acquire", Some(sandbox))) + .collect(); + let (outcomes, retired) = race(acquisitions, start("retire", None)); + // The retirement drains the leases of the acquisitions before it, and those after it find + // the generation quarantined, or gone if none leased it before. + let granted = acquired(&outcomes); + if granted.is_empty() { + assert_eq!(retired, "retired"); + assert!( + outcomes.iter().all(|outcome| outcome == "not-found"), + "{outcomes:?}" + ); + } else { + assert_eq!(retired, format!("draining {granted}")); + assert!( + outcomes.iter().all(|outcome| outcome.starts_with("acquired ") + || outcome == "template-tombstoned"), + "{outcomes:?}" + ); + assert!(store.snapshot_dir(&published).is_ok()); + } + assert_eq!(ids(&store.leases(generation).unwrap()), granted); + // The clones release their leases while another retirement runs, which deletes the + // generation only once none remains. + let releases = sandboxes + .iter() + .map(|sandbox| start("release", Some(sandbox))) + .collect(); + let (released, retired) = race(releases, start("retire", None)); + for (outcome, acquisition) in released.iter().zip(&outcomes) { + match acquisition.strip_prefix("acquired ") { + Some(id) => assert_eq!(outcome, &format!("released {id}")), + None => assert_eq!(outcome, "none"), + } + } + assert!( + retired == "retired" || retired.starts_with("draining "), + "{retired}" + ); + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); + assert!(!store.payload_dir(generation).exists()); + assert_eq!(store.index().unwrap().get(generation), None); + assert!(store.leases(generation).unwrap().is_empty()); + assert!(store.tombstones().unwrap().is_empty()); +} diff --git a/nvx-rs/src/host/template/store/tests.rs b/nvx-rs/src/host/template/store/tests.rs index e85c0ead2..d37dc2ae8 100644 --- a/nvx-rs/src/host/template/store/tests.rs +++ b/nvx-rs/src/host/template/store/tests.rs @@ -16,26 +16,26 @@ use crate::host::template::test_support::{ManifestSpec, class}; type Change = fn(&mut Value); /// Creates a store in a fresh temporary directory, and returns the directory and the store. -fn store() -> (tempfile::TempDir, Store) { +pub(super) fn store() -> (tempfile::TempDir, Store) { let directory = tempfile::tempdir().unwrap(); let store = Store::create(directory.path().join("templates")).unwrap(); (directory, store) } /// Returns the storage generation whose bytes are all `byte`, which must not be 0. -fn storage(byte: u8) -> StorageGeneration { +pub(super) fn storage(byte: u8) -> StorageGeneration { StorageGeneration::from_bytes([byte; ID_LEN]).unwrap() } /// Creates the directory `path` private to the current user, as the store creates its own. -fn private_dir(path: &Path) { +pub(super) fn private_dir(path: &Path) { assert_eq!(platform::create_owner_only_dir(path).unwrap(), None); } /// Publishes a capture of `class` with the storage generation `storage`, whose manifest `spec` /// encodes, as a publication does, under the store's lock: its snapshot at the place of the next /// store generation, its key beside it, and then the index, which lists it. -fn publish_with( +pub(super) fn publish_with( store: &Store, class: &CaptureClass, spec: &ManifestSpec, @@ -69,13 +69,17 @@ fn publish_with( } /// Publishes a capture of the class of [`ManifestSpec::template`], as [`publish_with`] does. -fn publish(store: &Store, spec: &ManifestSpec, storage: StorageGeneration) -> Generation { +pub(super) fn publish( + store: &Store, + spec: &ManifestSpec, + storage: StorageGeneration, +) -> Generation { publish_with(store, &class(), spec, storage) } /// Removes the generation of store generation `generation` from the index of `store`, under its /// lock, as a retirement does. -fn remove(store: &Store, generation: StoreGeneration) { +pub(super) fn remove(store: &Store, generation: StoreGeneration) { let _lock = take_lock(store.dir(), None).unwrap(); let mut index = store.index().unwrap(); index @@ -102,7 +106,7 @@ fn sorted_entries(dir: &Path) -> Vec { } /// Returns the message of the cause of `error`. -fn cause(error: &Error) -> String { +pub(super) fn cause(error: &Error) -> String { std::error::Error::source(error) .map(ToString::to_string) .unwrap_or_default() @@ -110,7 +114,7 @@ fn cause(error: &Error) -> String { /// Links `link` to the directory `target`: a symbolic link on Unix, and a junction on Windows, /// which any user may create. -fn link_dir(target: &Path, link: &Path) { +pub(super) fn link_dir(target: &Path, link: &Path) { #[cfg(unix)] std::os::unix::fs::symlink(target, link).unwrap(); #[cfg(windows)] @@ -133,7 +137,7 @@ fn link_dir(target: &Path, link: &Path) { /// Links `link` to the file `target` with a symbolic link, and returns whether it could: Windows /// lets only Developer Mode or a privilege create one. -fn link_file(target: &Path, link: &Path) -> bool { +pub(super) fn link_file(target: &Path, link: &Path) -> bool { #[cfg(unix)] let linked = std::os::unix::fs::symlink(target, link); #[cfg(windows)] @@ -145,7 +149,7 @@ fn link_file(target: &Path, link: &Path) -> bool { } /// Removes the link `link` to a directory, which leaves its target as it is. -fn unlink_dir(link: &Path) { +pub(super) fn unlink_dir(link: &Path) { #[cfg(unix)] fs::remove_file(link).unwrap(); #[cfg(windows)] diff --git a/nvx-rs/src/lib.rs b/nvx-rs/src/lib.rs index 0d7ce65c4..00c51adf6 100644 --- a/nvx-rs/src/lib.rs +++ b/nvx-rs/src/lib.rs @@ -34,7 +34,7 @@ //! network and filesystem policies, the host's NVXS connection, control sessions, process and //! platform primitives, the supervision of VM instances: starting, reattaching, executing //! workloads, and stopping, the store of a sandbox's runtime records and launch journal, and the -//! manifests, compatibility keys, and store of templates. +//! manifests, compatibility keys, store, and leases of templates. //! - `testing` (feature `testing`): a fake OpenVMM that serves the control console without //! running a VM. //! - `guest` (feature `guest`, Linux): the programs that run in NVX guests, such as From 4a9030583b3dacb536801ae32353527191e0a685 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 15:15:11 -0700 Subject: [PATCH 2/8] nvx-rs: check tombstones, and make removed bindings and stores durable Address the review of the leases of the template store: - Tombstones are read and checked wherever the store lists them, so that a tombstone of another store or of another format, or a link, fails a listing and a selection, as it fails an acquisition, rather than quarantining a generation. A retirement removes a tombstone only once it has checked it. - A retirement makes the absence of a clone binding durable in the directory that holds the sandbox's entry, besides the sandbox's own directory: a sandbox that was removed and provisioned again at its path is a new directory, whose entries never held the binding. - The first use of a store's directory of leases or of tombstones in a process also syncs the store's parent, so that no binding names a record of a store whose own entry a crash of the host could lose. - On Windows, a record that a transaction in another process removes as this one opens or removes it is gone, rather than an error: a file system without POSIX deletion refuses every open of a removed file while other handles, such as a scanner's, hold it. A retirement that raced releases failed so, rarely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/template/files.rs | 20 +- nvx-rs/src/host/template/store.rs | 9 +- nvx-rs/src/host/template/store/leases.rs | 92 +++++-- .../src/host/template/store/leases/tests.rs | 251 +++++++++++++++--- 4 files changed, 307 insertions(+), 65 deletions(-) diff --git a/nvx-rs/src/host/template/files.rs b/nvx-rs/src/host/template/files.rs index dd7ac8c9a..3cc998eb0 100644 --- a/nvx-rs/src/host/template/files.rs +++ b/nvx-rs/src/host/template/files.rs @@ -5,13 +5,31 @@ use std::fs::{File, OpenOptions}; use std::io::{self, Read}; use std::path::Path; -use crate::host::filesystem; +use crate::host::{filesystem, platform}; /// Returns the error of a file that is a link, or not a regular file. fn not_regular() -> io::Error { io::Error::other("it is a link, or not a regular file") } +/// Returns whether `error`, with which an open or a removal of the file at `path` failed, means +/// that the file was removed, by a transaction of another process, say: on Windows, a file system +/// without POSIX deletion keeps a removed file under its name while other handles hold it, such as +/// a scanner's, and refuses every open of it until they close, also one without access rights, +/// which opens a file whose access it refuses otherwise. So an open that Windows refused met a +/// removed file if one without access rights is refused too, or finds no file any more. No open +/// reaches such a file again, so it is gone for its readers, and for any other removal of it. +pub(super) fn was_removed(path: &Path, error: &io::Error) -> bool { + cfg!(windows) + && error.kind() == io::ErrorKind::PermissionDenied + && platform::file_identity(path).is_err_and(|error| { + matches!( + error.kind(), + io::ErrorKind::PermissionDenied | io::ErrorKind::NotFound + ) + }) +} + /// Opens the file at `path` for reading, without following a link, or on Windows any other /// reparse point, if it is a regular file. /// diff --git a/nvx-rs/src/host/template/store.rs b/nvx-rs/src/host/template/store.rs index 0e5cfd958..286d95f58 100644 --- a/nvx-rs/src/host/template/store.rs +++ b/nvx-rs/src/host/template/store.rs @@ -17,7 +17,7 @@ use std::time::{Duration, Instant, SystemTime}; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; -use super::files::{open_regular, read_at_most}; +use super::files::{open_regular, read_at_most, was_removed}; use super::key::{ClassIdentity, KeyIdentity, TemplateKey}; use super::manifest::{HostClock, TimeContract}; use crate::activation::StoreGeneration; @@ -813,7 +813,8 @@ fn take_lock(dir: &Path, deadline: Option) -> Result { } /// Reads the private regular file at `path`, the store's `description`, of at most `limit` bytes, -/// or returns `None` if it does not exist. +/// or returns `None` if it does not exist, or was removed as this opened it, as [`was_removed`] +/// tells, which a transaction in another process may do. /// /// The file is opened without following a link, or on Windows any other reparse point, at its /// own name, and refused unless it is a regular file, before anything is read from it. A file @@ -824,7 +825,9 @@ fn read_private(path: &Path, description: &str, limit: u64) -> Result file, - Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) if error.kind() == io::ErrorKind::NotFound || was_removed(path, &error) => { + return Ok(None); + } Err(error) => return Err(failed()(error)), }; if let Some(problem) = platform::private_file_problem(&file).map_err(failed())? { diff --git a/nvx-rs/src/host/template/store/leases.rs b/nvx-rs/src/host/template/store/leases.rs index 3f5a10fe2..3ba1eb348 100644 --- a/nvx-rs/src/host/template/store/leases.rs +++ b/nvx-rs/src/host/template/store/leases.rs @@ -26,6 +26,7 @@ use crate::host::platform::{self, parse_hex_id}; use crate::host::store::files::{remove_if_present, sync_dir}; use crate::host::store::{CLONE_BINDING_NAME, CallerFiles, LockedStore, SandboxStore}; use crate::host::template::eligibility::HostTime; +use crate::host::template::files::was_removed; use crate::host::template::key::KeyIdentity; /// Name of the directory of the records of the leases that a store granted. @@ -39,9 +40,9 @@ type SyncDir<'a> = &'a dyn Fn(&Path) -> Result<()>; /// The directories of leases and tombstones whose entries in their stores this process made /// durable, by the directory and the identity of the store, and the name of the directory. A store -/// creates each once, and never removes it, so one sync of the store's directory, once a process -/// sees one, keeps its entry durable, even if the process that created it did not finish its own -/// sync. +/// creates each once, and never removes it, so one sync of the store's directory, and of its +/// parent, once a process sees one, keeps its entry, and the store's, durable, even if the +/// process that created them did not finish its own syncs. static DURABLE_RECORD_DIRS: Mutex> = Mutex::new(Vec::new()); /// An entry of [`DURABLE_RECORD_DIRS`]: the directory and the identity of a store, and the name @@ -427,7 +428,7 @@ impl Store { let recorded = self.check_records_dir(LEASES_NAME, LEASES_WHAT)?; unbind(locked.dir(), sync)?; if recorded { - remove_if_present(&self.lease_path(lease.id))?; + remove_lease_record(&self.lease_path(lease.id))?; } Ok(Some(lease)) } @@ -470,12 +471,22 @@ impl Store { /// Returns the store generations that tombstones quarantine, in order. /// - /// Fails with [`ErrorCode::ForeignFiles`] if the directory of the tombstones holds entries - /// that are not the store's. + /// Each tombstone is read and checked, as an acquisition checks it: a tombstone that cannot be + /// read, or is malformed, fails with [`ErrorKind::Io`], and one of another format with + /// [`ErrorCode::FormatUnsupported`], rather than quarantining its generation. Fails with + /// [`ErrorCode::ForeignFiles`] if the directory of the tombstones holds entries that are not + /// the store's. pub fn tombstones(&self) -> Result> { - let mut tombstoned = self + let mut tombstoned = Vec::new(); + for generation in self .records(TOMBSTONES_NAME, TOMBSTONES_WHAT, tombstone_name)? - .records; + .records + { + // A retirement may remove the tombstone after the listing. + if self.read_tombstone(generation)? { + tombstoned.push(generation); + } + } tombstoned.sort(); Ok(tombstoned) } @@ -488,8 +499,9 @@ impl Store { /// leases whose sandboxes no longer hold them: a sandbox that is gone, that holds no clone /// binding, or one of another lease, as an interrupted release, or a removal of the sandbox, /// leaves it. It makes the binding's absence durable first, by syncing the sandbox's - /// directory, or the nearest of its ancestors that remains, through the links that the - /// sandbox's path names, as a release that a crash or a failed sync interrupted may not have. + /// directory, if it is one, and the directory that holds the sandbox's entry, the nearest of + /// its ancestors that remains, through the links that the sandbox's path names, as a release + /// that a crash or a failed sync interrupted, or a removal of the sandbox, may not have. /// If any lease remains, the generation keeps its /// payload and its entry in the index, and the retirement returns [`Retirement::Draining`] /// with the leases, whose clones [release](Self::release) them; a later retirement deletes it. @@ -554,8 +566,10 @@ impl Store { } else { // A release may have removed the binding without making that durable yet, so the // binding's absence must be durable before its record goes. - sync(binding_dir(&lease.sandbox)?)?; - remove_if_present(&self.lease_path(id))?; + for dir in binding_dirs(&lease.sandbox)? { + sync(dir)?; + } + remove_lease_record(&self.lease_path(id))?; } } if !live.is_empty() { @@ -598,8 +612,11 @@ impl Store { /// Creates the store's directory `name`, which `what` names, private to the current user, /// unless it exists, under the store's lock, and makes its entry in the store durable with - /// `sync`: it syncs the store's directory once it creates the directory, and once it finds - /// one that this process has not synced yet, whose creation may not have finished its sync. + /// `sync`, and the store's own entry in its parent: it syncs the store's directory, and then + /// the store's parent, once it creates the directory, and once it finds one that this process + /// has not synced yet, whose creation, or the store's, may not have finished its sync. A + /// binding then never names a record of a store whose own entry a crash of the host could + /// lose. fn create_records_dir(&self, name: &'static str, what: &str, sync: SyncDir<'_>) -> Result<()> { let key = (self.dir.clone(), self.id, name); if !self.check_records_dir(name, what)? { @@ -613,6 +630,9 @@ impl Store { return Ok(()); } sync(&self.dir)?; + if let Some(parent) = self.dir.parent() { + sync(parent)?; + } let mut durable = durable_record_dirs(); if !durable.contains(&key) { durable.push(key); @@ -701,6 +721,13 @@ impl Store { if !self.check_records_dir(TOMBSTONES_NAME, TOMBSTONES_WHAT)? { return Ok(false); } + self.read_tombstone(generation) + } + + /// Reads the tombstone of the generation `generation`, in the directory of the tombstones, + /// which exists, and returns whether it exists: a tombstone that cannot be read, is malformed, + /// or is of another format fails. + fn read_tombstone(&self, generation: StoreGeneration) -> Result { let path = self.tombstone_path(generation); let Some(record) = read_record::(&path, TOMBSTONE_DESCRIPTION, MAX_LEASE_RECORD_LEN)? @@ -739,9 +766,10 @@ impl Store { } /// Removes the tombstone of the generation `generation`, if it has one, under the store's - /// lock. + /// lock, once it has read and checked it: a tombstone that is not the store's tombstone of + /// that generation, or that is of another format, fails, and is kept. fn remove_tombstone(&self, generation: StoreGeneration) -> Result<()> { - if self.check_records_dir(TOMBSTONES_NAME, TOMBSTONES_WHAT)? { + if self.is_tombstoned(generation)? { remove_if_present(&self.tombstone_path(generation))?; } Ok(()) @@ -792,16 +820,19 @@ fn is_bound(lease: &Lease) -> Result { .is_some_and(|binding| binding.id == lease.id && binding.store == lease.store)) } -/// Returns the directory whose entries changed last as the clone binding of the sandbox in -/// `sandbox` went: the sandbox's own directory, if it remains one, and otherwise the nearest of -/// its ancestors that is a directory, following the links that its path names, as the removal of -/// the sandbox followed them, where the sandbox's entry went. +/// Returns the directories whose entries changed as the clone binding of the sandbox in `sandbox` +/// went: the sandbox's own directory, if it is one, and the directory that holds the sandbox's +/// entry, the nearest of its ancestors that is a directory, following the links that its path +/// names, as the removal of the sandbox followed them. A sandbox that was removed, and provisioned +/// again at its path, is a new directory, whose entries never held the binding: the binding went +/// with the directory that the entry of the sandbox named before. /// /// Fails for an ancestor that cannot be examined, or that is a link to nothing, whose target, the /// directory that held the sandbox, cannot be known. -fn binding_dir(sandbox: &Path) -> Result<&Path> { +fn binding_dirs(sandbox: &Path) -> Result> { + let mut dirs = Vec::with_capacity(2); if fs::symlink_metadata(sandbox).is_ok_and(|metadata| metadata.is_dir()) { - return Ok(sandbox); + dirs.push(sandbox); } for ancestor in sandbox.ancestors().skip(1) { let unknown = |error: io::Error| { @@ -813,7 +844,10 @@ fn binding_dir(sandbox: &Path) -> Result<&Path> { .with_source(error) }; match fs::metadata(ancestor) { - Ok(metadata) if metadata.is_dir() => return Ok(ancestor), + Ok(metadata) if metadata.is_dir() => { + dirs.push(ancestor); + return Ok(dirs); + } // A file replaced the directory, in the entries of its parent. Ok(_) => {} Err(error) if error.kind() == io::ErrorKind::NotFound => { @@ -844,6 +878,18 @@ fn unbind(dir: &Path, sync: SyncDir<'_>) -> Result<()> { sync(dir) } +/// Removes the record of a lease at `path`, if it exists, which a release and a retirement, in +/// other processes, may both remove at once: a record that another removal removed as this tried, +/// as [`was_removed`] tells, is gone already. +fn remove_lease_record(path: &Path) -> Result<()> { + match fs::remove_file(path) { + Err(error) if error.kind() != io::ErrorKind::NotFound && !was_removed(path, &error) => Err( + store_error(format!("cannot remove {}", path.display()))(error), + ), + _ => Ok(()), + } +} + /// Returns `error`, which ended an acquisition of the lease `id` whose binding the sandbox in /// `dir` may hold still, with the error's code and source, saying so. fn held_still(error: Error, dir: &Path, id: LeaseId) -> Error { diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 04550b17a..d2224992a 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -14,7 +14,7 @@ use crate::host::store::files::write_private; use crate::host::store::{FORMAT, RUNTIME_NAME, Recovery, RuntimeIdentity}; use crate::host::template::eligibility::MAX_DOWNTIME_NS; use crate::host::template::store::tests::{ - link_dir, publish, publish_with, remove, storage, store as new_store, unlink_dir, + link_dir, link_file, publish, publish_with, remove, storage, store as new_store, unlink_dir, }; use crate::host::template::test_support::{ManifestSpec, artifact, network_class}; use crate::protocol::CAPABILITY_LEN; @@ -571,8 +571,10 @@ fn acquisitions_make_their_records_durable_before_their_bindings_and_both_before *synced.borrow(), [ // The first makes the entry of the directory of the leases durable, which no - // acquisition of this process did before, ahead of any record in it. + // acquisition of this process did before, ahead of any record in it, and then the + // store's own entry. (store.dir().to_owned(), 0, 0), + (store.dir().parent().unwrap().to_owned(), 0, 0), // Each makes its record durable before its binding, and its binding before it // returns. (leases_dir.clone(), 1, 0), @@ -698,8 +700,10 @@ fn a_retirement_makes_each_step_durable_before_the_next() { assert_eq!( *synced.borrow(), [ - // The new directory of the tombstones, before the tombstone in it. + // The new directory of the tombstones, and the store's own entry, before the + // tombstone in it. (store.dir().to_owned(), false, true, true), + (store.dir().parent().unwrap().to_owned(), false, true, true), // The tombstone, before anything goes. (tombstones.clone(), true, true, true), // The payload's removal, before the index changes. @@ -717,8 +721,10 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec let first = publish(&store, &ManifestSpec::template(), storage(1)); let second = publish(&store, &ManifestSpec::template(), storage(2)); let third = publish(&store, &ManifestSpec::template(), storage(3)); + let fourth = publish(&store, &ManifestSpec::template(), storage(4)); let released = provision(directory.path(), "released"); let removed = provision(directory.path(), "removed"); + let reprovisioned = provision(directory.path(), "reprovisioned"); // A sandbox whose parent the path names through a link. let sandboxes = directory.path().join("sandboxes"); fs::create_dir(&sandboxes).unwrap(); @@ -728,13 +734,15 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec let released_lease = leased(&store, &first, &released); let removed_lease = leased(&store, &second, &removed); let linked_lease = leased(&store, &third, &removed_through_link); + let reprovisioned_lease = leased(&store, &fourth, &reprovisioned); // A release that removed its binding, but did not make that durable, and removals of - // sandboxes. + // sandboxes, one of which is provisioned again at its path. fs::remove_file(released.join(CLONE_BINDING_NAME)).unwrap(); - for sandbox in [&removed, &removed_through_link] { + for sandbox in [&removed, &removed_through_link, &reprovisioned] { let locked = SandboxStore::lock_existing(sandbox, &files(), None).unwrap(); assert!(locked.remove().unwrap().is_none()); } + assert_eq!(provision(directory.path(), "reprovisioned"), reprovisioned); let record = |lease: &Lease| { store .dir() @@ -756,12 +764,23 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec assert_eq!(error.message(), "the disk failed"); assert!(record(&released_lease).exists()); assert!(store.snapshot_dir(&first).is_ok()); - // A record goes once the binding's absence is durable, in the sandbox's directory, or in the - // nearest of its ancestors that remains, through the links that its path names. - for (generation, lease, dir) in [ - (&first, &released_lease, released.as_path()), - (&second, &removed_lease, directory.path()), - (&third, &linked_lease, linked.as_path()), + // A record goes once the binding's absence is durable: in the sandbox's directory, if it is + // one, and in the directory that holds its entry, the nearest of its ancestors that remains, + // through the links that its path names. The directory of a sandbox that was provisioned + // again never held the binding, which went with the directory that its entry named before. + for (generation, lease, dirs) in [ + ( + &first, + &released_lease, + vec![released.as_path(), directory.path()], + ), + (&second, &removed_lease, vec![directory.path()]), + (&third, &linked_lease, vec![linked.as_path()]), + ( + &fourth, + &reprovisioned_lease, + vec![reprovisioned.as_path(), directory.path()], + ), ] { let path = record(lease); let synced = RefCell::new(Vec::new()); @@ -777,31 +796,29 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec .unwrap(), Retirement::Retired ); - assert_eq!( - *synced.borrow(), - [ - (store.dir().join(TOMBSTONES_NAME), true), - (dir.to_owned(), true), - (store.dir().join(GENERATIONS_NAME), false), - (store.dir().to_owned(), false), - ] - ); + let mut expected = vec![(store.dir().join(TOMBSTONES_NAME), true)]; + expected.extend(dirs.into_iter().map(|dir| (dir.to_owned(), true))); + expected.extend([ + (store.dir().join(GENERATIONS_NAME), false), + (store.dir().to_owned(), false), + ]); + assert_eq!(*synced.borrow(), expected); } // A link to nothing hides the directory that held a sandbox, so the record stays. - let fourth = publish(&store, &ManifestSpec::template(), storage(4)); + let fifth = publish(&store, &ManifestSpec::template(), storage(5)); let dangling = provision(&linked, "dangling"); - let dangling_lease = leased(&store, &fourth, &dangling); + let dangling_lease = leased(&store, &fifth, &dangling); let locked = SandboxStore::lock_existing(&dangling, &files(), None).unwrap(); assert!(locked.remove().unwrap().is_none()); fs::remove_dir(&sandboxes).unwrap(); - let error = store.retire(fourth.generation(), None).unwrap_err(); + let error = store.retire(fifth.generation(), None).unwrap_err(); assert_eq!(error.kind(), ErrorKind::Io, "{error}"); assert!( error.message().contains("cannot find the directory"), "{error}" ); assert!(record(&dangling_lease).exists()); - assert!(store.snapshot_dir(&fourth).is_ok()); + assert!(store.snapshot_dir(&fifth).is_ok()); } #[test] @@ -833,6 +850,54 @@ fn a_retirement_that_finds_its_generation_unindexed_makes_the_index_durable_firs assert!(!tombstone.exists()); } +#[test] +fn a_retirement_keeps_a_tombstone_that_is_not_the_stores() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + // A retirement that a crash interrupted once it had written the index leaves the tombstone, + // which retiring the generation again removes only once it has checked it. + store.tombstone(generation, None).unwrap(); + fs::remove_dir_all(store.payload_dir(generation)).unwrap(); + remove(&store, generation); + let tombstone = store + .dir() + .join(TOMBSTONES_NAME) + .join(format!("{generation}.json")); + let valid = json_of(&tombstone); + let foreign: [(Change, ErrorCode); 2] = [ + ( + |value| value["store"] = json!("0".repeat(32)), + ErrorCode::Io, + ), + ( + |value| value["format"] = json!(2), + ErrorCode::FormatUnsupported, + ), + ]; + for (change, code) in foreign { + let mut value = valid.clone(); + change(&mut value); + fs::write(&tombstone, value.to_string()).unwrap(); + let error = store.retire(generation, None).unwrap_err(); + assert_eq!(error.code(), code, "{error}"); + assert_eq!(json_of(&tombstone), value); + } + // Nor does it remove a link in place of the tombstone. + fs::write(&tombstone, valid.to_string()).unwrap(); + let moved = directory.path().join("tombstone.json"); + fs::rename(&tombstone, &moved).unwrap(); + if link_file(&moved, &tombstone) { + let error = store.retire(generation, None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io, "{error}"); + assert!(fs::symlink_metadata(&tombstone).unwrap().is_symlink()); + fs::remove_file(&tombstone).unwrap(); + } + fs::rename(&moved, &tombstone).unwrap(); + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); + assert!(!tombstone.exists()); +} + #[test] fn directories_are_synced_where_the_platform_can() { let directory = tempfile::tempdir().unwrap(); @@ -979,23 +1044,37 @@ fn malformed_records_and_foreign_entries_fail_clearly() { } assert!(store.tombstones().unwrap().is_empty()); fs::remove_file(¬es).unwrap(); - // A tombstone of another store, and entries that are not tombstones. + // A tombstone of another store, or of another format, which neither an acquisition nor a + // listing or a selection takes for a quarantine, and entries that are not tombstones. store.tombstone(generation, None).unwrap(); let tombstones = store.dir().join(TOMBSTONES_NAME); - let tombstone = json!({ - "format": 1, - "store": "0".repeat(32), - "generation": generation.get().get(), - }); - fs::write( - tombstones.join(format!("{generation}.json")), - tombstone.to_string(), - ) - .unwrap(); + let tombstone = tombstones.join(format!("{generation}.json")); + let valid_tombstone = json_of(&tombstone); let other = provision(directory.path(), "other"); - let error = refusal(&store, generation, &other, &after(&published, 1)); - assert_eq!(error.code(), ErrorCode::Io, "{error}"); - assert!(error.message().contains("is malformed"), "{error}"); + let foreign_tombstones: [(Change, ErrorCode); 2] = [ + ( + |value| value["store"] = json!("0".repeat(32)), + ErrorCode::Io, + ), + ( + |value| value["format"] = json!(2), + ErrorCode::FormatUnsupported, + ), + ]; + for (change, code) in foreign_tombstones { + let mut value = valid_tombstone.clone(); + change(&mut value); + fs::write(&tombstone, value.to_string()).unwrap(); + for error in [ + refusal(&store, generation, &other, &after(&published, 1)), + store.tombstones().unwrap_err(), + store + .select(published.key(), &after(&published, 1)) + .unwrap_err(), + ] { + assert_eq!(error.code(), code, "{error}"); + } + } fs::write(tombstones.join("+1.json"), b"").unwrap(); for error in [ store.tombstones().unwrap_err(), @@ -1336,3 +1415,99 @@ fn a_retirement_that_races_acquisitions_and_releases_in_other_processes_waits_fo assert!(store.leases(generation).unwrap().is_empty()); assert!(store.tombstones().unwrap().is_empty()); } + +/// Tests of the records that a Windows file system without POSIX deletion keeps under their names +/// while other handles hold them, after a transaction in another process removed them. +#[cfg(windows)] +#[allow(unsafe_code)] +mod windows { + use std::os::windows::fs::OpenOptionsExt; + use std::os::windows::io::AsRawHandle; + + use windows_sys::Win32::Foundation::HANDLE; + use windows_sys::Win32::Storage::FileSystem::{ + DELETE, FILE_DISPOSITION_INFO, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + FileDispositionInfo, SetFileInformationByHandle, + }; + + use super::*; + + /// Deletes the file `path` as a file system without POSIX deletion deletes a file that other + /// handles hold: its name stays, and opening it fails, until the last handle closes, of which + /// the returned one is the last unless others hold the file. + fn delete_pending(path: &Path) -> fs::File { + let file = fs::OpenOptions::new() + .access_mode(DELETE) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE) + .open(path) + .unwrap(); + let disposition = FILE_DISPOSITION_INFO { DeleteFile: true }; + // SAFETY: the handle is open, and the information is a FILE_DISPOSITION_INFO of its size. + let set = unsafe { + SetFileInformationByHandle( + file.as_raw_handle() as HANDLE, + FileDispositionInfo, + (&raw const disposition).cast(), + size_of::() as u32, + ) + }; + assert_ne!(set, 0, "{}", io::Error::last_os_error()); + file + } + + #[test] + fn records_that_another_transaction_removes_are_gone_for_their_readers() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + // A release in another process removed the binding, and is removing the record, which a + // scanner, say, holds open. + let held_binding = delete_pending(&sandbox.join(CLONE_BINDING_NAME)); + let held_record = delete_pending(&record); + assert_eq!(binding(&sandbox), None); + assert!(store.leases(generation).unwrap().is_empty()); + // A tombstone that a retirement removes, likewise. + let other = publish(&store, &ManifestSpec::template(), storage(2)); + store.tombstone(other.generation(), None).unwrap(); + let tombstone = store + .dir() + .join(TOMBSTONES_NAME) + .join(format!("{}.json", other.generation())); + let held_tombstone = delete_pending(&tombstone); + assert!(store.tombstones().unwrap().is_empty()); + assert_eq!( + store.select(other.key(), &after(&other, 1)).unwrap(), + Some(other.clone()) + ); + // A retirement finds no lease of the generation. + assert_eq!(store.retire(generation, None).unwrap(), Retirement::Retired); + drop((held_binding, held_record, held_tombstone)); + assert!(!record.exists()); + assert!(!tombstone.exists()); + } + + #[test] + fn a_release_whose_record_another_transaction_removes_ends_its_lease() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + // A retirement in another process is removing the record, as the release does. + let held_record = delete_pending(&record); + assert_eq!(release(&store, &sandbox).unwrap(), Some(lease)); + assert_eq!(binding(&sandbox), None); + drop(held_record); + assert!(store.leases(generation).unwrap().is_empty()); + } +} From 9bbe44d675fc184f16011d9cc1b63c50b98b161e Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 15:35:24 -0700 Subject: [PATCH 3/8] nvx-rs: tell a pending deletion from a refused access On Windows, a refused read or removal of a template store's record counted as a removal by another transaction if an open without access rights was refused too. Win32 opens ask for SYNCHRONIZE and FILE_READ_ATTRIBUTES even then, so a file whose access control list grants neither was refused both ways, and taken for a removed one: a retirement skipped the record of a live lease, or the binding that names it, and deleted the generation, and a tombstone that refused access quarantined nothing. platform::is_deleted now opens the file with NtOpenFile, relative to its directory, whose status tells STATUS_DELETE_PENDING from STATUS_ACCESS_DENIED, which Win32 reports alike, and a refused file counts as removed only if its deletion is pending, or it is gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/Cargo.toml | 2 + nvx-rs/src/host/platform/linux.rs | 6 ++ nvx-rs/src/host/platform/other.rs | 6 ++ nvx-rs/src/host/platform/windows.rs | 72 ++++++++++++++ nvx-rs/src/host/template/files.rs | 18 ++-- .../src/host/template/store/leases/tests.rs | 97 ++++++++++++++++++- 6 files changed, 188 insertions(+), 13 deletions(-) diff --git a/nvx-rs/Cargo.toml b/nvx-rs/Cargo.toml index 9750c94af..959ec9dac 100644 --- a/nvx-rs/Cargo.toml +++ b/nvx-rs/Cargo.toml @@ -59,6 +59,8 @@ sha2 = { version = "0.10", optional = true } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.61", optional = true, features = [ + "Wdk_Foundation", + "Wdk_Storage_FileSystem", "Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem", diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index 384dab084..6ad0a8fa8 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -579,6 +579,12 @@ pub fn file_identity(path: &Path) -> io::Result<(u64, u64)> { Ok((metadata.dev(), metadata.ino())) } +/// Returns whether the file at `path` is deleted: whether no file has that name, which a removal +/// takes at once. +pub(crate) fn is_deleted(path: &Path) -> bool { + fs::symlink_metadata(path).is_err_and(|error| error.kind() == io::ErrorKind::NotFound) +} + /// Returns the device and inode of the file that `file` holds open. pub(crate) fn opened_file_identity(file: &File) -> io::Result<(u64, u64)> { let metadata = file.metadata()?; diff --git a/nvx-rs/src/host/platform/other.rs b/nvx-rs/src/host/platform/other.rs index ddc2e9fb8..08afc447f 100644 --- a/nvx-rs/src/host/platform/other.rs +++ b/nvx-rs/src/host/platform/other.rs @@ -88,6 +88,12 @@ pub fn file_identity(_path: &Path) -> io::Result<(u64, u64)> { Err(unsupported()) } +/// Returns whether the file at `path` is deleted: whether no file has that name, which a removal +/// takes at once. +pub(crate) fn is_deleted(path: &Path) -> bool { + fs::symlink_metadata(path).is_err_and(|error| error.kind() == io::ErrorKind::NotFound) +} + pub(crate) fn opened_file_identity(_file: &File) -> io::Result<(u64, u64)> { Err(unsupported()) } diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index b57574c1b..e4f299a04 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -1248,6 +1248,78 @@ pub fn file_identity(path: &Path) -> io::Result<(u64, u64)> { handle_identity(file.as_raw_handle() as HANDLE) } +/// Returns whether the file at `path` is deleted: whether an open of it finds no file, or one +/// whose deletion is pending, which a file system without POSIX deletion keeps under its name, +/// refusing every open of it, until the last handle to it closes. A file that opens is not, nor is +/// one whose open fails otherwise, such as one whose access control list refuses the open. +/// +/// Win32 reports a pending deletion as a denied access, so the file is opened with the native API, +/// whose status tells the two apart, relative to its directory, which spares a native path. +pub(crate) fn is_deleted(path: &Path) -> bool { + use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; + use windows_sys::Wdk::Storage::FileSystem::{FILE_OPEN_REPARSE_POINT, NtOpenFile}; + use windows_sys::Win32::Foundation::{ + OBJ_CASE_INSENSITIVE, STATUS_DELETE_PENDING, STATUS_OBJECT_NAME_NOT_FOUND, UNICODE_STRING, + }; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + }; + use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; + + let Some(name) = path.file_name() else { + return false; + }; + let dir = match path.parent() { + Some(dir) if !dir.as_os_str().is_empty() => dir, + _ => Path::new("."), + }; + let dir = match OpenOptions::new() + .access_mode(0) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(dir) + { + Ok(dir) => dir, + // The file went with its directory. + Err(error) => return error.kind() == io::ErrorKind::NotFound, + }; + let name: Vec = name.encode_wide().collect(); + let Ok(len) = u16::try_from(mem::size_of_val(name.as_slice())) else { + return false; + }; + let name = UNICODE_STRING { + Length: len, + MaximumLength: len, + Buffer: name.as_ptr().cast_mut(), + }; + let attributes = OBJECT_ATTRIBUTES { + Length: mem::size_of::() as u32, + RootDirectory: dir.as_raw_handle() as HANDLE, + ObjectName: &name, + // As Win32 opens files. + Attributes: OBJ_CASE_INSENSITIVE, + ..OBJECT_ATTRIBUTES::default() + }; + let mut handle = ptr::null_mut(); + let mut status_block = IO_STATUS_BLOCK::default(); + // SAFETY: the directory's handle is open, the attributes and the name that they point to + // outlive the call, and the outputs are valid. + let status = unsafe { + NtOpenFile( + &mut handle, + FILE_READ_ATTRIBUTES, + &attributes, + &mut status_block, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + FILE_OPEN_REPARSE_POINT, + ) + }; + if status >= 0 { + drop(owned(handle)); + return false; + } + matches!(status, STATUS_DELETE_PENDING | STATUS_OBJECT_NAME_NOT_FOUND) +} + /// Returns the volume serial number and file index of the file that `file` holds open. pub(crate) fn opened_file_identity(file: &fs::File) -> io::Result<(u64, u64)> { handle_identity(file.as_raw_handle() as HANDLE) diff --git a/nvx-rs/src/host/template/files.rs b/nvx-rs/src/host/template/files.rs index 3cc998eb0..e8a229307 100644 --- a/nvx-rs/src/host/template/files.rs +++ b/nvx-rs/src/host/template/files.rs @@ -15,19 +15,13 @@ fn not_regular() -> io::Error { /// Returns whether `error`, with which an open or a removal of the file at `path` failed, means /// that the file was removed, by a transaction of another process, say: on Windows, a file system /// without POSIX deletion keeps a removed file under its name while other handles hold it, such as -/// a scanner's, and refuses every open of it until they close, also one without access rights, -/// which opens a file whose access it refuses otherwise. So an open that Windows refused met a -/// removed file if one without access rights is refused too, or finds no file any more. No open -/// reaches such a file again, so it is gone for its readers, and for any other removal of it. +/// a scanner's, and refuses every open of it until they close, which Win32 reports as a denied +/// access. So a denied access met a removed file if the file is deleted now, or its deletion is +/// pending, as [`platform::is_deleted`] tells, which a file whose access control list refuses the +/// access is not. No open reaches a removed file again, so it is gone for its readers, and for any +/// other removal of it. pub(super) fn was_removed(path: &Path, error: &io::Error) -> bool { - cfg!(windows) - && error.kind() == io::ErrorKind::PermissionDenied - && platform::file_identity(path).is_err_and(|error| { - matches!( - error.kind(), - io::ErrorKind::PermissionDenied | io::ErrorKind::NotFound - ) - }) + cfg!(windows) && error.kind() == io::ErrorKind::PermissionDenied && platform::is_deleted(path) } /// Opens the file at `path` for reading, without following a link, or on Windows any other diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index d2224992a..750a462c5 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -1417,7 +1417,8 @@ fn a_retirement_that_races_acquisitions_and_releases_in_other_processes_waits_fo } /// Tests of the records that a Windows file system without POSIX deletion keeps under their names -/// while other handles hold them, after a transaction in another process removed them. +/// while other handles hold them, after a transaction in another process removed them, and of the +/// records whose access control lists refuse access, which are not removed ones. #[cfg(windows)] #[allow(unsafe_code)] mod windows { @@ -1455,6 +1456,100 @@ mod windows { file } + /// The access control list of a file that allows no one anything, which its owner may still + /// change: every open of the file fails, also one without access rights. + const NO_ACCESS: &str = "D:P"; + + /// The access control list of a file that allows its owner alone, as the store's files do. + const OWNER_ONLY: &str = "D:P(A;;FA;;;OW)"; + + #[test] + fn only_a_file_that_is_gone_or_whose_deletion_is_pending_was_removed() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("record.json"); + fs::write(&path, b"{}").unwrap(); + let denied = io::Error::from(io::ErrorKind::PermissionDenied); + assert!(!was_removed(&path, &denied)); + // A file whose list refuses every open, also one without access rights, is not removed. + platform::set_access(&path, NO_ACCESS).unwrap(); + let error = platform::file_identity(&path).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(!was_removed(&path, &denied)); + platform::set_access(&path, OWNER_ONLY).unwrap(); + // One whose deletion is pending is, as is one that is gone, but only a denied access tells + // so. + let held = delete_pending(&path); + assert!(was_removed(&path, &denied)); + assert!(!was_removed(&path, &io::Error::other("the open failed"))); + drop(held); + assert!(was_removed(&path, &denied)); + // So is a file whose directory is gone. + let gone = directory.path().join("gone").join("record.json"); + assert!(was_removed(&gone, &denied)); + } + + #[test] + fn records_that_refuse_access_are_not_taken_for_removed_ones() { + let (directory, store) = new_store(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let sandbox = provision(directory.path(), "sandbox"); + let lease = leased(&store, &published, &sandbox); + let record = store + .dir() + .join(LEASES_NAME) + .join(format!("{}.json", lease.id())); + // A retirement that cannot read the record of a lease, or its clone binding, cannot tell + // whether the lease is live, and fails, keeping the generation. + for (path, description) in [ + (record, LEASE_DESCRIPTION), + (sandbox.join(CLONE_BINDING_NAME), BINDING_DESCRIPTION), + ] { + platform::set_access(&path, NO_ACCESS).unwrap(); + let error = store.retire(generation, None).unwrap_err(); + assert!( + error + .message() + .starts_with(&format!("cannot read the {description} ")), + "{error}" + ); + platform::set_access(&path, OWNER_ONLY).unwrap(); + } + assert_eq!( + store.retire(generation, None).unwrap(), + Retirement::Draining(vec![lease.clone()]) + ); + assert_eq!(binding(&sandbox), Some(lease)); + // A tombstone that refuses access quarantines its generation still. + let other = publish(&store, &ManifestSpec::template(), storage(2)); + store.tombstone(other.generation(), None).unwrap(); + let tombstone = store + .dir() + .join(TOMBSTONES_NAME) + .join(format!("{}.json", other.generation())); + platform::set_access(&tombstone, NO_ACCESS).unwrap(); + let now = after(&other, 1); + let leasing = provision(directory.path(), "leasing"); + for error in [ + store.tombstones().unwrap_err(), + store.select(other.key(), &now).unwrap_err(), + refusal(&store, other.generation(), &leasing, &now), + ] { + assert!( + error + .message() + .starts_with(&format!("cannot read the {TOMBSTONE_DESCRIPTION} ")), + "{error}" + ); + } + assert_eq!(binding(&leasing), None); + platform::set_access(&tombstone, OWNER_ONLY).unwrap(); + assert_eq!( + store.tombstones().unwrap(), + [generation, other.generation()] + ); + } + #[test] fn records_that_another_transaction_removes_are_gone_for_their_readers() { let (directory, store) = new_store(); From 1640de52fa5cf3ca160c5287b4f6a04a627a4618 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 16:00:10 -0700 Subject: [PATCH 4/8] nvx-rs: name a leased sandbox by its resolved path A lease named its sandbox by the absolute path through which it was leased. That path could run through a link, or, on Unix, through a directory that a `..` component passes through. Once that link or directory went, the path named nothing, though the sandbox and its clone binding remained, so a retirement took the lease for unbound, and could delete its generation while the clone ran. And a link that changed while the sandbox was leased could have redirected the binding to another sandbox. acquire now resolves the links of the sandbox's path before it takes the sandbox's lock, and the lease, the binding, and the lock that it returns all name the sandbox by that path. The tests that build paths through links now move a sandbox's directory and link its old path after the lease. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/filesystem.rs | 2 +- nvx-rs/src/host/template/store/leases.rs | 50 +++++-- .../src/host/template/store/leases/tests.rs | 139 +++++++++++++++--- 3 files changed, 161 insertions(+), 30 deletions(-) diff --git a/nvx-rs/src/host/filesystem.rs b/nvx-rs/src/host/filesystem.rs index daab0f329..20e8a6b0c 100644 --- a/nvx-rs/src/host/filesystem.rs +++ b/nvx-rs/src/host/filesystem.rs @@ -1972,7 +1972,7 @@ fn depth(path: &str) -> usize { /// name, among other things. A path that resolves to itself again names the same object; one that /// only the verbatim prefix keeps, such as that of a directory whose name ends in a period, would /// name another object, or none, so it is refused. -fn canonicalize(path: &Path) -> std::io::Result { +pub(crate) fn canonicalize(path: &Path) -> std::io::Result { let canonical = fs::canonicalize(path)?; let Some(stripped) = strip_verbatim(&canonical) else { return Err(std::io::Error::new( diff --git a/nvx-rs/src/host/template/store/leases.rs b/nvx-rs/src/host/template/store/leases.rs index 3ba1eb348..5618de682 100644 --- a/nvx-rs/src/host/template/store/leases.rs +++ b/nvx-rs/src/host/template/store/leases.rs @@ -22,6 +22,7 @@ use super::{ }; use crate::activation::{Identity, StoreGeneration, TemplateReference}; use crate::error::{Error, ErrorCode, ErrorKind, Result}; +use crate::host::filesystem; use crate::host::platform::{self, parse_hex_id}; use crate::host::store::files::{remove_if_present, sync_dir}; use crate::host::store::{CLONE_BINDING_NAME, CallerFiles, LockedStore, SandboxStore}; @@ -121,8 +122,9 @@ impl FromStr for LeaseId { /// /// The store keeps the record of each lease that it granted, and the sandbox keeps the same /// record as its clone binding, [`CLONE_BINDING_NAME`], beside its launch journal. A lease names -/// its sandbox by the absolute path of its directory, so a sandbox that holds a lease must not -/// move. +/// its sandbox by the path of its directory with its links resolved, so that no removal or change +/// of a link, or of a directory that a `..` component passes through, hides the sandbox from a +/// [retirement](Store::retire), and a sandbox that holds a lease must not move. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Lease { /// The lease's identity. @@ -133,8 +135,8 @@ pub struct Lease { generation: StoreGeneration, /// The identity of the leased generation's key. key: KeyIdentity, - /// The directory of the sandbox that holds the lease, as an absolute path, which is valid - /// Unicode. + /// The directory of the sandbox that holds the lease, as an absolute path with its links + /// resolved, which is valid Unicode. sandbox: PathBuf, } @@ -169,7 +171,8 @@ impl Lease { self.key } - /// Returns the directory of the sandbox that holds the lease, as an absolute path. + /// Returns the directory of the sandbox that holds the lease, as an absolute path with its + /// links resolved, which may differ from the path through which the sandbox was leased. pub fn sandbox(&self) -> &Path { &self.sandbox } @@ -189,8 +192,8 @@ impl Lease { pub struct Acquired { /// The lease. pub lease: Lease, - /// The lock of the sandbox, under which the caller launches its clone. Dropping it releases - /// the lock, and keeps the lease. + /// The lock of the sandbox, which names it by the lease's path, under which the caller launches + /// its clone. Dropping it releases the lock, and keeps the lease. pub locked: LockedStore, } @@ -241,8 +244,11 @@ impl Store { /// and no launch, and holds no lease; and it writes the lease's record in the store, and then /// the same record as the sandbox's clone binding, beside its launch journal, each durable /// before the next step, so that no binding outlasts, through a crash of the host, the record - /// that keeps its generation in the store. It returns the lease, with the sandbox's lock, - /// under which the caller launches the clone, and releases the store's lock. + /// that keeps its generation in the store. It resolves the links of the sandbox's path before + /// it takes the sandbox's lock, and the lease, the binding, and the lock that it returns all + /// name the sandbox by that path, as [`Lease::sandbox`] returns it, which no later change of a + /// link redirects to another directory. It returns the lease, with the sandbox's lock, under + /// which the caller launches the clone, and releases the store's lock. /// /// Fails, and leases nothing: /// @@ -254,8 +260,9 @@ impl Store { /// - with [`ErrorCode::SandboxRunning`] if the sandbox records a VM, or a launch, which may /// still run; /// - with [`ErrorCode::SandboxLeased`] if the sandbox holds a lease already; - /// - with [`ErrorKind::Unsupported`] if the sandbox's path is not valid Unicode, which its - /// record cannot hold; + /// - with [`ErrorKind::Unsupported`] if the sandbox's path, with its links resolved, is not + /// valid Unicode, which its record cannot hold, or, on Windows, is a network path, or one + /// that names the directory only in its verbatim form; /// - with [`ErrorKind::TimedOut`] if the deadline passes before it takes the store's lock, and /// as [`SandboxStore::lock_existing`] fails before it takes the sandbox's. /// @@ -315,7 +322,26 @@ impl Store { format!("no sandbox is provisioned in {}", sandbox.display()), ) }; - let locked = match SandboxStore::lock_existing(&sandbox, files, deadline) { + // The lease, the binding, and the lock name the sandbox by its path with its links + // resolved, which no later change of a link redirects to another directory. + let resolved = match filesystem::canonicalize(&sandbox) { + Ok(resolved) => resolved, + Err(error) if error.kind() == io::ErrorKind::NotFound => { + return Err(not_provisioned().with_source(error)); + } + Err(error) => { + let message = format!( + "cannot resolve the path of the sandbox in {}", + sandbox.display() + ); + return Err(if error.kind() == io::ErrorKind::Unsupported { + Error::unsupported(message).with_source(error) + } else { + store_error(message)(error) + }); + } + }; + let locked = match SandboxStore::lock_existing(&resolved, files, deadline) { Ok(locked) => locked, Err(error) if error.kind() == ErrorKind::NotFound => { return Err(not_provisioned().with_source(error)); diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 750a462c5..0d317b57c 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -48,13 +48,14 @@ fn files() -> CallerFiles { CallerFiles::new(CONFIG).unwrap() } -/// Provisions the sandbox `name` in `parent`, and returns its directory. +/// Provisions the sandbox `name` in `parent`, and returns its directory, with its links resolved, +/// as a lease names it. fn provision(parent: &Path, name: &str) -> PathBuf { let dir = parent.join(name); let locked = SandboxStore::lock_for_provision(&dir, &files(), None).unwrap(); fs::write(dir.join(CONFIG), b"{}").unwrap(); drop(locked); - dir + filesystem::canonicalize(&dir).unwrap() } /// Returns the time of the host that captured `generation`, during the boot of the capture, @@ -718,23 +719,26 @@ fn a_retirement_makes_each_step_durable_before_the_next() { #[test] fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_record() { let (directory, store) = new_store(); + let root = filesystem::canonicalize(directory.path()).unwrap(); let first = publish(&store, &ManifestSpec::template(), storage(1)); let second = publish(&store, &ManifestSpec::template(), storage(2)); let third = publish(&store, &ManifestSpec::template(), storage(3)); let fourth = publish(&store, &ManifestSpec::template(), storage(4)); - let released = provision(directory.path(), "released"); - let removed = provision(directory.path(), "removed"); - let reprovisioned = provision(directory.path(), "reprovisioned"); - // A sandbox whose parent the path names through a link. - let sandboxes = directory.path().join("sandboxes"); + let released = provision(&root, "released"); + let removed = provision(&root, "removed"); + let reprovisioned = provision(&root, "reprovisioned"); + let sandboxes = root.join("sandboxes"); fs::create_dir(&sandboxes).unwrap(); - let linked = directory.path().join("linked"); - link_dir(&sandboxes, &linked); - let removed_through_link = provision(&linked, "removed"); + let removed_through_link = provision(&sandboxes, "removed"); let released_lease = leased(&store, &first, &released); let removed_lease = leased(&store, &second, &removed); let linked_lease = leased(&store, &third, &removed_through_link); let reprovisioned_lease = leased(&store, &fourth, &reprovisioned); + // The directory that holds a sandbox moves, and a link at its path names it, so the path that + // names the sandbox's lease runs through the link. + let moved = root.join("moved"); + fs::rename(&sandboxes, &moved).unwrap(); + link_dir(&moved, &sandboxes); // A release that removed its binding, but did not make that durable, and removals of // sandboxes, one of which is provisioned again at its path. fs::remove_file(released.join(CLONE_BINDING_NAME)).unwrap(); @@ -742,7 +746,7 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec let locked = SandboxStore::lock_existing(sandbox, &files(), None).unwrap(); assert!(locked.remove().unwrap().is_none()); } - assert_eq!(provision(directory.path(), "reprovisioned"), reprovisioned); + assert_eq!(provision(&root, "reprovisioned"), reprovisioned); let record = |lease: &Lease| { store .dir() @@ -772,14 +776,14 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec ( &first, &released_lease, - vec![released.as_path(), directory.path()], + vec![released.as_path(), root.as_path()], ), - (&second, &removed_lease, vec![directory.path()]), - (&third, &linked_lease, vec![linked.as_path()]), + (&second, &removed_lease, vec![root.as_path()]), + (&third, &linked_lease, vec![sandboxes.as_path()]), ( &fourth, &reprovisioned_lease, - vec![reprovisioned.as_path(), directory.path()], + vec![reprovisioned.as_path(), root.as_path()], ), ] { let path = record(lease); @@ -806,11 +810,16 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec } // A link to nothing hides the directory that held a sandbox, so the record stays. let fifth = publish(&store, &ManifestSpec::template(), storage(5)); - let dangling = provision(&linked, "dangling"); + let parked = root.join("parked"); + fs::create_dir(&parked).unwrap(); + let dangling = provision(&parked, "dangling"); let dangling_lease = leased(&store, &fifth, &dangling); let locked = SandboxStore::lock_existing(&dangling, &files(), None).unwrap(); assert!(locked.remove().unwrap().is_none()); - fs::remove_dir(&sandboxes).unwrap(); + let away = root.join("away"); + fs::rename(&parked, &away).unwrap(); + link_dir(&away, &parked); + fs::remove_dir(&away).unwrap(); let error = store.retire(fifth.generation(), None).unwrap_err(); assert_eq!(error.kind(), ErrorKind::Io, "{error}"); assert!( @@ -821,6 +830,102 @@ fn a_retirement_makes_the_absence_of_a_binding_durable_before_it_removes_its_rec assert!(store.snapshot_dir(&fifth).is_ok()); } +#[test] +fn a_lease_names_its_sandbox_by_its_path_with_its_links_resolved() { + let (directory, store) = new_store(); + let root = filesystem::canonicalize(directory.path()).unwrap(); + let first = publish(&store, &ManifestSpec::template(), storage(1)); + let second = publish(&store, &ManifestSpec::template(), storage(2)); + let sandboxes = root.join("sandboxes"); + fs::create_dir(&sandboxes).unwrap(); + let parent = provision(&sandboxes, "parent"); + let linked = provision(&sandboxes, "linked"); + // Paths through a directory that a `..` component passes through, and through a link, both of + // which go while the clones run. + let helper = root.join("helper"); + fs::create_dir(&helper).unwrap(); + let link = root.join("link"); + link_dir(&sandboxes, &link); + let mut leases = Vec::new(); + for (generation, sandbox, path) in [ + ( + &first, + &parent, + helper.join("..").join("sandboxes").join("parent"), + ), + (&second, &linked, link.join("linked")), + ] { + let acquired = store + .acquire( + generation.generation(), + &path, + &files(), + &after(generation, 1), + None, + ) + .unwrap(); + assert_eq!(acquired.lease.sandbox(), sandbox); + assert_eq!(acquired.locked.dir(), sandbox); + drop(acquired.locked); + assert_eq!(binding(sandbox), Some(acquired.lease.clone())); + leases.push(acquired.lease); + } + fs::remove_dir(&helper).unwrap(); + unlink_dir(&link); + // A retirement still finds both bindings, and keeps the generations. + for (generation, lease) in [&first, &second].into_iter().zip(leases) { + assert_eq!( + store.retire(generation.generation(), None).unwrap(), + Retirement::Draining(vec![lease]) + ); + } +} + +#[test] +fn an_acquisition_binds_the_sandbox_that_it_resolved_whatever_its_path_names_meanwhile() { + let (directory, store) = new_store(); + let root = filesystem::canonicalize(directory.path()).unwrap(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let (first, second) = (root.join("first"), root.join("second")); + fs::create_dir(&first).unwrap(); + fs::create_dir(&second).unwrap(); + let leased_sandbox = provision(&first, "sandbox"); + let other = provision(&second, "sandbox"); + let link = root.join("link"); + link_dir(&first, &link); + // The link names the other sandbox's directory once the acquisition has resolved its path, as + // it makes the lease's record durable. + let retargeted = Cell::new(false); + let retarget = |dir: &Path| -> Result<()> { + if dir == store.dir().join(LEASES_NAME) && !retargeted.replace(true) { + unlink_dir(&link); + link_dir(&second, &link); + } + Ok(()) + }; + let acquired = store + .acquire_with( + published.generation(), + link.join("sandbox"), + &files(), + &after(&published, 1), + None, + &retarget, + ) + .unwrap(); + assert!(retargeted.get()); + // The binding, and the lock, are those of the sandbox that the lease names. + assert_eq!(acquired.lease.sandbox(), leased_sandbox); + assert_eq!(acquired.locked.dir(), leased_sandbox); + drop(acquired.locked); + assert_eq!(binding(&leased_sandbox), Some(acquired.lease.clone())); + assert_eq!(binding(&other), None); + assert_eq!( + store.retire(published.generation(), None).unwrap(), + Retirement::Draining(vec![acquired.lease]) + ); +} + #[test] fn a_retirement_that_finds_its_generation_unindexed_makes_the_index_durable_first() { let (_directory, store) = new_store(); From 298947a25a00d8eca97c37027dfef455e84a3b2f Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 16:26:05 -0700 Subject: [PATCH 5/8] nvx-rs: release a lease only through its sandbox's resolved path A release took the sandbox's lock through any path. If that path ran through a link, and the link changed after the release checked that no VM runs, the release could remove another sandbox's binding, whose lock it does not hold, so a retirement could then delete the generation that the other sandbox's running clone uses. A release now refuses, before it removes anything, a lock that does not name the sandbox by its lease's path. That path has its links resolved, and acquire takes the lock through it too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/template/store/leases.rs | 19 +++++++- .../src/host/template/store/leases/tests.rs | 44 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/nvx-rs/src/host/template/store/leases.rs b/nvx-rs/src/host/template/store/leases.rs index 5618de682..6f58f7a61 100644 --- a/nvx-rs/src/host/template/store/leases.rs +++ b/nvx-rs/src/host/template/store/leases.rs @@ -172,7 +172,8 @@ impl Lease { } /// Returns the directory of the sandbox that holds the lease, as an absolute path with its - /// links resolved, which may differ from the path through which the sandbox was leased. + /// links resolved, which may differ from the path through which the sandbox was leased, and + /// through which a [release](Store::release) takes the sandbox's lock. pub fn sandbox(&self) -> &Path { &self.sandbox } @@ -429,7 +430,13 @@ impl Store { /// [retirement](Self::retire) removes, and never a binding without its record. It takes no /// lock of the store. /// - /// Fails with [`ErrorKind::Other`] if the sandbox holds a lease of another store. + /// The caller takes the sandbox's lock through the lease's path, as [`Lease::sandbox`] + /// returns it, and as [`acquire`](Self::acquire) takes it, which has no links: a link in the + /// path of the lock could change as the release runs, and make it remove another sandbox's + /// binding, whose lock it does not hold. + /// + /// Fails with [`ErrorKind::Other`] if the sandbox holds a lease of another store, or if the + /// lock does not name the sandbox by its lease's path, and then removes nothing. pub fn release(&self, locked: &mut LockedStore) -> Result> { self.release_with(locked, &sync_dir) } @@ -451,6 +458,14 @@ impl Store { self.id ))); } + if locked.dir() != lease.sandbox { + return Err(Error::other(format!( + "the lock of the sandbox in {} does not name it by the path of its lease, {}, \ + through which a release takes it", + locked.dir().display(), + lease.sandbox.display() + ))); + } let recorded = self.check_records_dir(LEASES_NAME, LEASES_WHAT)?; unbind(locked.dir(), sync)?; if recorded { diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 0d317b57c..9cab87827 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -926,6 +926,50 @@ fn an_acquisition_binds_the_sandbox_that_it_resolved_whatever_its_path_names_mea ); } +#[test] +fn a_release_takes_the_lock_of_its_sandbox_through_the_path_of_its_lease() { + let (directory, store) = new_store(); + let root = filesystem::canonicalize(directory.path()).unwrap(); + let published = publish(&store, &ManifestSpec::template(), storage(1)); + let generation = published.generation(); + let (first, second) = (root.join("first"), root.join("second")); + fs::create_dir(&first).unwrap(); + fs::create_dir(&second).unwrap(); + let leased_sandbox = provision(&first, "sandbox"); + let other = provision(&second, "sandbox"); + let lease = leased(&store, &published, &leased_sandbox); + let other_lease = leased(&store, &published, &other); + let mut both = vec![lease.clone(), other_lease.clone()]; + both.sort_by_key(Lease::id); + let link = root.join("link"); + link_dir(&first, &link); + // A lock taken through a link refuses a release, which removes nothing, also once the link + // names another sandbox, whose lease a release would otherwise end without its lock. + let mut locked = SandboxStore::lock_existing(link.join("sandbox"), &files(), None).unwrap(); + for retargeted in [false, true] { + if retargeted { + unlink_dir(&link); + link_dir(&second, &link); + } + let error = store.release(&mut locked).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Other, "{error}"); + assert!( + error + .message() + .contains("does not name it by the path of its lease"), + "{error}" + ); + assert_eq!(binding(&leased_sandbox), Some(lease.clone())); + assert_eq!(binding(&other), Some(other_lease.clone())); + assert_eq!(store.leases(generation).unwrap(), both); + } + drop(locked); + // A lock taken through the lease's path ends it. + assert_eq!(release(&store, lease.sandbox()).unwrap(), Some(lease)); + assert_eq!(binding(&leased_sandbox), None); + assert_eq!(store.leases(generation).unwrap(), [other_lease]); +} + #[test] fn a_retirement_that_finds_its_generation_unindexed_makes_the_index_durable_first() { let (_directory, store) = new_store(); From d3f2dc22f53f874238efe4edd279a50f7d5aad7f Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 16:53:15 -0700 Subject: [PATCH 6/8] nvx-rs: make a removal's unbinding durable before the sandbox goes A removal of a sandbox removed its clone binding with its other files without syncing the directory. A retirement that later found the binding gone synced only the directories that still existed. If the sandbox's parent had been removed and created again, that sync missed the directory that held the old entry, so a crash of the host could bring the binding back after the retirement had deleted the generation it names. A removal now removes the binding first and syncs the sandbox's directory before anything else goes. The configuration record still goes last, so a failed sync leaves a provisioned sandbox that can be removed again. The sync also covers a release that a crash interrupted before its own sync. This costs one more directory sync per removal on Unix; on Windows syncing a directory is a no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/store/lifecycle.rs | 45 +++++++++++----- nvx-rs/src/host/store/lifecycle/tests.rs | 65 ++++++++++++++++++++++-- nvx-rs/src/host/store/mod.rs | 19 ++++--- nvx-rs/src/host/template/store.rs | 11 ++-- nvx-rs/src/host/template/store/leases.rs | 26 ++++++---- 5 files changed, 124 insertions(+), 42 deletions(-) diff --git a/nvx-rs/src/host/store/lifecycle.rs b/nvx-rs/src/host/store/lifecycle.rs index 91934a900..2e389d300 100644 --- a/nvx-rs/src/host/store/lifecycle.rs +++ b/nvx-rs/src/host/store/lifecycle.rs @@ -10,9 +10,10 @@ use std::path::{Path, PathBuf}; use std::time::Instant; use super::caller::{is_report_staging, is_store_temporary}; +use super::files::sync_dir; use super::{ - CallerFiles, FILE_NAMES, JOURNAL_NAME, LOCK_NAME, LockedStore, RUNTIME_NAME, Races, - SandboxStore, Taken, absolute, pause, transition_in_progress, + CLONE_BINDING_NAME, CallerFiles, FILE_NAMES, JOURNAL_NAME, LOCK_NAME, LockedStore, + RUNTIME_NAME, Races, SandboxStore, Taken, absolute, pause, transition_in_progress, }; use crate::error::{Error, ErrorCode, ErrorKind, Result}; use crate::host::paths::not_unicode; @@ -267,14 +268,18 @@ impl LockedStore { /// The removal refuses, and removes nothing, while the store records a VM or a launch: the /// caller reattaches to the VM and stops it, or recovers the launch, first, as /// [`supervisor::reattach`](crate::host::supervisor::reattach) does. It then removes the - /// store's files but its lock, the caller's files, its configuration record last, so that an - /// interrupted removal leaves a provisioned sandbox, which can be removed again, and the - /// temporary files that the store owns. It refuses the entries that are neither the store's - /// nor the caller's, which it names in the order in which the directory lists them, and keeps - /// them and the directory; the lock file goes as the lock is released, since the configuration - /// record is gone. Otherwise it removes its lock file while it still holds the lock, so that a - /// transition that waited for the lock finds the sandbox gone rather than halfway removed, and - /// then the directory, as long as the path still names it. + /// sandbox's [clone binding](CLONE_BINDING_NAME), if it holds one, and makes the binding's + /// absence durable before anything else goes, so that no crash after the removal of the + /// directory, or of one of its ancestors, brings back a binding whose lease a retirement may + /// have ended. It then removes the store's files but its lock, the caller's files, its + /// configuration record last, so that an interrupted removal leaves a provisioned sandbox, + /// which can be removed again, and the temporary files that the store owns. It refuses the + /// entries that are neither the store's nor the caller's, which it names in the order in which + /// the directory lists them, and keeps them and the directory; the lock file goes as the lock + /// is released, since the configuration record is gone. Otherwise it removes its lock file + /// while it still holds the lock, so that a transition that waited for the lock finds the + /// sandbox gone rather than halfway removed, and then the directory, as long as the path still + /// names it. /// /// Lock files of other transitions, and on NFS other removed files that handles still hold, /// can keep the emptied directory, and are not refused: NFS renames a removed file that is @@ -287,15 +292,19 @@ impl LockedStore { /// [`wait`](Lingering::wait) removes the directory once it empties. It returns `None` once the /// directory is gone, whoever removed it. pub fn remove(self) -> std::result::Result, RemovalError> { - self.remove_contents()?; + self.remove_contents(&sync_dir)?; self.remove_directory() } /// Empties the directory but for lingering files, its own lock file included, as /// [`remove`](Self::remove) does first: it refuses without the caller's configuration record - /// and while the store records a VM or a launch, removes the store's files and the caller's, - /// and the temporary files that the store owns, and refuses any other entry. - fn remove_contents(&self) -> std::result::Result<(), RemovalError> { + /// and while the store records a VM or a launch, removes the clone binding, and makes its + /// absence durable with `sync`, removes the store's files and the caller's, and the temporary + /// files that the store owns, and refuses any other entry. + fn remove_contents( + &self, + sync: &dyn Fn(&Path) -> Result<()>, + ) -> std::result::Result<(), RemovalError> { if self.files.config().is_none() { return Err(RemovalError::Undeclared); } @@ -307,6 +316,14 @@ impl LockedStore { Err(error) => return Err(RemovalError::Io { path, error }), } } + // The binding's absence, after this removal or after a release that a crash interrupted + // before its sync, is durable while the directory, which keeps the sandbox provisioned, + // remains. + remove_entry(self.path(CLONE_BINDING_NAME))?; + sync(self.dir()).map_err(|error| RemovalError::Io { + path: self.dir().to_owned(), + error: io::Error::other(error), + })?; let owned: Vec = FILE_NAMES .into_iter() .filter(|&name| name != LOCK_NAME) diff --git a/nvx-rs/src/host/store/lifecycle/tests.rs b/nvx-rs/src/host/store/lifecycle/tests.rs index 2fe9595b7..3886ade6b 100644 --- a/nvx-rs/src/host/store/lifecycle/tests.rs +++ b/nvx-rs/src/host/store/lifecycle/tests.rs @@ -1,6 +1,7 @@ //! Tests of the lifecycle locks, their release, and the removal of a store, by threads of one //! process, and by transitions in processes of their own, which [`transition_role`] runs. +use std::cell::RefCell; use std::env; use std::io::{BufRead, BufReader, Read}; use std::process::{Child, ChildStdout, Command, Stdio}; @@ -319,6 +320,7 @@ fn a_removal_removes_the_store_the_callers_files_and_the_directory() { CONSOLE_NAME, CAPABILITY_NAME, SOCKET_NAME, + CLONE_BINDING_NAME, BOOT, ".runtime.json.12345.tmp", ".sandbox.lock.7.tmp", @@ -389,18 +391,73 @@ fn a_removal_is_refused_while_a_vm_or_a_launch_is_recorded() { let (_directory, dir) = sandbox(); fs::write(dir.join(name), b"{}").unwrap(); fs::write(dir.join(LOG_NAME), b"log").unwrap(); + fs::write(dir.join(CLONE_BINDING_NAME), b"{}").unwrap(); assert!( matches!(remove(&dir), Err(RemovalError::Launched)), "{name}" ); - // Nothing was removed, and the provisioned sandbox keeps its lock file. + // Nothing was removed, not even the clone binding of the VM that may run, and the + // provisioned sandbox keeps its lock file. assert_eq!( entries_of(&dir), - sorted(&[CONFIG, LOCK_NAME, LOG_NAME, name]) + sorted(&[CLONE_BINDING_NAME, CONFIG, LOCK_NAME, LOG_NAME, name]) ); } } +#[test] +fn a_removal_makes_the_absence_of_the_clone_binding_durable_before_it_removes_anything_else() { + let (_directory, dir) = sandbox(); + for name in [CLONE_BINDING_NAME, LOG_NAME, BOOT] { + fs::write(dir.join(name), b"{}").unwrap(); + } + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let synced = RefCell::new(Vec::new()); + locked + .remove_contents(&|synced_dir: &Path| { + synced + .borrow_mut() + .push((synced_dir.to_owned(), entries_of(synced_dir))); + Ok(()) + }) + .unwrap(); + // A crash after the sync, even once the directory or one of its ancestors is gone, cannot + // bring the binding back, and one before the configuration record goes leaves a provisioned + // sandbox, whose removal can be retried. + assert_eq!( + synced.into_inner(), + [( + locked.dir().to_owned(), + sorted(&[BOOT, CONFIG, LOCK_NAME, LOG_NAME]) + )] + ); + assert_eq!(entries_of(&dir), sorted(&[LOCK_NAME])); + assert!(locked.remove_directory().unwrap().is_none()); + assert!(!dir.exists()); +} + +#[test] +fn a_removal_that_cannot_make_the_absence_of_the_clone_binding_durable_keeps_the_sandbox() { + let (_directory, dir) = sandbox(); + for name in [CLONE_BINDING_NAME, LOG_NAME] { + fs::write(dir.join(name), b"{}").unwrap(); + } + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let error = locked + .remove_contents(&|_: &Path| Err(Error::io("the sync failed"))) + .unwrap_err(); + assert!( + matches!(&error, RemovalError::Io { path, .. } if path == locked.dir()), + "{error}" + ); + assert_eq!(Error::from(error).kind(), ErrorKind::Io); + drop(locked); + // Only the binding went, so the sandbox is still provisioned, and its removal can be retried. + assert_eq!(entries_of(&dir), sorted(&[CONFIG, LOCK_NAME, LOG_NAME])); + assert!(remove(&dir).unwrap().is_none()); + assert!(!dir.exists()); +} + #[test] fn lingering_lock_files_keep_the_directory_until_they_go() { let (_directory, dir) = sandbox(); @@ -501,7 +558,7 @@ fn a_removal_that_another_finishes_succeeds_and_keeps_what_replaced_the_director }; let (_directory, dir) = sandbox(); let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); - locked.remove_contents().unwrap(); + locked.remove_contents(&sync_dir).unwrap(); let lingering = locked .remove_directory_after(|| removed_by_another(&dir)) .unwrap(); @@ -510,7 +567,7 @@ fn a_removal_that_another_finishes_succeeds_and_keeps_what_replaced_the_director // A directory that replaces it then stays, even an empty one. let (_directory, dir) = sandbox(); let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); - locked.remove_contents().unwrap(); + locked.remove_contents(&sync_dir).unwrap(); let lingering = locked .remove_directory_after(|| { removed_by_another(&dir); diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index a28ca22b9..0ee64a06e 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -193,13 +193,15 @@ //! //! [`LockedStore::remove`] removes a sandbox that no VMM can run in: it refuses while the store //! records a VM or a launch, which the caller reattaches to, or recovers, first. It removes the -//! store's files and the caller's, the configuration record last, and the temporary files that the -//! store owns: those that an interrupted replacement of one of its files leaves, -//! `.NAME.NUMBER.tmp`, and the VMM's [report staging file](#the-report-staging-file). It then -//! refuses entries that are neither the store's nor the caller's, naming them, and keeps the -//! directory, or removes its lock file while it still holds the lock, and then the directory. A -//! removal needs the caller's configuration record, whose absence tells the transitions that wait -//! through it that the sandbox is gone: a lock taken without one refuses it. +//! sandbox's [clone binding](CLONE_BINDING_NAME) first, and makes its absence durable before +//! anything else goes. It then removes the store's files and the caller's, the configuration +//! record last, and the temporary files that the store owns: those that an interrupted +//! replacement of one of its files leaves, `.NAME.NUMBER.tmp`, and the VMM's +//! [report staging file](#the-report-staging-file). It then refuses entries that are neither the +//! store's nor the caller's, naming them, and keeps the directory, or removes its lock file while +//! it still holds the lock, and then the directory. A removal needs the caller's configuration +//! record, whose absence tells the transitions that wait through it that the sandbox is gone: a +//! lock taken without one refuses it. //! //! Files that are still open can keep the emptied directory for a while: NFS renames a removed //! file that is still open, such as the lock file of a waiting transition, to `.nfs*`, Windows @@ -269,7 +271,8 @@ pub const OUTCOME_NAME: &str = "outcome.json"; /// Name of the binding of the sandbox's clone to the generation of a template that it leases, /// which [`template::Store::acquire`](crate::host::template::Store::acquire) writes, durably, /// before the clone's launch, and [`template::Store::release`](crate::host::template::Store::release) -/// removes once its VM has exited. A [removal](self#removal) of the sandbox removes it too. +/// removes once its VM has exited. A [removal](self#removal) of the sandbox removes it first, and +/// makes its absence durable before the sandbox goes. pub const CLONE_BINDING_NAME: &str = "clone.json"; /// Every file that the store keeps in a sandbox's directory. Replacing a record leaves a /// temporary file named `.NAME.NUMBER.tmp` beside it only if the replacement was interrupted, and diff --git a/nvx-rs/src/host/template/store.rs b/nvx-rs/src/host/template/store.rs index 286d95f58..6996afb28 100644 --- a/nvx-rs/src/host/template/store.rs +++ b/nvx-rs/src/host/template/store.rs @@ -377,11 +377,12 @@ impl Index { /// a crash of the host too, even when a sandbox and the store are on different filesystems: an /// acquisition makes the record of a lease durable before it writes the clone binding, and the /// binding before it returns, and a release, or a retirement that finds a binding gone, makes the -/// binding's absence durable before it removes the record, so that no binding outlasts the record -/// that keeps its generation; a retirement makes the tombstone durable before it deletes -/// anything, and each deletion before the next. On Windows, which has no portable way to flush a -/// directory, a rename or a removal is as durable as the filesystem makes it, as OpenVMM's -/// publication of a snapshot assumes. +/// binding's absence durable before it removes the record, as a +/// [removal](crate::host::store#removal) of a sandbox does before the sandbox goes, so that no +/// binding outlasts the record that keeps its generation; a retirement makes the tombstone durable +/// before it deletes anything, and each deletion before the next. On Windows, which has no +/// portable way to flush a directory, a rename or a removal is as durable as the filesystem makes +/// it, as OpenVMM's publication of a snapshot assumes. /// /// # Privacy /// diff --git a/nvx-rs/src/host/template/store/leases.rs b/nvx-rs/src/host/template/store/leases.rs index 6f58f7a61..f928b6305 100644 --- a/nvx-rs/src/host/template/store/leases.rs +++ b/nvx-rs/src/host/template/store/leases.rs @@ -124,7 +124,9 @@ impl FromStr for LeaseId { /// record as its clone binding, [`CLONE_BINDING_NAME`], beside its launch journal. A lease names /// its sandbox by the path of its directory with its links resolved, so that no removal or change /// of a link, or of a directory that a `..` component passes through, hides the sandbox from a -/// [retirement](Store::retire), and a sandbox that holds a lease must not move. +/// [retirement](Store::retire). A sandbox that holds a lease must not move, and goes only through +/// its store's [removal](LockedStore::remove), which makes the binding's absence durable before +/// the sandbox goes. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Lease { /// The lease's identity. @@ -542,16 +544,18 @@ impl Store { /// leaves it. It makes the binding's absence durable first, by syncing the sandbox's /// directory, if it is one, and the directory that holds the sandbox's entry, the nearest of /// its ancestors that remains, through the links that the sandbox's path names, as a release - /// that a crash or a failed sync interrupted, or a removal of the sandbox, may not have. - /// If any lease remains, the generation keeps its - /// payload and its entry in the index, and the retirement returns [`Retirement::Draining`] - /// with the leases, whose clones [release](Self::release) them; a later retirement deletes it. - /// Otherwise it removes the generation's payload, then its entry in the index, and then its - /// tombstone, each durable before the next, as the tombstone is before the payload goes, so - /// that a retirement that a crash interrupts, even one of the host, leaves a tombstoned - /// generation, which retiring it again deletes, and returns [`Retirement::Retired`]. A - /// generation that the store indexed once, and no longer indexes, is retired already. Its - /// store generation is never assigned again. + /// that a crash or a failed sync interrupted may not have. A [removal](LockedStore::remove) of + /// the sandbox made it durable before the sandbox went; for a sandbox that went otherwise, + /// these syncs make it durable only while the directories that held the sandbox remain. If + /// any lease remains, the generation keeps its payload and its entry in the index, and the + /// retirement returns [`Retirement::Draining`] with the leases, whose clones + /// [release](Self::release) them; a later retirement deletes it. Otherwise it removes the + /// generation's payload, then its entry in the index, and then its tombstone, each durable + /// before the next, as the tombstone is before the payload goes, so that a retirement that a + /// crash interrupts, even one of the host, leaves a tombstoned generation, which retiring it + /// again deletes, and returns [`Retirement::Retired`]. A generation that the store indexed + /// once, and no longer indexes, is retired already. Its store generation is never assigned + /// again. /// /// It also removes the temporary files that interrupted replacements of the records of leases /// and tombstones left, since the store writes those only under its lock. From fb2517fd23b4382289bff087374666a29960d1fa Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 19:24:27 -0700 Subject: [PATCH 7/8] nvx-rs: unbind a removed sandbox through a handle on its directory A removal checked that the store recorded no VM and no launch, and then removed the sandbox's clone binding, through the sandbox's path. A lock taken through a path with a link in it stays on the directory that the link named then. If the link changed between the checks and the unlink, the removal could remove the binding of another sandbox, whose VM may run, and a retirement could then delete the generation that its clone still needs. A removal now opens the directory once. Through that handle, it checks that the directory holds the lock file whose lock the removal holds, and refuses otherwise. It then checks that no VM and no launch is recorded, removes the binding, and syncs the directory. Unix uses fstatat, unlinkat and fsync; Windows uses native opens relative to the directory, as is_deleted already did. The rest of the removal still goes through the path, under the store's trust model. A removal also removed the binding twice: first, and again with the store's other files. On a Windows file system without POSIX deletion, a reader that holds the binding open leaves the first removal pending, so the second failed with PermissionDenied and aborted the removal. The binding is now removed once, and one whose deletion is already pending counts as removed. Such a binding lingers like a lock file until its readers close it, in the removal's scan and in Lingering::wait. Where the file system supports POSIX deletion, the binding's name goes at once. The lease tests also retry the recoveries of closed launches, as the supervisor seam's tests do: a process that a concurrent test forks holds a copy of the closed log until it executes its program. Per removal, this adds a check of the lock file's identity, two stat calls, and on Windows one open of the directory; Unix already opened the directory to sync it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/platform/linux.rs | 45 ++++ nvx-rs/src/host/platform/other.rs | 25 ++ nvx-rs/src/host/platform/windows.rs | 228 ++++++++++++++---- nvx-rs/src/host/store/lifecycle.rs | 169 ++++++++----- nvx-rs/src/host/store/lifecycle/tests.rs | 136 ++++++++++- nvx-rs/src/host/store/mod.rs | 33 +-- .../src/host/template/store/leases/tests.rs | 13 +- 7 files changed, 522 insertions(+), 127 deletions(-) diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index 6ad0a8fa8..f60aa2f27 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -591,6 +591,51 @@ pub(crate) fn opened_file_identity(file: &File) -> io::Result<(u64, u64)> { Ok((metadata.dev(), metadata.ino())) } +/// An open directory, through which operations name its entries, so that they reach the entries +/// of the directory that it opened, whatever its path names meanwhile. +pub(crate) struct Directory(File); + +impl Directory { + /// Opens the directory `path`, following its links. + pub(crate) fn open(path: &Path) -> io::Result { + use nix::fcntl::OFlag; + + OpenOptions::new() + .read(true) + .custom_flags(OFlag::O_DIRECTORY.bits()) + .open(path) + .map(Self) + } + + /// Returns the device and inode of its entry `name`, not following a link, as + /// [`opened_file_identity`] gives them, or `None` if it has no such entry. + pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { + use nix::fcntl::AtFlags; + use nix::sys::stat::fstatat; + + match fstatat(&self.0, name, AtFlags::AT_SYMLINK_NOFOLLOW) { + Ok(status) => Ok(Some((status.st_dev, status.st_ino))), + Err(Errno::ENOENT) => Ok(None), + Err(errno) => Err(errno.into()), + } + } + + /// Removes its file `name`, if it has one. + pub(crate) fn remove_file(&self, name: &str) -> io::Result<()> { + use nix::unistd::{UnlinkatFlags, unlinkat}; + + match unlinkat(&self.0, name, UnlinkatFlags::NoRemoveDir) { + Ok(()) | Err(Errno::ENOENT) => Ok(()), + Err(errno) => Err(errno.into()), + } + } + + /// Makes the changes to its entries so far durable, such as a file that was removed from it. + pub(crate) fn sync(&self) -> io::Result<()> { + self.0.sync_all() + } +} + /// Claims a launch's fresh log, which `file` holds open. /// /// The claim is an exclusive `flock` lock. It belongs to the open file description, which the diff --git a/nvx-rs/src/host/platform/other.rs b/nvx-rs/src/host/platform/other.rs index 08afc447f..1b0df5d55 100644 --- a/nvx-rs/src/host/platform/other.rs +++ b/nvx-rs/src/host/platform/other.rs @@ -98,6 +98,31 @@ pub(crate) fn opened_file_identity(_file: &File) -> io::Result<(u64, u64)> { Err(unsupported()) } +/// An open directory, which this host never holds: it keeps no sandbox's store. +pub(crate) enum Directory {} + +impl Directory { + /// Fails: this host keeps no sandbox's store. + pub(crate) fn open(_path: &Path) -> io::Result { + Err(unsupported()) + } + + /// Never called: no directory is open. + pub(crate) fn entry_identity(&self, _name: &str) -> io::Result> { + match *self {} + } + + /// Never called: no directory is open. + pub(crate) fn remove_file(&self, _name: &str) -> io::Result<()> { + match *self {} + } + + /// Never called: no directory is open. + pub(crate) fn sync(&self) -> io::Result<()> { + match *self {} + } +} + pub(crate) fn claim_log(_file: &File) -> io::Result<()> { Err(unsupported()) } diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index e4f299a04..d92f16be9 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -1256,15 +1256,9 @@ pub fn file_identity(path: &Path) -> io::Result<(u64, u64)> { /// Win32 reports a pending deletion as a denied access, so the file is opened with the native API, /// whose status tells the two apart, relative to its directory, which spares a native path. pub(crate) fn is_deleted(path: &Path) -> bool { - use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; - use windows_sys::Wdk::Storage::FileSystem::{FILE_OPEN_REPARSE_POINT, NtOpenFile}; - use windows_sys::Win32::Foundation::{ - OBJ_CASE_INSENSITIVE, STATUS_DELETE_PENDING, STATUS_OBJECT_NAME_NOT_FOUND, UNICODE_STRING, - }; - use windows_sys::Win32::Storage::FileSystem::{ - FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, - }; - use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; + use windows_sys::Wdk::Storage::FileSystem::FILE_OPEN_REPARSE_POINT; + use windows_sys::Win32::Foundation::{STATUS_DELETE_PENDING, STATUS_OBJECT_NAME_NOT_FOUND}; + use windows_sys::Win32::Storage::FileSystem::FILE_READ_ATTRIBUTES; let Some(name) = path.file_name() else { return false; @@ -1273,51 +1267,197 @@ pub(crate) fn is_deleted(path: &Path) -> bool { Some(dir) if !dir.as_os_str().is_empty() => dir, _ => Path::new("."), }; - let dir = match OpenOptions::new() - .access_mode(0) - .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) - .open(dir) - { + let dir = match Directory::open(dir) { Ok(dir) => dir, // The file went with its directory. Err(error) => return error.kind() == io::ErrorKind::NotFound, }; - let name: Vec = name.encode_wide().collect(); - let Ok(len) = u16::try_from(mem::size_of_val(name.as_slice())) else { - return false; + match dir.open_entry(name, FILE_READ_ATTRIBUTES, FILE_OPEN_REPARSE_POINT) { + Ok(_) => false, + Err(status) => matches!(status, STATUS_DELETE_PENDING | STATUS_OBJECT_NAME_NOT_FOUND), + } +} + +/// An open directory, through which operations name its entries, so that they reach the entries +/// of the directory that it opened, whatever its path names meanwhile. +/// +/// They use the native API, whose status tells a file whose deletion is pending from one whose +/// access control list refuses the open, which Win32 both reports as a denied access. +pub(crate) struct Directory(fs::File); + +impl Directory { + /// Opens the directory `path`, following its links, without access rights, which the opens of + /// its entries do not need. + pub(crate) fn open(path: &Path) -> io::Result { + OpenOptions::new() + .access_mode(0) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(path) + .map(Self) + } + + /// Returns the volume serial number and file index of its entry `name`, not following a link + /// or other reparse point, as [`opened_file_identity`] gives them, or `None` if it has no such + /// entry. + pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { + use windows_sys::Wdk::Storage::FileSystem::{ + FILE_OPEN_REPARSE_POINT, FILE_SYNCHRONOUS_IO_NONALERT, + }; + use windows_sys::Win32::Foundation::STATUS_OBJECT_NAME_NOT_FOUND; + use windows_sys::Win32::Storage::FileSystem::{FILE_READ_ATTRIBUTES, SYNCHRONIZE}; + + match self.open_entry( + OsStr::new(name), + FILE_READ_ATTRIBUTES | SYNCHRONIZE, + FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, + ) { + Ok(entry) => handle_identity(raw(&entry)).map(Some), + Err(STATUS_OBJECT_NAME_NOT_FOUND) => Ok(None), + Err(status) => Err(status_error(status)), + } + } + + /// Removes its file `name`, if it has one. A file whose deletion is pending already, which a + /// file system without POSIX deletion keeps under its name until its last handle closes, + /// counts as removed. + pub(crate) fn remove_file(&self, name: &str) -> io::Result<()> { + use windows_sys::Wdk::Storage::FileSystem::{ + FILE_NON_DIRECTORY_FILE, FILE_OPEN_REPARSE_POINT, FILE_SYNCHRONOUS_IO_NONALERT, + }; + use windows_sys::Win32::Foundation::{STATUS_DELETE_PENDING, STATUS_OBJECT_NAME_NOT_FOUND}; + use windows_sys::Win32::Storage::FileSystem::{DELETE, SYNCHRONIZE}; + + match self.open_entry( + OsStr::new(name), + DELETE | SYNCHRONIZE, + FILE_OPEN_REPARSE_POINT | FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT, + ) { + Ok(entry) => delete_opened(&entry), + Err(STATUS_OBJECT_NAME_NOT_FOUND | STATUS_DELETE_PENDING) => Ok(()), + Err(status) => Err(status_error(status)), + } + } + + /// Does nothing: Windows has no portable way to flush a directory, so a removal from it is as + /// durable as the file system makes it. + pub(crate) fn sync(&self) -> io::Result<()> { + Ok(()) + } + + /// Opens its entry `name` with the native API, with the access `access` and the options + /// `options`, sharing it with every other open, or returns the status with which the open + /// fails. + fn open_entry( + &self, + name: &OsStr, + access: u32, + options: u32, + ) -> Result { + use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; + use windows_sys::Wdk::Storage::FileSystem::NtOpenFile; + use windows_sys::Win32::Foundation::{ + OBJ_CASE_INSENSITIVE, STATUS_INVALID_HANDLE, STATUS_OBJECT_NAME_INVALID, UNICODE_STRING, + }; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + }; + use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; + + let name: Vec = name.encode_wide().collect(); + let Ok(len) = u16::try_from(mem::size_of_val(name.as_slice())) else { + return Err(STATUS_OBJECT_NAME_INVALID); + }; + let name = UNICODE_STRING { + Length: len, + MaximumLength: len, + Buffer: name.as_ptr().cast_mut(), + }; + let attributes = OBJECT_ATTRIBUTES { + Length: mem::size_of::() as u32, + RootDirectory: self.0.as_raw_handle() as HANDLE, + ObjectName: &name, + // As Win32 opens files. + Attributes: OBJ_CASE_INSENSITIVE, + ..OBJECT_ATTRIBUTES::default() + }; + let mut handle = ptr::null_mut(); + let mut status_block = IO_STATUS_BLOCK::default(); + // SAFETY: the directory's handle is open, the attributes and the name that they point to + // outlive the call, and the outputs are valid. + let status = unsafe { + NtOpenFile( + &mut handle, + access, + &attributes, + &mut status_block, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + options, + ) + }; + if status < 0 { + return Err(status); + } + owned(handle).ok_or(STATUS_INVALID_HANDLE) + } +} + +/// Deletes the file that `file`, opened with the right to delete it, holds: with POSIX semantics +/// where the file system supports them, so that its name goes at once, even while other handles +/// hold it, and otherwise once the last handle to it closes. +fn delete_opened(file: &OwnedHandle) -> io::Result<()> { + use windows_sys::Win32::Foundation::{ERROR_INVALID_FUNCTION, ERROR_NOT_SUPPORTED}; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_DISPOSITION_FLAG_DELETE, FILE_DISPOSITION_FLAG_POSIX_SEMANTICS, FILE_DISPOSITION_INFO, + FILE_DISPOSITION_INFO_EX, FileDispositionInfo, FileDispositionInfoEx, + SetFileInformationByHandle, }; - let name = UNICODE_STRING { - Length: len, - MaximumLength: len, - Buffer: name.as_ptr().cast_mut(), + + let posix = FILE_DISPOSITION_INFO_EX { + Flags: FILE_DISPOSITION_FLAG_DELETE | FILE_DISPOSITION_FLAG_POSIX_SEMANTICS, }; - let attributes = OBJECT_ATTRIBUTES { - Length: mem::size_of::() as u32, - RootDirectory: dir.as_raw_handle() as HANDLE, - ObjectName: &name, - // As Win32 opens files. - Attributes: OBJ_CASE_INSENSITIVE, - ..OBJECT_ATTRIBUTES::default() + // SAFETY: the handle is open, and the information is a FILE_DISPOSITION_INFO_EX of its size. + let set = unsafe { + SetFileInformationByHandle( + raw(file), + FileDispositionInfoEx, + (&raw const posix).cast(), + mem::size_of::() as u32, + ) }; - let mut handle = ptr::null_mut(); - let mut status_block = IO_STATUS_BLOCK::default(); - // SAFETY: the directory's handle is open, the attributes and the name that they point to - // outlive the call, and the outputs are valid. - let status = unsafe { - NtOpenFile( - &mut handle, - FILE_READ_ATTRIBUTES, - &attributes, - &mut status_block, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - FILE_OPEN_REPARSE_POINT, + if set != 0 { + return Ok(()); + } + let error = io::Error::last_os_error(); + // File systems without POSIX deletion refuse the extended information. + if !matches!( + error.raw_os_error().map(|code| code as u32), + Some(ERROR_INVALID_PARAMETER | ERROR_NOT_SUPPORTED | ERROR_INVALID_FUNCTION) + ) { + return Err(error); + } + let disposition = FILE_DISPOSITION_INFO { DeleteFile: true }; + // SAFETY: the handle is open, and the information is a FILE_DISPOSITION_INFO of its size. + let set = unsafe { + SetFileInformationByHandle( + raw(file), + FileDispositionInfo, + (&raw const disposition).cast(), + mem::size_of::() as u32, ) }; - if status >= 0 { - drop(owned(handle)); - return false; + if set == 0 { + return Err(io::Error::last_os_error()); } - matches!(status, STATUS_DELETE_PENDING | STATUS_OBJECT_NAME_NOT_FOUND) + Ok(()) +} + +/// Returns the Win32 error to which the native status `status` maps. +fn status_error(status: windows_sys::Win32::Foundation::NTSTATUS) -> io::Error { + use windows_sys::Win32::Foundation::RtlNtStatusToDosError; + + // SAFETY: the conversion only reads its argument. + let code = unsafe { RtlNtStatusToDosError(status) }; + io::Error::from_raw_os_error(code as i32) } /// Returns the volume serial number and file index of the file that `file` holds open. diff --git a/nvx-rs/src/host/store/lifecycle.rs b/nvx-rs/src/host/store/lifecycle.rs index 2e389d300..0310337b3 100644 --- a/nvx-rs/src/host/store/lifecycle.rs +++ b/nvx-rs/src/host/store/lifecycle.rs @@ -10,7 +10,6 @@ use std::path::{Path, PathBuf}; use std::time::Instant; use super::caller::{is_report_staging, is_store_temporary}; -use super::files::sync_dir; use super::{ CLONE_BINDING_NAME, CallerFiles, FILE_NAMES, JOURNAL_NAME, LOCK_NAME, LockedStore, RUNTIME_NAME, Races, SandboxStore, Taken, absolute, pause, transition_in_progress, @@ -267,66 +266,63 @@ impl LockedStore { /// /// The removal refuses, and removes nothing, while the store records a VM or a launch: the /// caller reattaches to the VM and stops it, or recovers the launch, first, as - /// [`supervisor::reattach`](crate::host::supervisor::reattach) does. It then removes the - /// sandbox's [clone binding](CLONE_BINDING_NAME), if it holds one, and makes the binding's - /// absence durable before anything else goes, so that no crash after the removal of the - /// directory, or of one of its ancestors, brings back a binding whose lease a retirement may - /// have ended. It then removes the store's files but its lock, the caller's files, its - /// configuration record last, so that an interrupted removal leaves a provisioned sandbox, - /// which can be removed again, and the temporary files that the store owns. It refuses the - /// entries that are neither the store's nor the caller's, which it names in the order in which - /// the directory lists them, and keeps them and the directory; the lock file goes as the lock - /// is released, since the configuration record is gone. Otherwise it removes its lock file - /// while it still holds the lock, so that a transition that waited for the lock finds the - /// sandbox gone rather than halfway removed, and then the directory, as long as the path still - /// names it. + /// [`supervisor::reattach`](crate::host::supervisor::reattach) does. It also refuses, with + /// [`RemovalError::Io`], and removes nothing, once the directory that its path names holds + /// another lock file than this lock's, as after a link in the path changed. It makes both + /// checks, and removes the sandbox's [clone binding](CLONE_BINDING_NAME), if it holds one, + /// through one handle of the directory that it opens, so that no change of a link in the path + /// meanwhile redirects the binding's removal to another sandbox, whose VM may run. It makes + /// the binding's absence durable before anything else goes, so that no crash after the + /// removal of the directory, or of one of its ancestors, brings back a binding whose lease a + /// retirement may have ended. It then removes the store's files but its lock, the caller's + /// files, its configuration record last, so that an interrupted removal leaves a provisioned + /// sandbox, which can be removed again, and the temporary files that the store owns. It + /// refuses the entries that are neither the store's nor the caller's, which it names in the + /// order in which the directory lists them, and keeps them and the directory; the lock file + /// goes as the lock is released, since the configuration record is gone. Otherwise it removes + /// its lock file while it still holds the lock, so that a transition that waited for the lock + /// finds the sandbox gone rather than halfway removed, and then the directory, as long as the + /// path still names it. /// - /// Lock files of other transitions, and on NFS other removed files that handles still hold, - /// can keep the emptied directory, and are not refused: NFS renames a removed file that is - /// still open to `.nfs*`; a Windows file system without POSIX deletion keeps a removed lock - /// file under its own name until its last handle closes; and a transition that opens the lock - /// file between the removals of the lock file and of the directory creates a new one. Each of - /// those transitions finds no sandbox, and removes its lock file as it releases its lock, and - /// the handles of other removed files close, so the directory empties. The removal then holds - /// the directory's identity, releases the lock, and returns the [`Lingering`] removal, whose + /// Lock files of other transitions, a clone binding whose deletion is pending, and on NFS other + /// removed files that handles still hold, can keep the emptied directory, and are not refused: + /// NFS renames a removed file that is still open to `.nfs*`; a Windows file system without + /// POSIX deletion keeps a removed lock file, or a removed binding that a reader holds, under + /// its own name until its last handle closes; and a transition that opens the lock file + /// between the removals of the lock file and of the directory creates a new one. Each of those + /// transitions finds no sandbox, and removes its lock file as it releases its lock, and the + /// handles of other removed files close, so the directory empties. The removal then holds the + /// directory's identity, releases the lock, and returns the [`Lingering`] removal, whose /// [`wait`](Lingering::wait) removes the directory once it empties. It returns `None` once the /// directory is gone, whoever removed it. pub fn remove(self) -> std::result::Result, RemovalError> { - self.remove_contents(&sync_dir)?; + self.remove_contents()?; self.remove_directory() } /// Empties the directory but for lingering files, its own lock file included, as - /// [`remove`](Self::remove) does first: it refuses without the caller's configuration record - /// and while the store records a VM or a launch, removes the clone binding, and makes its - /// absence durable with `sync`, removes the store's files and the caller's, and the temporary - /// files that the store owns, and refuses any other entry. - fn remove_contents( + /// [`remove`](Self::remove) does first: it refuses without the caller's configuration record, + /// removes the clone binding as [`unbind`](Self::unbind) does, then the store's files and the + /// caller's, and the temporary files that the store owns, and refuses any other entry. + fn remove_contents(&self) -> std::result::Result<(), RemovalError> { + self.remove_contents_with(&|| {}, &platform::Directory::sync) + } + + /// Empties the directory as [`remove_contents`](Self::remove_contents) does, with `checked` + /// and `sync` as [`unbind`](Self::unbind) takes them. + fn remove_contents_with( &self, - sync: &dyn Fn(&Path) -> Result<()>, + checked: &dyn Fn(), + sync: &dyn Fn(&platform::Directory) -> io::Result<()>, ) -> std::result::Result<(), RemovalError> { if self.files.config().is_none() { return Err(RemovalError::Undeclared); } - for name in [JOURNAL_NAME, RUNTIME_NAME] { - let path = self.path(name); - match fs::symlink_metadata(&path) { - Ok(_) => return Err(RemovalError::Launched), - Err(error) if error.kind() == io::ErrorKind::NotFound => {} - Err(error) => return Err(RemovalError::Io { path, error }), - } - } - // The binding's absence, after this removal or after a release that a crash interrupted - // before its sync, is durable while the directory, which keeps the sandbox provisioned, - // remains. - remove_entry(self.path(CLONE_BINDING_NAME))?; - sync(self.dir()).map_err(|error| RemovalError::Io { - path: self.dir().to_owned(), - error: io::Error::other(error), - })?; + self.unbind(checked, sync)?; + // The binding is removed once: one whose deletion is pending refuses a second removal. let owned: Vec = FILE_NAMES .into_iter() - .filter(|&name| name != LOCK_NAME) + .filter(|&name| name != LOCK_NAME && name != CLONE_BINDING_NAME) .chain(self.files.names()) .map(|name| self.path(name)) .collect(); @@ -339,9 +335,10 @@ impl LockedStore { })?; let mut foreign = Vec::new(); for name in listed { - // Lock files, and removed files that NFS keeps while handles hold them, go once their - // holders release them, which the removal waits for, if they keep the directory. - if lingers(&name) { + // Lock files, a binding whose deletion is pending, and removed files that NFS keeps + // while handles hold them, go once their holders release them, which the removal + // waits for, if they keep the directory. + if lingers(self.dir(), &name) { continue; } if is_store_temporary(&name) || is_report_staging(&name) { @@ -357,6 +354,58 @@ impl LockedStore { } } + /// Removes the sandbox's clone binding, if it holds one, and makes its absence durable with + /// `sync`, before anything else of the sandbox goes, through one handle of the directory that + /// the path names. Through that handle, it checks first that the directory holds this lock's + /// file, and refuses otherwise, then that the store records no VM and no launch, and runs + /// `checked` once both checks pass, before it removes the binding. No change of a link in the + /// path meanwhile redirects these steps to another sandbox, whose VM may run, and whose + /// binding then stays. + /// + /// The binding's absence, after this removal or after a release that a crash interrupted + /// before its sync, is then durable while the directory, which keeps the sandbox provisioned, + /// remains. A binding whose deletion is pending already counts as removed. + fn unbind( + &self, + checked: &dyn Fn(), + sync: &dyn Fn(&platform::Directory) -> io::Result<()>, + ) -> std::result::Result<(), RemovalError> { + let dir = self.dir(); + let failed = |error: io::Error| RemovalError::Io { + path: dir.to_owned(), + error, + }; + let entry_failed = |name: &str| { + let path = self.path(name); + move |error: io::Error| RemovalError::Io { path, error } + }; + let directory = platform::Directory::open(dir).map_err(failed)?; + let held = platform::opened_file_identity(&self.lock).map_err(entry_failed(LOCK_NAME))?; + if directory + .entry_identity(LOCK_NAME) + .map_err(entry_failed(LOCK_NAME))? + != Some(held) + { + return Err(failed(io::Error::other( + "the directory that the path names does not hold the sandbox's lock", + ))); + } + for name in [JOURNAL_NAME, RUNTIME_NAME] { + if directory + .entry_identity(name) + .map_err(entry_failed(name))? + .is_some() + { + return Err(RemovalError::Launched); + } + } + checked(); + directory + .remove_file(CLONE_BINDING_NAME) + .map_err(entry_failed(CLONE_BINDING_NAME))?; + sync(&directory).map_err(failed) + } + /// Removes the lock file while it still holds the lock, and then the directory, as /// [`remove`](Self::remove) does once the directory holds nothing else of the sandbox. fn remove_directory(self) -> std::result::Result, RemovalError> { @@ -440,8 +489,8 @@ impl Lingering { /// appears in it, as a provision that took the lock after the removal released it writes it, /// which it looks up by its path, so that a file system that ignores case finds it whatever /// the case of the name that the provision gave it, with [`RemovalError::Foreign`] for other - /// entries than lock files and the removed files that NFS keeps, sorted, and with - /// [`RemovalError::TimedOut`] once the deadline passes. + /// entries than lock files, a clone binding whose deletion is pending, and the removed files + /// that NFS keeps, sorted, and with [`RemovalError::TimedOut`] once the deadline passes. pub fn wait(&mut self, deadline: Option) -> std::result::Result<(), RemovalError> { loop { let error = match remove_held_directory(&self.dir, self.identity) { @@ -464,7 +513,7 @@ impl Lingering { }; let mut unexpected: Vec = remaining .iter() - .filter(|name| !lingers(name)) + .filter(|name| !lingers(&self.dir, name)) .cloned() .collect(); unexpected.sort(); @@ -598,12 +647,16 @@ fn remove_entry(path: PathBuf) -> std::result::Result<(), RemovalError> { } } -/// Returns whether `name` is a lock file, or a removed file that a handle still keeps, which goes -/// once its holder releases or closes it: NFS renames a removed file that is still open to -/// `.nfs*`, and Windows file systems without POSIX deletion keep a removed lock file under its -/// own name, as they keep the lock files that waiting transitions create until they release them. -fn lingers(name: &str) -> bool { - name == LOCK_NAME || name.starts_with(".nfs") +/// Returns whether `name`, an entry of the directory `dir`, is a lock file, or a removed file that +/// a handle still keeps, which goes once its holder releases or closes it: NFS renames a removed +/// file that is still open to `.nfs*`, and Windows file systems without POSIX deletion keep a +/// removed lock file under its own name, as they keep the lock files that waiting transitions +/// create until they release them, and a removed clone binding until its readers close it, whose +/// deletion is then pending. +fn lingers(dir: &Path, name: &str) -> bool { + name == LOCK_NAME + || name.starts_with(".nfs") + || (name == CLONE_BINDING_NAME && platform::is_deleted(&dir.join(name))) } /// Returns whether `error` is how the removal of a directory reports one that is not empty. diff --git a/nvx-rs/src/host/store/lifecycle/tests.rs b/nvx-rs/src/host/store/lifecycle/tests.rs index 3886ade6b..f024866af 100644 --- a/nvx-rs/src/host/store/lifecycle/tests.rs +++ b/nvx-rs/src/host/store/lifecycle/tests.rs @@ -414,10 +414,10 @@ fn a_removal_makes_the_absence_of_the_clone_binding_durable_before_it_removes_an let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); let synced = RefCell::new(Vec::new()); locked - .remove_contents(&|synced_dir: &Path| { - synced - .borrow_mut() - .push((synced_dir.to_owned(), entries_of(synced_dir))); + .remove_contents_with(&|| {}, &|directory| { + // The directory that the sync makes durable holds the sandbox's lock file. + let lock = directory.entry_identity(LOCK_NAME).unwrap(); + synced.borrow_mut().push((lock, entries_of(&dir))); Ok(()) }) .unwrap(); @@ -427,7 +427,7 @@ fn a_removal_makes_the_absence_of_the_clone_binding_durable_before_it_removes_an assert_eq!( synced.into_inner(), [( - locked.dir().to_owned(), + Some(platform::file_identity(&dir.join(LOCK_NAME)).unwrap()), sorted(&[BOOT, CONFIG, LOCK_NAME, LOG_NAME]) )] ); @@ -444,7 +444,7 @@ fn a_removal_that_cannot_make_the_absence_of_the_clone_binding_durable_keeps_the } let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); let error = locked - .remove_contents(&|_: &Path| Err(Error::io("the sync failed"))) + .remove_contents_with(&|| {}, &|_| Err(io::Error::other("the sync failed"))) .unwrap_err(); assert!( matches!(&error, RemovalError::Io { path, .. } if path == locked.dir()), @@ -458,6 +458,106 @@ fn a_removal_that_cannot_make_the_absence_of_the_clone_binding_durable_keeps_the assert!(!dir.exists()); } +/// Provisions a sandbox in `dir`, whose parent exists. +fn provision(dir: &Path) { + let locked = SandboxStore::lock_for_provision(dir, &files(), None).unwrap(); + fs::write(dir.join(CONFIG), b"{}").unwrap(); + drop(locked); +} + +/// Links `link` to the directory `target`: a symbolic link on Unix, and a junction on Windows, +/// which any user may create. +fn link_dir(target: &Path, link: &Path) { + #[cfg(unix)] + std::os::unix::fs::symlink(target, link).unwrap(); + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + + let status = Command::new("cmd") + .raw_arg(format!( + "/C mklink /J \"{}\" \"{}\"", + link.display(), + target.display() + )) + .stdout(Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "cannot link {}", link.display()); + } +} + +/// Makes the link `link`, which [`link_dir`] made, name the directory `target` instead. +fn relink(target: &Path, link: &Path) { + #[cfg(unix)] + fs::remove_file(link).unwrap(); + // Removing a junction removes the link alone. + #[cfg(windows)] + fs::remove_dir(link).unwrap(); + link_dir(target, link); +} + +/// Provisions the sandboxes `one/sandbox` and `two/sandbox` in `root`, each holding a clone +/// binding, links `root/link` to `one`, and returns `one`, `two`, and the link. +fn linked_sandboxes(root: &Path) -> (PathBuf, PathBuf, PathBuf) { + let (one, two, link) = (root.join("one"), root.join("two"), root.join("link")); + for parent in [&one, &two] { + fs::create_dir(parent).unwrap(); + let sandbox = parent.join("sandbox"); + provision(&sandbox); + fs::write(sandbox.join(CLONE_BINDING_NAME), b"{}").unwrap(); + } + link_dir(&one, &link); + (one, two, link) +} + +#[test] +fn a_removal_unbinds_the_sandbox_whose_lock_it_holds_whatever_its_path_names_meanwhile() { + let directory = tempfile::tempdir().unwrap(); + let (one, two, link) = linked_sandboxes(directory.path()); + // The other sandbox records a launch, so its VM may run, and its binding must stay. + fs::write(two.join("sandbox").join(JOURNAL_NAME), b"{}").unwrap(); + let locked = SandboxStore::lock_existing(link.join("sandbox"), &files(), None).unwrap(); + // The link names the other sandbox's parent from the checks to the binding's removal, and + // this one's again from the sync on. + locked + .remove_contents_with(&|| relink(&two, &link), &|_| { + relink(&one, &link); + Ok(()) + }) + .unwrap(); + assert_eq!(entries_of(&one.join("sandbox")), [LOCK_NAME]); + assert_eq!( + entries_of(&two.join("sandbox")), + sorted(&[CLONE_BINDING_NAME, CONFIG, JOURNAL_NAME, LOCK_NAME]) + ); + assert!(locked.remove_directory().unwrap().is_none()); + assert!(!one.join("sandbox").exists()); +} + +#[test] +fn a_removal_through_a_path_that_names_another_sandbox_meanwhile_removes_nothing() { + let directory = tempfile::tempdir().unwrap(); + let (one, two, link) = linked_sandboxes(directory.path()); + let locked = SandboxStore::lock_existing(link.join("sandbox"), &files(), None).unwrap(); + let dir = locked.dir().to_owned(); + // Once the lock is taken, the link comes to name the other sandbox's parent, whose lock file + // is not this lock's. + relink(&two, &link); + let error = locked.remove().unwrap_err(); + assert!( + matches!(&error, RemovalError::Io { path, .. } if *path == dir), + "{error}" + ); + assert_eq!(Error::from(error).kind(), ErrorKind::Io); + for parent in [&one, &two] { + assert_eq!( + entries_of(&parent.join("sandbox")), + sorted(&[CLONE_BINDING_NAME, CONFIG, LOCK_NAME]) + ); + } +} + #[test] fn lingering_lock_files_keep_the_directory_until_they_go() { let (_directory, dir) = sandbox(); @@ -558,7 +658,7 @@ fn a_removal_that_another_finishes_succeeds_and_keeps_what_replaced_the_director }; let (_directory, dir) = sandbox(); let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); - locked.remove_contents(&sync_dir).unwrap(); + locked.remove_contents().unwrap(); let lingering = locked .remove_directory_after(|| removed_by_another(&dir)) .unwrap(); @@ -567,7 +667,7 @@ fn a_removal_that_another_finishes_succeeds_and_keeps_what_replaced_the_director // A directory that replaces it then stays, even an empty one. let (_directory, dir) = sandbox(); let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); - locked.remove_contents(&sync_dir).unwrap(); + locked.remove_contents().unwrap(); let lingering = locked .remove_directory_after(|| { removed_by_another(&dir); @@ -1016,6 +1116,26 @@ mod windows { assert!(entries_of(&dir).is_empty()); } + #[test] + fn a_removal_takes_a_binding_whose_deletion_is_pending_for_removed_and_waits_until_it_goes() { + let (_directory, dir) = sandbox(); + fs::write(dir.join(CLONE_BINDING_NAME), b"{}").unwrap(); + fs::write(dir.join(LOG_NAME), b"log").unwrap(); + // A release removed the binding while a reader held it. + let pending = delete_pending(&dir.join(CLONE_BINDING_NAME)); + let mut lingering = remove(&dir) + .unwrap() + .expect("the binding whose deletion is pending keeps the directory"); + // Everything else went, and the binding lingers, as a removed lock file does, rather than + // being refused as an entry that is not the store's. + assert_eq!(entries_of(&dir), [CLONE_BINDING_NAME]); + let error = lingering.wait(Some(Instant::now())).unwrap_err(); + assert!(matches!(error, RemovalError::TimedOut(_)), "{error}"); + drop(pending); + lingering.wait(within(10)).unwrap(); + assert!(!dir.exists()); + } + #[test] fn locks_refuse_a_lock_file_that_refuses_access_for_good() { // A read-only lock file, and a directory at its path, refuse an open for writing as a diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 0ee64a06e..ea7a25930 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -194,24 +194,27 @@ //! [`LockedStore::remove`] removes a sandbox that no VMM can run in: it refuses while the store //! records a VM or a launch, which the caller reattaches to, or recovers, first. It removes the //! sandbox's [clone binding](CLONE_BINDING_NAME) first, and makes its absence durable before -//! anything else goes. It then removes the store's files and the caller's, the configuration -//! record last, and the temporary files that the store owns: those that an interrupted -//! replacement of one of its files leaves, `.NAME.NUMBER.tmp`, and the VMM's -//! [report staging file](#the-report-staging-file). It then refuses entries that are neither the -//! store's nor the caller's, naming them, and keeps the directory, or removes its lock file while -//! it still holds the lock, and then the directory. A removal needs the caller's configuration -//! record, whose absence tells the transitions that wait through it that the sandbox is gone: a -//! lock taken without one refuses it. +//! anything else goes. It checks for those records, and removes the binding, through one handle of +//! the directory, once it has checked through that handle that the directory holds its lock, so +//! that no change of a link in the path meanwhile makes it remove another sandbox's binding. It +//! then removes the store's files and the caller's, the configuration record last, and the +//! temporary files that the store owns: those that an interrupted replacement of one of its files +//! leaves, `.NAME.NUMBER.tmp`, and the VMM's [report staging file](#the-report-staging-file). It +//! then refuses entries that are neither the store's nor the caller's, naming them, and keeps the +//! directory, or removes its lock file while it still holds the lock, and then the directory. A +//! removal needs the caller's configuration record, whose absence tells the transitions that wait +//! through it that the sandbox is gone: a lock taken without one refuses it. //! //! Files that are still open can keep the emptied directory for a while: NFS renames a removed //! file that is still open, such as the lock file of a waiting transition, to `.nfs*`, Windows -//! file systems without POSIX deletion keep a removed lock file under its name until its last -//! handle closes, and a transition that opens the lock file after the removal removed it creates -//! a new one. Each such transition finds no sandbox, and removes its lock file as it releases its -//! lock. The removal leaves those files, then holds the directory's identity, releases its lock, -//! and returns a [`Lingering`] removal, which waits until they go, and reports a provision that -//! took the lock meanwhile as [`RemovalError::ProvisionedAgain`]. A removal removes the directory -//! only while the path still names it, which it checks before each attempt. +//! file systems without POSIX deletion keep a removed lock file, or a removed clone binding that a +//! reader holds, under its name until its last handle closes, and a transition that opens the lock +//! file after the removal removed it creates a new one. Each such transition finds no sandbox, and +//! removes its lock file as it releases its lock. The removal leaves those files, then holds the +//! directory's identity, releases its lock, and returns a [`Lingering`] removal, which waits until +//! they go, and reports a provision that took the lock meanwhile as +//! [`RemovalError::ProvisionedAgain`]. A removal removes the directory only while the path still +//! names it, which it checks before each attempt. mod caller; pub(crate) mod files; diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 9cab87827..c59a9a66c 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -17,6 +17,7 @@ use crate::host::template::store::tests::{ link_dir, link_file, publish, publish_with, remove, storage, store as new_store, unlink_dir, }; use crate::host::template::test_support::{ManifestSpec, artifact, network_class}; +use crate::host::test_support::released; use crate::protocol::CAPABILITY_LEN; /// A change to a record, as JSON. @@ -127,6 +128,14 @@ fn record_vm(sandbox: &Path) { write_private(&sandbox.join(RUNTIME_NAME), record.to_string().as_bytes()).unwrap(); } +/// Ends, through `locked`, a launch that spawned nothing, and whose log is closed. +/// +/// A process that a concurrent test spawned may still hold the log, until it executes its +/// program, and a later recovery then ends the launch. +fn recover_unspawned(locked: &mut LockedStore) { + assert!(released(|| locked.recover().unwrap() == Recovery::Exited)); +} + /// Returns the file at `path` as JSON. fn json_of(path: &Path) -> Value { serde_json::from_slice(&fs::read(path).unwrap()).unwrap() @@ -236,7 +245,7 @@ fn acquisitions_refuse_what_they_cannot_lease_and_lease_nothing() { let error = refusal(&store, generation, &sandbox, &now); assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); let mut locked = SandboxStore::lock_existing(&sandbox, &files(), None).unwrap(); - assert_eq!(locked.recover().unwrap(), Recovery::Exited); + recover_unspawned(&mut locked); record_vm(&sandbox); drop(locked); let error = refusal(&store, generation, &sandbox, &now); @@ -273,7 +282,7 @@ fn releases_end_leases_once_their_clones_have_exited() { ); let error = store.release(&mut acquired.locked).unwrap_err(); assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); - assert_eq!(acquired.locked.recover().unwrap(), Recovery::Exited); + recover_unspawned(&mut acquired.locked); record_vm(&sandbox); let error = store.release(&mut acquired.locked).unwrap_err(); assert_eq!(error.code(), ErrorCode::SandboxRunning, "{error}"); From 3fbaf8aeb78a0b8d610285461bb2f03a87e5e062 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 27 Oct 2026 08:04:31 -0700 Subject: [PATCH 8/8] nvx-rs: let the pending-deletion test run where an open can bypass the access list The Windows test of `was_removed` gives a file an access control list that allows no one anything, and asserted that `platform::file_identity`, whose open has no access rights but backup semantics, fails with a denied access. On GitHub's hosted Windows runners that open succeeds, most likely because the runner's elevated process holds the backup privilege, which lets an open with backup semantics bypass the list. The test failed there every time, at that setup assertion, before any of its assertions about removals. Check the open's error only where the open fails. Every assertion about which files were removed stays: a file that the list refuses is not removed, whether or not the open bypasses the list, and a file whose deletion is pending, or that is gone, is, but only a denied access tells so. No other test assumes that an open with backup semantics of a file that allows no one anything fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101 --- nvx-rs/src/host/template/store/leases/tests.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index c59a9a66c..56fb937cb 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -1615,7 +1615,8 @@ mod windows { } /// The access control list of a file that allows no one anything, which its owner may still - /// change: every open of the file fails, also one without access rights. + /// change: every open of the file fails, also one without access rights, unless the process's + /// privileges bypass the list. const NO_ACCESS: &str = "D:P"; /// The access control list of a file that allows its owner alone, as the store's files do. @@ -1629,9 +1630,12 @@ mod windows { let denied = io::Error::from(io::ErrorKind::PermissionDenied); assert!(!was_removed(&path, &denied)); // A file whose list refuses every open, also one without access rights, is not removed. + // Nor is it where the process's privileges bypass the list, as the backup privilege lets + // the open of `file_identity`, which has backup semantics, through. platform::set_access(&path, NO_ACCESS).unwrap(); - let error = platform::file_identity(&path).unwrap_err(); - assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + if let Err(error) = platform::file_identity(&path) { + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + } assert!(!was_removed(&path, &denied)); platform::set_access(&path, OWNER_ONLY).unwrap(); // One whose deletion is pending is, as is one that is gone, but only a denied access tells