Repository navigation
nvx-rs: let launch tests wait for a log that a test's child holds - #786
Merged
Pedro Henrique Penna (ppenna) merged 2 commits intoOct 23, 2026
Merged
Conversation
…ld holds A process that a concurrent test spawns holds the test's descriptors from its fork until it executes its program, and with them the claim of a launch's log that the test closed, for longer than the claim release wait if the machine is loaded. A recovery, an abandon, or the check of a recorded VMM that cannot be held then ends as indeterminate. an_abandoned_launch_that_spawned_nothing_has_exited and a_recorded_vmm_that_cannot_be_held_has_exited_once_its_log_is_released expected Exited at once. Accept an indeterminate abandon, and wait with released() for a later recovery or check to end the launch, as c32b50e does for the staging test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bba527e3-2680-407f-ba91-a21eeb2ea18a
unrecorded_launches_end_once_their_log_is_released starts the sandbox once it releases the log of a launch that it began itself. A process that a concurrent test spawned while the log was open holds it until it executes its program, for longer than the claim release wait if the machine is loaded, and the start then fails as indeterminate, with BackendError. Retry such a start for up to 10 seconds, as a caller retries an indeterminate recovery, and fail on any other error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bba527e3-2680-407f-ba91-a21eeb2ea18a
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused test changes accommodate temporary log ownership while preserving the required recovery and lifecycle outcomes.
No open issues 🎉
What changed in this PR
This test-only follow-up to #754 reduces launch-recovery flakiness when children spawned by concurrent tests temporarily retain a log claim. Production behavior is unchanged.
Changes:
- Store tests use
released()to wait for recovery or exit checks after the log is closed. - The fake OpenVMM test retries starts returning
BackendErrorfor up to 10 seconds, while preserving cleanup and workload assertions.
| File | Description |
|---|---|
nvx-rs/src/host/store/tests.rs |
Allows delayed log-claim release in abandon and recorded-VMM tests. |
aci_edge_sandboxes/tests/openvmm_fake.rs |
Adds bounded start retries after releasing an unrecorded launch’s log. |
Review effort: Balanced
Enrique Saurez (esaurez)
marked this pull request as ready for review
October 23, 2026 05:31
Pedro Henrique Penna (ppenna)
deleted the
esaurez/a1-log-claim-test-waits-20261023
branch
October 23, 2026 13:15
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.
Follow-up to #754.
A process that a concurrent test spawns holds the test process's descriptors from its fork until it executes its program, and with them the claim of a launch's log that a test closed. The store waits up to
CLAIM_RELEASE_WAIT, one second (#656), for such a claim to be released before it reports a launch as indeterminate, which covers that moment unless the machine is loaded. #754 let its staging test wait for a later recovery in that case (c32b50e). Three other tests still expect a launch to end at once:an_abandoned_launch_that_spawned_nothing_has_exited(nvx-rs: add the sandbox store, launch journal, and runtime identity #628,nvx-rs/src/host/store/tests.rs): its recovery after a refused commit, and its abandon.a_recorded_vmm_that_cannot_be_held_has_exited_once_its_log_is_released(nvx-rs: add the sandbox store, launch journal, and runtime identity #628, same file): its checks of a recorded VMM that cannot be held, once the test releases the log.unrecorded_launches_end_once_their_log_is_released(nvx-rs: move host::instance onto the store and supervisor #721,aci_edge_sandboxes/tests/openvmm_fake.rs): its start, once the test releases the log.Changes
released()for a later recovery, or a later check, to end the launch, as the staging test does.aci_edge_sandboxestest retries a start that fails withBackendErrorfor up to 10 seconds, as a caller retries an indeterminate recovery, and fails on any other error.Only tests change.
Reproduction
On Linux, temporary tests, which this PR does not add, spawned from another thread a child that sleeps three seconds between its fork and its exec, while a launch's log was open:
abandon()returnedIndeterminate("a process holds its log, and no process serves its control endpoint yet"), wherean_abandoned_launch_that_spawned_nothing_has_exitedexpectedExited.released()saw the next recovery end the launch 1.7 seconds later.end_identified()returnedIndeterminate("a process still holds its log, and no process matches its VMM process 42"), wherea_recorded_vmm_that_cannot_be_held_has_exited_once_its_log_is_releasedexpectedExited.released()saw it end the launch 1.7 seconds later.Validation
At this branch's head, on
dev's6b6e1215:nvx-rs, withNVX_BUNDLE=skip:cargo fmt --all --check;cargo clippy --locked --all-targets --all-features -- -D warningsandcargo clippy --locked --lib --no-default-features -- -D warnings;cargo test --locked --all-features, thencargo test --locked;cargo doc --locked --no-deps --all-featureswithRUSTDOCFLAGS=-D warnings;cargo +1.89 check --locked --all-targets --all-features.aci_edge_sandboxes: the same commands with--workspace, andpython scripts/nvx.py test-agent-bundling --toolchain 1.95.0.tests/runtime-adapter: the protocol-only and host-only configurations, as check-nvx-rs runs them.nvx-rsandaci_edge_sandboxescommands, and both macOScargo check --target aarch64-apple-darwinruns.