Skip to content

Fix container start hang on invalid executable - #1278

Merged
JaewonHur merged 3 commits into
apple:mainfrom
JaewonHur:fix-hang
Mar 2, 2026
Merged

Fix container start hang on invalid executable#1278
JaewonHur merged 3 commits into
apple:mainfrom
JaewonHur:fix-hang

Conversation

@JaewonHur

@JaewonHur JaewonHur commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

This PR fixes container hang when starting an invalid executable (#1277).

Current ProcessIO.handleProcess has races between process.wait() and process.start(). In process.wait, SandboxService.wait registers and block on a continuation so that later exit of that LinuxContainer can resume it.

Even if SandboxService.wait checks the container state, there can be a race so that SandboxService.startInitProcess fails earlier with no continuation to resume, then SandboxService.wait registers the continuation, which forever hang the thread.

The root cause is because SandboxService.waiters is not updated atomically, so this PR adds mutex to protect updating SandboxService.waiters after checking it's in the correct state.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Starting a container with invalid executable hangs the terminal.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

Stopped state is an irrecoverable state where the VM is shutdown, and
cannot boot VM again. Stopped state has a non-nil exit code if the
container successfully ran and exited inside the VM. Otherwise,
something error happens while `bootstrap` or `startInitProcess`.

When the container exits too early (before start waiting), the exitCode
is set -1, which might give confusion whether it's real exitcode or not.
@dcantah

dcantah commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

This feels like a design/implementation failure on wait's part. Is there any way we can fix that instead?

@dcantah

dcantah commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Oh never mind this PR is now entirely different from my first viewing LOL

@JaewonHur
JaewonHur merged commit 5917a60 into apple:main Mar 2, 2026
5 of 6 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