Repository navigation
nvx-rs: reserve the launch of a leased clone, and document the lock order - #886
Open
Enrique Saurez (esaurez) wants to merge 2 commits into
Open
Enrique Saurez (esaurez) wants to merge 2 commits into
Enrique Saurez (esaurez) wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The cross-platform crash-recovery guarantees and breaking lifecycle APIs warrant final human review, despite no concrete defects identified.
No open issues 🎉
What changed in this PR
This PR completes the supervisor and lock-order portion of #839, building on #885’s template leases. It ties each lease to one reserved clone launch and strengthens protection against stale VMM handles.
Changes:
- Accept caller-reserved launch identities and restrict each lease to one launch under the acquiring lock.
- Preserve another VM’s runtime record when stopping through a stale handle.
- Document lock ordering and add tests for clone-launch crashes, recovery, and lease release.
| File | Description |
|---|---|
tests/runtime-adapter/src/bin/runtime-adapter-host.rs |
Reserve an identity before launching through the supervisor. |
nvx-rs/tests/supervisor.rs |
Update supervisor fixtures for explicit launch identities. |
nvx-rs/tests/store.rs |
Update store integration tests for explicit launch identities. |
nvx-rs/src/host/template/store/leases/tests.rs |
Test launch reservations and crashes across acquisition, launch, and release. |
nvx-rs/src/host/template/store/leases.rs |
Persist reserved launch identities and manage the acquiring lock’s grant. |
nvx-rs/src/host/template/store.rs |
Document reservations and add a shared test publication helper. |
nvx-rs/src/host/supervisor/tests.rs |
Test exit evidence, failed teardown, and stale-handle protection. |
nvx-rs/src/host/supervisor/mod.rs |
Accept reserved identities, compare full runtime records, and document lock order. |
nvx-rs/src/host/store/tests.rs |
Adapt store unit tests to caller-supplied identities. |
nvx-rs/src/host/store/record.rs |
Make LaunchId::generate public and document single-launch use. |
nvx-rs/src/host/store/mod.rs |
Enforce lease reservations and consume the grant when launch begins. |
nvx-rs/src/host/instance.rs |
Generate and pass an identity for instance launches. |
nvx-rs/src/host/cli/sandbox/lifecycle.rs |
Generate and pass an identity for CLI sandbox launches. |
nvx-rs/src/error.rs |
Expand sandbox-leased documentation to cover launch restrictions. |
nvx-rs/README.md |
Describe reserved launches, lock ordering, and lease restrictions. |
doc/usage.md |
Update the documented meaning of sandbox-leased. |
aci_edge_sandboxes/tests/openvmm_fake.rs |
Adapt fake-VMM tests to the changed launch API. |
Review effort: Balanced
Enrique Saurez (esaurez)
force-pushed
the
esaurez/839-pr4-supervisor-seam-20261024
branch
from
October 25, 2026 00:21
2fe2f26 to
84aec9b
Compare
This was referenced Oct 25, 2026
Enrique Saurez (esaurez)
force-pushed
the
esaurez/839-pr3-leases-20261024
branch
from
October 27, 2026 02:56
780ab5f to
4a8b534
Compare
Enrique Saurez (esaurez)
force-pushed
the
esaurez/839-pr4-supervisor-seam-20261024
branch
from
October 27, 2026 02:56
26b78dc to
b7ac52f
Compare
This was referenced Oct 27, 2026
…rder A clone's lease now reserves the launch of its clone. The acquisition draws the launch's identity, records it in the lease's record and in the clone binding before any VMM of the clone exists, and returns it as Lease::launch. The supervisor takes the launch's identity from its caller: supervisor::launch and LockedStore::begin_launch take a LaunchId, which callers that launch no clone draw with LaunchId::generate, now public. A lease allows one launch of its clone, under the lock that acquired it: begin_launch refuses, with sandbox-leased, any other launch of a sandbox that holds a lease, a launch under another lock, and a second launch, until a release ends the lease. After a crash before the launch, a release ends the lease at once, and another acquisition checks the generation's eligibility again. A stop or a termination clears the runtime record only if it is still the record of its own VM, and not another VMM's under a launch identity that its caller used again. The supervisor's documentation describes the launch of a clone, its crashes, and one lock order: the template store's lock, then the sandbox's lifecycle lock, which guards its clone binding and launch journal, then the scratch image's claim. No lock lasts for a VM's lifetime. Tests crash a clone's orchestrator, in a process of its own, at each step of an acquisition and of a release, and around the spawn of the clone's VMM, and check what each evidence of a VM's exit means for its lease: an earlier boot, a reused process ID, a process that cannot be examined, and a forced teardown that fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101
Four lease tests began a launch, dropped it, and asserted that the next recovery ends it. On Linux, a process that a concurrent test forks holds a copy of the closed log until it executes its program. If that outlasts recovery's own wait, the recovery is indeterminate and the test fails although nothing is wrong. The tests now retry the recovery until it ends the launch, through test_support::released, as the sandbox store's tests do. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc335726-838c-4538-a3c4-d6791aba5101
Enrique Saurez (esaurez)
force-pushed
the
esaurez/839-pr3-leases-20261024
branch
from
October 27, 2026 04:39
4a8b534 to
c2aa75a
Compare
Enrique Saurez (esaurez)
force-pushed
the
esaurez/839-pr4-supervisor-seam-20261024
branch
from
October 27, 2026 04:39
b7ac52f to
061e368
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Stacked on #885; this PR retargets to dev when the pull requests below it merge. The chain on
devis #883 → #876 → #871 → #873 → #874 → #879 → #881 → #885 → #886, after #863, #864, #865, #866, #867, #868, #869, #870, #872, #875, #878, and #882, which have merged.A clone's lease must outlive every process that may restore its generation, through a crash of the orchestrator at any step of the clone's launch, and a crash must never leave a VMM that no record names. #885 writes the lease, and the clone binding beside the sandbox's launch journal, before any VMM of the clone exists; this ties the lease to the launch that it reserves, and lets the supervisor launch under an identity that its caller reserved. It also documents one lock order for the template store, the sandbox, and the scratch image, and tests the crashes of a clone's orchestrator at each step, in processes of their own.
Part of #839: the last of its four pull requests. The first, #879, adds the compatibility key, the second, #881, the store, and the third, #885, eligibility, leases, tombstones, and retirement.
What changed
The supervisor seam (
host/supervisor,host/store).supervisor::launchandLockedStore::begin_launchtake the launch's identity from their caller, who reserves it before the launch begins, withLaunchId::generate, now public, a fresh one for each launch. The CLI,Instance, and the runtime adapter draw one just before they launch, so their launches do as before.A lease reserves its clone's launch (
template/store/leases.rs).Store::acquiredraws the identity of the clone's launch, and records it, in the lease's record and in the clone binding, before it returns, so that the reservation is durable before any VMM of the clone exists.Lease::launchreturns it. A lease allows one launch of its clone, under the lock that acquired it, whatever the launch's outcome: while a sandbox holds a lease,begin_launchrefuses, withsandbox-leased, a launch under another identity, the reserved launch under another lock, and a second launch. The lock thatacquirereturns holds the grant in memory, and the launch's journal consumes it. A release ends the lease once the clone's VM has exited, and another acquisition, which checks the generation's eligibility again, reserves another launch.Stale handles. Once the VMM exits,
Vmm::stopandVmm::terminateclear the runtime record only if it is still the VM's own record, field for field. A record that named the same launch used to suffice, but callers now choose launch identities and could use one again, so a stale handle could have cleared the record of another VM.The lock order (
supervisormodule documentation, "Clones"). Transitions take their locks in one order: the template store's lock, then the sandbox's lifecycle lock, which guards both its clone binding and its launch journal, then the scratch image's claim. None waits for a lock while it holds a later one: an acquisition releases the store's lock before it returns, a release, a launch, a stop, and a reattach take no lock of the store, and a retirement takes no lock of a sandbox. No lock lasts for a VM's lifetime: the caller releases the sandbox's lock once the launch commits or ends, and the scratch image's claim belongs to the VMM. The documentation also lists what a crash at each step leaves, and which evidence of a VM's exit ends its lease.Public API changes to review
LaunchId::generateis public.LockedStore::begin_launch(&mut self, launch: &LaunchId, runtime, capability)andsupervisor::launch(locked, reserved: &LaunchId, runtime, capability, command)take the launch's identity: a breaking change for their callers.template::Lease::launch.launchfield.STORE_FORMATstays 1: nothing has released the records that nvx-rs: lease eligible templates to clones, and retire them #885 writes, which lack it.sandbox-leasedalso refuses a launch that a sandbox's lease does not allow. Its documentation, innvx::ErrorCode, the README, anddoc/usage.md, says so.Assumptions to review
acquirereturns, so the caller keeps that lock from the acquisition to the launch, and claims the clone's scratch image under it. A crash in between leaves a lease that launches nothing, which a release ends at once.Validation
At
061e368, the head, restacked on #885'sc2aa75a, on Windows and on Linux x86_64:cargo fmt --check;cargo clippywith--all-targets --all-featuresand--lib --no-default-features,-D warnings;cargo testwith all, default, and no features;cargo docwith-D warnings, also of private items on Windows;cargo +1.89 check;tests/openvmm_vectors.rswith--ignored, against the OpenVMM submodule at its pin,a12d916;supervisor::launch, with their MSRV checks, and, on Linux, its guest-only configuration;aci_edge_sandboxes, which builds onnvx:cargo fmt --check,cargo clippywith all features and without default features,cargo testwith all and default features, the agent's bundling,cargo doc, and its MSRV check, and, on Linux, its macOS check;nvx, against a synthetic bundle;scripts/test_nvx_tools.py's check thatdoc/usage.mddocuments every error code;cargo fmt --check,cargo clippywith--all-targets --all-featuresand--lib --no-default-features,-D warnings, the runtime adapter's host-only Clippy, andaci_edge_sandboxes's Clippy with all features, and, on Linux, the macOS check and the runtime adapter's guest-only Clippy.The lease tests check that a lease allows one launch of its clone, under the lock that acquired it, and that its records keep the reserved launch. The crash tests each run a clone's orchestrator in a process of its own, which exits at one step: after the lease's record, after the binding, once acquired, before the VMM's spawn, after it, after the VMM's identity is recorded, once the launch commits, and while it releases. Each test then checks what a release, a reattach, and a retirement do with what the crash left. The lease lasts while the clone's VMM may run, even one that the journal does not name yet, and a release ends it once that VMM has exited. The supervisor's tests check the evidence of a leased clone's exit: an earlier boot and a reused process ID end the lease; a process that cannot be examined keeps it; and so does a forced teardown that fails, until a teardown ends the VMM. They also check that a stop keeps the record of another VMM under the same launch identity.
Every check passed on both hosts.
A final review by a separate model found no significant issues. It reviewed this change before its restack on #885's latest commits. The restack changed none of this change's code: it merged three of its documentation paragraphs with #885's new text, and the documentation of a clone's release now says that the release takes the sandbox's lock through the lease's path, as #885 requires. A later restack, onto #885's
a7e68ad, which makes a removal of a sandbox remove its clone binding first and durably, merged one more paragraph, the documentation ofLease, and changed no code either.Copilot's review then found that four lease tests began a launch, dropped it, and asserted that the next recovery ends it at once. On Linux, a process that a concurrent test forks holds a copy of the closed log until it executes its program, so such a recovery can be indeterminate.
26b78dcretries those recoveries until they end the launch, throughtest_support::released, as the sandbox store's tests do. Two of the four tests come from #885, whose versions of them have the same pattern.The restack on #874, through #879, #881, and #885, resolved one conflict, in the runtime adapter's host, whose launch
devchanged meanwhile. The host keepsdev's extendable deadline and its passing of the prepared launch's checked files to OpenVMM, and launches under the reserved identity through a named command closure. That closure's command now borrows the prepared launch, so its return type names an inferred lifetime rather than'static: the only change to code. The second commit now changes only this pull request's two lease tests, since #885 makes the same change to its own. A final review of the restack by a separate model found no issues.A second restack, on #874's
fa45e48, which drops the configuration identity of activation records, through #879, #881, and #885, replayed this pull request's commits without conflicts, and changed no code. A final review of it by a separate model found no issues.NVX CI does not run until this pull request targets
dev.