[HIP] [OPUS] [JIT] fix(jit): baton liveness is undecidable from pid+host under --network host - #5269
Conversation
… host
FileBaton records the holder as pid + hostname. A container started with
--network host reports the HOST's hostname while keeping its own PID
namespace, and every container has a pid 1 and low worker pids (100-104). So
when a tuner container is killed mid-build, the next container reads the leaked
lock, sees a matching hostname, checks os.kill(103, 0) -- finds its OWN worker
at pid 103 -- and concludes the dead holder is alive. Its workers then wait
forever:
[pid=101 pname=SpawnPoolWorker-1] waiting for baton release at
.../lock_module_gemm_a8w8_bpreshuffle_cktile_tune
With a host-mounted build cache the leak survives container restarts, so the
wedge is permanent until someone deletes the file by hand. We lost hours to
this before finding it, and 'delete the lock files' became a required step in
our tuning runbook.
Record the PID namespace (/proc/self/ns/pid) and the holder's start time
(/proc/<pid>/stat field 22) alongside pid and host:
* same namespace -> pid liveness AND start time must match, which also
closes the pid-reuse hole on any host;
* other namespace -> the pid is meaningless here, so decide on the holder's
heartbeat: the holder now touches the lock every
heartbeat_seconds while it works, and a lock untouched
for heartbeat_stale_seconds (default 60s) is stale.
Locks written by an older AITER carry two lines; they parse with empty ns and
start time and keep exactly the previous behaviour.
Checked: self-held lock never looks stale; foreign-namespace holder with a live
local pid is respected while heartbeating and goes stale when it stops;
recycled pid detected via start time; dead pid; legacy 2-line locks both live
and dead; 0-byte orphan grace unchanged; wait() returns False after breaking a
stale lock; heartbeat observed to touch the file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
PR title tags & labels: |
github-actions
Bot
changed the title
![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: 13c88607f7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8508b6de91
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
github-actions
Bot
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
aiter/aiter/jit/utils/file_baton.py
Lines 185 to 186 in 5070048
Preserve the replacement lock when an expired holder resumes
If a foreign-namespace holder is paused or otherwise misses heartbeats for more than 60 seconds, a waiter can legitimately classify its lease as stale and replace the lock; when the original process resumes, this unconditional pathname removal deletes the new holder's lock rather than its own unlinked file. That exposes the replacement build to additional concurrent builders and possible artifact corruption. Before unlinking, verify that the path still identifies this instance's lock (for example, by comparing it with the still-open descriptor or an ownership token).
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92e813185f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d7a802e66
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01ca846e14
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 077336737c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fba58ebe82
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7fe959b84
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8d5a3a21b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 195309bb26
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdc7f4ca8c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdfd08a1fe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fde5d07a69
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Fixes #5266.
The bug
FileBatonrecords its holder as pid + hostname. A container started withdocker run --network host— the normal way to run a tuner — reports the host's hostname while keeping its own PID namespace, and every container has a pid 1 and low worker pids (100-104).So when a tuner container is killed mid-build, the next container reads the leaked lock, sees a matching hostname, checks
os.kill(103, 0), finds its own worker at pid 103, and concludes the dead holder is alive. Its workers then wait forever:With a host-mounted build cache the leak survives container restarts, so the wedge is permanent until someone deletes the file by hand. "Delete the lock files first" became a required step in our tuning runbook.
The fix
Record the PID namespace (
/proc/self/ns/pid) and the holder's start time (/proc/<pid>/statfield 22) alongside pid and host:heartbeat_secondswhile it works; a lock untouched forheartbeat_stale_seconds(default 60 s) is stale.Locks written by an older AITER carry two lines; they parse with empty ns/start-time and keep exactly the previous behaviour.
Checked
Self-held lock never looks stale · foreign-namespace holder with a live local pid is respected while heartbeating and goes stale when it stops · recycled pid caught via start time · dead pid · legacy 2-line locks (live and dead) · 0-byte orphan grace unchanged ·
wait()returns False after breaking a stale lock · heartbeat observed touching the file.Same campaign as #5262 / #5268 / #5263–#5265.
🤖 Generated with Claude Code