-
Notifications
You must be signed in to change notification settings - Fork 154
Fixes a race condition in killing Sandboxes #959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
96a6fce to
d200c7c
Compare
4e211f2 to
4de305a
Compare
cf14cfd to
f1aef3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a race condition where a sandbox kill could arrive after a guest call completes, incorrectly causing the next guest call to be canceled. The fix introduces generation tracking to ensure kill() only affects the intended guest call.
Key changes:
- Generation tracking prevents stale kills from affecting new guest calls
- Added
call_activeflag to prevent "kill-in-advance" scenario - Breaking change: kill() now only affects actively running guest calls, not future ones
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/rust_guests/simpleguest/src/main.rs | Adds SpinForMs guest function for timing-sensitive tests |
| src/hyperlight_host/tests/integration_test.rs | Updates interrupt tests to reflect new behavior and adds comprehensive stress test |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Introduces CallActiveGuard RAII guard to manage call_active flag |
| src/hyperlight_host/src/mem/shared_mem.rs | Fixes test invocation to properly scope guard page crash tests |
| src/hyperlight_host/src/hypervisor/mod.rs | Adds generation tracking to InterruptHandle trait with extensive documentation |
| src/hyperlight_host/src/hypervisor/kvm.rs | Implements generation-aware interrupt handling for KVM |
| src/hyperlight_host/src/hypervisor/hyperv_windows.rs | Implements generation-aware interrupt handling for Windows Hyper-V |
| src/hyperlight_host/src/hypervisor/hyperv_linux.rs | Implements generation-aware interrupt handling for Linux Hyper-V |
| src/hyperlight_host/Cargo.toml | Updates dependency versions and adds Windows feature flags |
| Justfile | Separates stress test execution to show output clearly |
ab3ce0a to
10dae15
Compare
ludfjig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok but still not convinced about the call_active flag.
898cbee to
031641d
Compare
f9b9edd to
3a271df
Compare
3a271df to
d83b9aa
Compare
161701a to
d3c790b
Compare
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
d3c790b to
932285a
Compare
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
* Add poison state to sandbox to prevent misuse (#931) Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> * Fixes a race condition in killing Sandboxes (#959) Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com> * Update common workflow to 1.8.0 Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com> * Fixes issues from merge and splits trait to expose only public method (#994) Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com> * updates to change log and crate version for v0.9.1 Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com> --------- Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com> Co-authored-by: Simon Davies <simongdavies@users.noreply.github.com>
Fixes a race condition where a sandbox kill arrives after a sandbox has successfully exited causing the subsequent run to fail.
There is a breaking change in this PR, previously if kill was called on an
InterruptHandlebefore or while a guest call was not in progress the next guest call made on theSandboxwould be cancelled , now this scenario is a no-op. kill only takes effect if there is a guest call running.