Skip to content

Conversation

@ramsyana
Copy link
Contributor

This PR resolves a race condition when removing a container immediately after stopping it, caused by the stop command returning before the container fully transitions to the stopped state (#130).

Changes:

  • Enhanced TestCLIRmRace.swift with robust test logic and helper methods (containerExists, safeRemove).
  • Improved error handling to distinguish race conditions from successful removals.
  • Added exponential backoff retry logic for cleanup operations.
  • Updated CLITest.swift with missing doRemove method.
  • Fixed BuilderStart.swift to handle .stopping case.
  • Improved error messages with container ID for better debugging.

Testing:

  • ✅ All tests pass (make test, make integration).
  • ✅ Verified on macOS 26.
  • ✅ Race condition test validates success and failure scenarios.
  • ✅ Code formatted (make fmt).

Hopefully, this will pass the integration tests on GitHub.

@ramsyana ramsyana force-pushed the fix/stop-rm-race branch 2 times, most recently from 9862a5b to d315cf2 Compare June 16, 2025 16:33
- Add proper handling for container state transitions

- Fix race condition test flakiness with improved error handling

- Add missing .stopping case in BuilderStart.swift

- Improve error messaging with container ID

- Resolve build and test failures

- Update license headers and formatting

- Add helper methods for safer container management

- Enhanced test logic to handle both immediate success and controlled failure cases

Signed-off-by: ramsyana <[email protected]>
Copy link
Contributor

@adityaramani adityaramani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@dcantah
Copy link
Contributor

dcantah commented Jun 17, 2025

Thanks for this

@dcantah dcantah merged commit 21cfebb into apple:main Jun 17, 2025
2 checks passed
@ramsyana ramsyana deleted the fix/stop-rm-race branch June 17, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants