fix(serve): close serve races and translate GPU selection through visibility masks (EAI-7194) - #267
Open
r0x0r wants to merge 3 commits into
Open
fix(serve): close serve races and translate GPU selection through visibility masks (EAI-7194)#267r0x0r wants to merge 3 commits into
r0x0r wants to merge 3 commits into
Conversation
r0x0r
force-pushed
the
fix-race-conditions
branch
from
August 18, 2026 11:01
599630c to
4a1e4c3
Compare
…ces (EAI-7194) Three concurrency races existed in the serving path: 1. Daemon autostart was a check-then-spawn TOCTOU: two concurrent callers could both read "not running" and each spawn a background daemon. 2. GPU --gpu auto read the busy-GPU list without a lock, so two concurrent serves could select the same idle GPU. 3. The read-select-launch sequence for managed serving was not atomic: a second serve could target the same GPU between the select and the claim. Add a cross-process advisory lock primitive (rocm_core::FileLock, backed by std File::lock) and two lock files: - automation autostart lock: ensure_background_helper_running_quiet now re-checks liveness under the lock before spawning, so only the first holder starts the daemon. - managed-launch lock: serve() holds it across GPU auto-selection and the claiming service-record write (dropped before the readiness wait / attached stream), making select-then-claim atomic so concurrent serves land on distinct GPUs. Tests: FileLock mutual-exclusion / parent-dir / independence unit tests, and a launch_lock_makes_gpu_select_and_claim_atomic regression that races two select-then-claim sequences and asserts distinct GPUs. The behavior only diverges under concurrent invocation, which the single-process cucumber e2e harness cannot exercise deterministically, so no Gherkin scenario is added. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>
…EAI-7194) Complete the visible-ordinal-correctness half of EAI-7194: `rocm serve` GPU selection and `--gpu` validation are now interpreted against the devices visible under the active HIP_VISIBLE_DEVICES/ROCR_VISIBLE_DEVICES mask, not the raw amd-smi ordinal space. - serve() computes the visible device set once via usable_amd_gpu_indices() and threads it through resolve_gpu_indices, reusing it for both the no-usable-GPU fail-fast and selection. - auto-select restricts candidates to the visible set and, when every visible device is busy, falls back to the lowest VISIBLE ordinal instead of a hardcoded 0. - an explicit `--gpu N` that is in range but hidden by the active mask is now rejected up front with an actionable message, rather than being silently reinterpreted by the runtime onto a different device. - narrow the ROCR advisory to describe the mask-relative interpretation and correct the launch-lock critical-section comment. Full ROCR physical->HIP export translation is intentionally deferred: it is a cross-engine contract change (rocm-engine-protocol::apply_gpu_visibility and every engine) on the GPU-required path that cannot be verified without multi-GPU hardware under an active ROCR mask, where a wrong translation would misroute serves. The narrowed advisory remains until then. Tests: mask-aware auto-select/validation unit tests; launch-lock regression now drives resolve_gpu_indices; e2e Scenario 15 asserts a mask-hidden `--gpu` is refused (exercised on the multi-GPU e2e-gpu lane). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>
Run `cargo fmt --all` over the two preceding EAI-7194 commits so the prek `cargo fmt --all --check` hygiene hook passes. No functional change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>
r0x0r
force-pushed
the
fix-race-conditions
branch
from
August 21, 2026 12:29
4a1e4c3 to
e6fb3bd
Compare
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
1 participant
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.
Summary
Closes the concurrency races in
rocm serveand completes the visible-ordinal-correctness half of EAI-7194, so GPU selection is honest under an active visibility mask. The branch is two focused commits:66e6ee5— close daemon-autostart, GPU-select, and managed-launch races. Adds aFileLockprimitive plus two lock files so concurrent serves cannot autostart the daemon twice, race the GPU-selection read, or interleave the managed-launch record write.ccff388— translate GPU selection through active visibility masks.serveGPU selection and--gpuvalidation are now interpreted against the devices visible under the activeHIP_VISIBLE_DEVICES/ROCR_VISIBLE_DEVICESmask instead of the raw amd-smi ordinal space.What changed (visible-ordinal correctness)
serve()computes the visible device set once viausable_amd_gpu_indices()and threads it throughresolve_gpu_indices, reusing it for both the no-usable-GPU fail-fast and selection.0.--gpu Nthat is in range but hidden by the active mask is now rejected up front with an actionable message, rather than being silently reinterpreted by the runtime onto a different device.Deferred (intentional)
Full ROCR physical→HIP export translation is not included. That is a cross-engine contract change (
rocm-engine-protocol::apply_gpu_visibilityand every engine) on the GPU-required path that cannot be verified without multi-GPU hardware under an active ROCR mask, where a wrong translation would misroute serves to the wrong device. The narrowed advisory remains until that work can be done and verified on hardware.Testing
--gpumask rejection.resolve_gpu_indicesentry pointserve()uses.--gpuis refused. It is exercised on the multi-GPU e2e-gpu lane (on single-GPU lanes the same request is out of range — both are honest refusals); the no-GPU mock lane skips it via@requires-gpu.cargo test -p rocm(458/458 serial),cargo clippy --workspace --all-targets -D warningsclean,scripts/smoke_local.pyok.GPU-hardware verification of Scenario 15 will run on the e2e-gpu lane in CI.