implement UFFD failure handling and unit tests#2856
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The failure callback in uffd.go is set and read across different goroutines without synchronization, creating a data race. Additionally, because the callback is registered after the handler starts, any immediate startup failure will be missed. Protecting the callback registration and invocation with a mutex, and persisting the failure state, is required to ensure thread safety and guarantee that late-registered callbacks still receive the failure event.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 271ff3e68b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Address data race and late-registration issues identified in code review: - Add sync.Mutex to guard onFailure field across goroutines - Persist failure state (failedErr/failedSbx/failedCtx) so callbacks registered after a failure still receive the event immediately - Invoke callback in a goroutine to avoid holding the lock during potentially slow sandbox teardown - Add TestLateCallbackRegistrationAfterFailure to verify the guarantee - Add TestCallbackNotInvokedOnCleanStop to verify no false positives
2 participants
Fixes part of #2813 : This ensures Firecracker instances are terminated correctly on UFFD faults, preventing new network and iptables leaks. Note that the broader cleanup mechanism for existing orphans remains tracked in #2813.
Implement complete UFFD failure handling mechanism and corresponding unit tests:
Key design notes:
All fast tests pass, error handling is complete, and changes are minimal and compliant.