Repository navigation
cli: serialize sandbox exec admission across CLI processes (SL3.9) - #1469
Draft
Enrique Saurez (esaurez) wants to merge 1 commit into
Draft
Enrique Saurez (esaurez) wants to merge 1 commit into
Enrique Saurez (esaurez) wants to merge 1 commit into
Conversation
Enrique Saurez (esaurez)
added
tests-modified
Contributor
This pull request changes existing testsIt modifies or removes 16 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests. Test code
End-to-end tests
Relocated unchanged: 0; added: 33. Compared the merge base |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A retry can launch a workload after the --wait admission deadline expires, and missing --wait documentation causes the existing usage validation to fail when NVX_RS is set.
2 open issues
What changed in this PR
Implements SL3.9’s cross-process serialization of sandbox exec using a persistent per-sandbox file lock, while allowing Stop to proceed independently.
Changes:
- Adds exec admission locking and a
--waitoption with a 60-second default. - Integrates the persistent lock into state ownership and deprovision cleanup, including Windows delete-pending handling.
- Adds admission and cleanup tests and updates existing file-inventory expectations.
| File | Description |
|---|---|
nvx-rs/tests/cli/usage.rs |
Checks that sandbox help includes --wait. |
nvx-rs/tests/cli/lifecycle.rs |
Updates persistent-lock inventories and exposes a Windows test helper. |
nvx-rs/tests/cli/exec_lock.rs |
Adds cross-process admission, release, Stop, and cleanup tests. |
nvx-rs/tests/cli.rs |
Registers admission tests and updates state inventories. |
nvx-rs/src/host/cli/store/names.rs |
Registers exec.lock without accepting temporary lookalikes. |
nvx-rs/src/host/cli/store/mod.rs |
Documents the lock and re-exports its opening helper. |
nvx-rs/src/host/cli/store/locked.rs |
Makes the existing lock-opening helper reusable. |
nvx-rs/src/host/cli/sandbox/tests.rs |
Tests --wait defaults and numeric validation. |
nvx-rs/src/host/cli/sandbox/options.rs |
Defines, parses, and describes --wait. |
nvx-rs/src/host/cli/sandbox/mod.rs |
Passes the admission timeout to managed exec. |
nvx-rs/src/host/cli/sandbox/lock/tests.rs |
Tests owned-lock cleanup and foreign-file preservation. |
nvx-rs/src/host/cli/sandbox/lock.rs |
Handles confirmed delete-pending exec locks during cleanup. |
nvx-rs/src/host/cli/sandbox/lifecycle/start.rs |
Adds admission retries and holds the guard through execution. |
nvx-rs/src/host/cli/mod.rs |
Registers the exec-lock module. |
nvx-rs/src/host/cli/exec_lock/tests.rs |
Tests lock identity, privacy, contention, and persistence. |
nvx-rs/src/host/cli/exec_lock.rs |
Implements the nonblocking OS lock and release guard. |
Review effort: Balanced
Comment on lines
284
to
+286
| let mut lock = LifecycleLock::acquire(&state_dir, response_timeout, false)?; | ||
| running_vm(&state_dir, &mut lock, artifacts)? | ||
| require_config(&state_dir)?; | ||
| if let Some(exec) = ExecLock::try_acquire(lock.locked())? { |
| --timeout TIMEOUT control operation timeout, and the longest wait for | ||
| another lifecycle transition of the sandbox, in | ||
| seconds (default: 60) | ||
| --wait WAIT wait for another sandbox exec, in seconds (default: 60) |
Pedro Henrique Penna (ppenna)
force-pushed
the
esaurez/sl35b-lifecycle-4d48405a-20261110
branch
from
November 11, 2026 03:57
369ad2d to
5b7882a
Compare
Pedro Henrique Penna (ppenna)
force-pushed
the
esaurez/sl39-exec-lock-cd47-20261110
branch
from
November 11, 2026 08:01
9715f91 to
9ddd53d
Compare
Pedro Henrique Penna (ppenna)
force-pushed
the
esaurez/sl35b-lifecycle-4d48405a-20261110
branch
from
November 11, 2026 08:02
d9c518e to
bcaaa60
Compare
Pedro Henrique Penna (ppenna)
force-pushed
the
esaurez/sl39-exec-lock-cd47-20261110
branch
from
November 11, 2026 08:13
9ddd53d to
9ca2a26
Compare
Base automatically changed from
esaurez/sl35b-lifecycle-4d48405a-20261110
to
feature/nvx-refactor
November 11, 2026 08:15
Concurrent nvx-rs processes could run two `sandbox exec` workloads in one sandbox at once (#819). An exec now takes a per-sandbox OS file lock, `exec.lock` in the state directory, which the new `host::cli::exec_lock` module owns, and holds it until its workload ends. Under the lifecycle lock, after it checks the configuration, it opens the lock file as the store opens its own, without following links and refusing one that is not a private regular file, tries the lock without waiting, and checks that the directory entry still names the locked file. An exec that finds the lock held releases the lifecycle lock, pauses for the store's retry interval and tries again: once at least, and only until `--wait` seconds, 60 by default, have passed, even when a retry waited past them for the lifecycle lock. It then fails with `sandbox-busy`. `--timeout` still bounds each wait for the lifecycle lock, and `stop` takes no exec lock, so it proceeds while a workload runs. The lock file stays, as `sandbox.lock` does, so that no release unlinks a file that another exec may hold open. `deprovision` removes it with the store's files, also when a file system without POSIX deletion keeps it delete-pending after the VM exited. It is the exec lock module's own file rather than one of the store's, so the store's temporary names and snapshot capture's owned files do not change. The docs describe `--wait`, the exec lock and the new cause of `sandbox-busy`, and the sandbox acceptance expects the lock in the state directory from the first exec on. Tests: tests/cli/exec_lock.rs runs CLI processes against the fake OpenVMM: a held lock makes a second exec fail with `sandbox-busy` after `--wait`, while other sandboxes and `stop` proceed; results, errors and cancellation release it; a retry that waits for the lifecycle lock past the deadline is busy; a removed configuration or a replaced lock file refuses admission; and deprovision waits for a delete-pending lock on Windows. Unit tests cover the lock's privacy and identity, the `--wait` default and number rules, and deprovision's removal of the file, and the exact state listings of existing tests include the lock. Part of #1111 (SL3.9). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
esaurez/sl39-exec-lock-cd47-20261110
branch
from
November 11, 2026 08:28
9ca2a26 to
120f2b0
Compare
Pedro Henrique Penna (ppenna)
changed the title
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.
Part of #1111 (SL3.9: one exec runs at a time per sandbox, across CLI processes).
What and why
Concurrent
nvx-rsprocesses could run twosandbox execworkloads in one sandbox at once (#819). This PR does SL3.9's Scope and no more:host::cli::exec_lock(new) owns one OS file lock per sandbox,exec.lockin its state directory. Under the lifecycle lock, after the configuration check, an exec opens it as the store opens its own lock file (no link followed, a private regular file), tries the lock without waiting, and checks that the directory entry still names the locked file. The exec holds it through connect andSession::exec; the lifecycle lock is released before connecting, as before.sandbox exec --wait SECONDS(60 by default): an exec that finds the lock held releases the lifecycle lock, pauses for the store's 25 ms retry interval and tries again — once at least, and only until--waithas passed, even when a retry waited past it for the lifecycle lock (--timeoutstill bounds that wait) — then fails withsandbox-busy.sandbox stoptakes no exec lock, and proceeds while a workload runs.sandbox.lockdoes, so that no release unlinks a file that another exec has open;deprovisionremoves it, also when a Windows file system without POSIX deletion keeps it delete-pending after the VM exited. It is the exec lock module's own file, not aStateFile, so the store's temporary-name rules and snapshot capture's owned files do not change.doc/usage.md(--wait,sandbox-busy),doc/run.md(lifecycle rules, sandbox-lifecycle acceptance),nvx-rs/README.md(state files, exec rules, error codes), andErrorCode::SandboxBusy's rustdoc. The sandbox acceptance (scripts/nvx_tools/sandbox_lifecycle_tests.py) expectsexec.lockfrom a sandbox's first exec until deprovision.Not in this PR (per #1111): a queue, daemon or fairness; more than one exec per sandbox or a global lock; stop taking the lock; other flags.
Takeover. Enrique Saurez (@esaurez)'s Draft (9 commits on an earlier head of #1460) was replayed onto
feature/nvx-refactor, now that #1460 (SL3.5b) has merged, and squashed into one commit that keeps his authorship, with: the deadline recheck and its regression test (review thread), the--waitdocs (review thread), the acceptance update, which the earlier commits lacked (the public sandbox scenarios check exact state listings), andexec.lockas the module's own file instead ofStateFile::ExecLock, which had made snapshot capture's recovery treat a foreignexec.lockas its own.Size. 82 core lines (estimate 60, stop 120). 6 counted files:
host/cli/exec_lock.rs(new, 39),sandbox/lifecycle/start.rs(25),sandbox/lock.rs(7),sandbox/options.rs(7),cli/mod.rs(1),sandbox/mod.rs(1). Not counted:store/locked.rs, which only makesopen_lockcrate-private (1 line),store/mod.rs, with the re-export that this forces and one doc line, anderror.rs, rustdoc only.BOOT_CONTRACT_VERSION,CONTROL_CONTRACT_REVISIONand the public API are unchanged.Validation
cargo fmt --check; Clippy-D warnings:--all-targets --all-features,--lib,--lib --no-default-features,--lib --bins --features cli,--lib --bins --no-default-features --features testingcargo test --all-features --test cliexec_lockcases pass; see the note below on load-sensitive timing testscargo test --all-features/cargo test/--lib --no-default-featuresguest::failures are the same set at the base on that host (isolated child processes of that environment). Windows: see notecargo docwith-D warnings, MSRV 1.89 check, macOS check,python3 scripts/public_api.py, runtime adapter protocol-only and host-only fmt, Clippy, tests and MSRVexec_lock::a_retry_that_waits_for_the_lifecycle_lock_past_the_deadline_is_busycheck_test_layout.py,check_markdown_links.py, Ruff check and format, Pyright (Linux and Windows)python -m unittest scripts/test_microvm_tests.py -k PublicSandbox -k PublicManagedExec;scripts.test_nvx_toolstest_usage_documents_every_option_of_nvx_rswithNVX_RSsetnvx.py test-microvm --scenario managed-exec-config / sandbox-lifecycle / sandbox-proxy-only / sandbox-ram-upper, KVM, nvx-kvm-test (Azure Linux 3, virtual machine)The scenarios used the guest artifacts that #1460's CI built (run 42979403236), OpenVMM
14dc4116from CI, andnvx-rsbuilt from this commit.How the deadline regression synchronizes. The race that it checks needs an exec whose retry waits for the lifecycle lock until after its
--waitdeadline, while the exec lock becomes free. The test holds the exec lock itself and starts an exec with--wait 4. That exec holds the lifecycle lock only while it tries the exec lock, so the test polls the lifecycle lock until it finds one of these tries, and then takes the lock. When it took it within the first half of--wait, the exec's deadline, which lies at least--waitafter the spawn, is still at least 2 s away, so the exec sleeps one 25 ms retry interval and tries again, and that try waits for the lifecycle lock: this is the precondition. Such a round then has no way out but its assertions: it holds the lifecycle lock until the deadline has surely passed, fails unless the exec is still waiting, frees the exec lock, releases the lifecycle lock, and requiressandbox-busywith the workload not run. Rounds repeat only to reach the precondition: a round that misses every try, or finds one too late, releases the lifecycle lock and requires thesandbox-busyof an exec that waited out--waitbefore another starts, and the test fails if 5 rounds never reach it. No outcome is retried.Note: on the loaded Windows developer machine, a few timing-sensitive tests in modules that this PR does not touch failed under load and passed alone (
one_shot::a_ready_console_stream_is_not_throttled_by_idle_sleeps,host::template::clone::tests::restore::the_restore_to_ready_deadline_runs_from_the_launch_through_every_step,host::platform::windows::tests::children_inherit_only_their_standard_handles);host::cli::snapshots::capture::tests::a_capture_ack_and_successful_output_without_timely_exit_are_stopped_but_never_finishedfails the same way at the base in the same batch. The newexec_processes_contend_but_other_sandboxes_and_stop_proceedbounds its refused exec and its stop by 30 s, which still rules out the 60-second default and the minute-long workload, since a refused exec took up to 7 s there under load.Left to CI: MSHV, the guest-binary checks (no guest code changes), the adapter's guest-only configuration, and the remaining
scripts/test_*.pymodules.Changes to existing tests (
tests-modified)check_test_changes.py --base origin/feature/nvx-refactor: 16 changes, 33 added tests, none relocated. Every other test of the area is kept.cli/lifecycle:check_lifecycle,lifecycle_of_a_sandbox_whose_upper_is_in_ram,stops_keep_an_outcome_report_that_the_caller_reads_whatever_its_umask,managed_execs_exit_with_the_workload_status— their exact state listings includeexec.lockafter the sandbox's first exec (with_exec).refusals_of_a_running_sandbox— a refusedexecof a stopped sandbox leavesexec.lock, since admission creates it before it finds no VM; the listing afterstopis unchanged.cli:a_restarted_cli_drives_only_a_vm_of_its_runtime_identity,a_sandbox_whose_openvmm_exited_reports_it_once_and_starts_again— their exact listings include theexec.lockof their earlier execs.cli/usage:HELP—sandbox exec --helplists--wait WAIT.host::cli::sandbox::lock::tests:removals_refuse_live_records_and_keep_foreign_files— deprovision also removes the ownedexec.lockand keepsexec.lock.foreign,.exec.lock.42.tmp,.exec.lock.x.tmpand.exec.lock.42.tmp.moreas foreign;removal_cleans_owned_temporaries_without_touching_a_neighbour—exec.lockis among the owned files that removal deletes.scripts/nvx_tools/sandbox_lifecycle_tests.py:LifecycleAcceptance.__init__,invoke,require_entries— the acceptance expectsexec.lockin a state directory from an exec of the provisioned sandbox until deprovision removes the configuration, in the lifecycle, proxy-only and RAM-upper scenarios.scripts/test_microvm_tests.py:_FakeSandboxCli._dispatch,_deprovision— the fake CLI createsexec.lockfor an exec of a provisioned sandbox and deprovision removes it;test_acceptance_detects_lifecycle_regressions— the newexec-skips-lockfault must fail the acceptance.