Skip to content

Remove XPC timeout based on SIGTERM timeout in container stop#1387

Merged
dcantah merged 1 commit intoapple:mainfrom
katiewasnothere:stop_sigterm_timeout
Apr 4, 2026
Merged

Remove XPC timeout based on SIGTERM timeout in container stop#1387
dcantah merged 1 commit intoapple:mainfrom
katiewasnothere:stop_sigterm_timeout

Conversation

@katiewasnothere
Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix

Motivation and Context

container stop has an option -t for Seconds to wait before killing the containers (see here). This timeout is used to determine when the SandboxService should stop waiting for the container to gracefully exit with SIGTERM (or another provided signal) and send SIGKILL.

Today, we were setting the SandboxService.stop XPC call's timeout to the value provided to container stop -t plus 1 second. This is problematic in cases where the code flow to send SIGKILL to the container then forcefully stop the VM does not complete within one second of the SIGTERM timeout being hit. In those cases, we will return that the XPC stop call failed due to hitting the timeout. In some cases this may happen even though the container and VM have both been successfully stopped.

This PR removes that XPC timeout for SandboxService.stop entirely. We cannot reliably determine in the SandboxClient, which may be used by third party runtime plugins to support additional container runtimes, how long it may take for SIGKILL to kill the container and stop related resources.

Testing

  • Tested locally

Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
Copy link
Copy Markdown
Member

@dcantah dcantah left a comment

Choose a reason for hiding this comment

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

LGTM. I think this mainly existed as stop used to be fairly flaky and hang often. It was very much a band-aid

@dcantah dcantah merged commit 10df89e into apple:main Apr 4, 2026
4 checks passed
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