Fix container hang when Ctrl-C is pressed on apple runtime#13
Merged
Conversation
Wrap the user command in a login shell in Container.Attach to work around PATH resolution bugs in apple/containerization <= 0.26.3. Replace direct signal handling in Container.Wait with ctx.Done() so Ctrl-C correctly triggers container shutdown. Add a 10-second timeout context to ForceRemove in the cleanup handler.
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.
What
Fix a hang that occurred when pressing Ctrl-C while running contagent against the apple runtime.
Key changes:
/bin/sh -l -c) inContainer.Attachso profile files are sourced and PATH includes user-local directoriesContainer.Waitwithctx.Done()so Ctrl-C (which cancels the context inmain.go) correctly triggers container shutdownForceRemovein the cleanup handlerapple/containerships a release pinningcontainerization >= 0.26.5Why
Two bugs in
apple/containerization(PRs #550 and #562) causedcontainer execto ignore the container's configured PATH and fall back to a hardcoded default that omits user-local directories like~/.local/bin. When the command couldn't be found, the container hung instead of exiting cleanly. The latestapple/containerrelease (v0.10.0) pinscontainerization 0.26.3and does not include these fixes.Separately,
Waitwas listening for OS signals directly alongside actx.Done()case that only waited for the process without stopping the container — thectx.Done()path would block forever if the container hung. Consolidating toctx.Done()with acontainer stopcall fixes this.How to Test
contagent <command>Notes
The login shell workaround is intentionally temporary. Removal steps are documented in a TODO comment in
container.go.