Repository navigation
fix(gc): freeze a fork chain behind its zombies before gc reads it ended - #326
Conversation
… it ended Codex on #302: under a PID 1 that never reaps, a fork chain leaves its zombies in view while its live member escapes both scans, so gc read the group ended and removed program.json while the chain ran. judgeFrozenGroup now sends SIGSTOP, as stopProgramGroup does, judges the stopped group, and sends SIGCONT in a finally. The integration test runs the chain under the subreaper that never reaps.
The model gets the guard GcFreezesUnseenGroup and the environment constant ZombiesInView. With zombies in view the probe alone proves nothing, and gc sees the chain only through the freeze. mutant-GcFreezesUnseenGroup-unseen-zombies must violate R20. With every guard, ZombiesInView changes nothing, so current-unseen covers the pass run. ProvesGroupOwnership lets judgeFrozenGroup send SIGSTOP and SIGCONT, and the raw kill stays in stopProgramGroup. USAGE limits 3 and 5 name the two signals.
check.sh --update-coverage at 1c1bd1c6 gave 79/79 runs as expected in 1,121 s. current has 29,309,460 distinct states and current-no-token 29,186,546.
|
@codex review |
|
@codex security review |
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. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ea48c887e
ℹ️ 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 on #326: SIGSTOP does not nest, so another gc or a destroy can send SIGCONT before this gc judges the frozen group, and a fork chain then read gone and lost program.json. judgeFrozenGroup now reads a frozen group that still reads gone as inconclusive, so provenProgramGroupRuns freezes it again, and only GC_JUDGEMENTS such judgements read it ended. The model has one gc and does not change, so the record comes from facts.mjs at this commit.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…ded in one pass Advisor on #326: a zombie-only ended session under a PID 1 that never reaps took ten passes of six /proc scans and ten SIGSTOP/SIGCONT pairs: about 2.5 s with 2,264 processes in /proc. judgeFrozenGroup now freezes and judges at most twice (FROZEN_JUDGEMENTS). The second freeze covers a SIGCONT from another gc or a destroy. When that judgement reads gone too, nothing live was in view to freeze, and the group reads ended: about 300 ms there, and 50 ms with 258 processes. Also from the advisor: unit tests for a frozen group whose live member is in another autogroup or session (ended, and resumed) and for a /proc/self of another pid (no SIGSTOP), and USAGE limit 5 says that gc keeps the session only when the frozen check proves the group.
Codex round 3 on #327 (P1) and the maintainer: reading a group ended after two frozen judgements that read gone was a regression from #326, which retried up to GC_JUDGEMENTS times. A SIGCONT from another gc or a destroy before both judgements let gc remove the folder of a live fork chain. judgeFrozenGroup now returns null (inconclusive) after both frozen judgements read gone, so provenProgramGroupRuns judges the group again. It keeps the two freezes per judgement, so a gc sends at most 10 x 2 SIGSTOP/SIGCONT pairs. An ended zombie-only group with about 2,300 processes in /proc now takes about 3.5 s at a load of 90, against about 2.5 s for #326's ten single freezes. New unit test: a live member that escapes both frozen judgements of one pass keeps the session (fails with the old return). facts.mjs expects the null return, and a false return turns GcFreezesUnseenGroup FALSE. USAGE limit 5 and SKILL state the cost and the remaining race (all 20 frozen judgements).
Why
Codex on #302 (round 6, thread 4464932183) found a fork-chain case that gc misses. A PID 1 that never reaps leaves the zombies of a fast fork chain in view, while the chain's live process escapes both
/procscans. Thenkill(-pgid, 0)finds the id, but the scans show only zombies. So gc read the group as ended and removedprogram.jsonwhile the chain ran. After that, nodestroycould find the chain. Main removes every such session, and #302 did too for this case. This PR closes it.Follow-up of #302.
What changes for users
finally. A stopped process stays in view, so the check finds the chain, and gc keeps the session with "the session's program group still runs (destroy ends it)"./procdoes not show its own pid space. That is the same condition thatstopProgramGroupuses.No JSON shape changes.
What changes for maintainers
judgeGroupForGccalls the newjudgeFrozenGroupwhen the probe finds the id and both scans show only zombies.judgeFrozenGroupuses the freeze, judge and SIGCONT-in-finallysteps ofstopProgramGroup. A frozen group that readsownedkeeps the session, andunprovenreads ended.goneandunknownare inconclusive, so gc freezes and judges again, up toGC_JUDGEMENTS(10) times. SIGSTOP does not nest: another gc or adestroycan send SIGCONT before this judgement (Codex round 1). A group that has ended readsgoneevery time, so after ten judgements it reads ended.GcFreezesUnseenGroupand the environment constantZombiesInView. With zombies in view, the probe alone proves nothing, and gc sees the chain only through the freeze.mutant-GcFreezesUnseenGroup-unseen-zombiesmust violate R20 (RemovedSessionStopped). With every guard TRUE,ZombiesInViewdoes not changeGcSeesGroup, socurrent-unseencovers the pass run. That makes 79 runs and thirty-six guards.facts.mjsreads the new guard fromjudgeGroupForGcandjudgeFrozenGroup, and hashesjudgeFrozenGroup.ProvesGroupOwnershipnow allowsjudgeFrozenGroupas a second caller ofsignalProcessGroup, which sends only SIGSTOP or SIGCONT. The raw group kill stays insidestopProgramGroup.Validation
programGroup.test.ts: "freezes a group whose scans show only zombies…" (two cases: the frozen group shows a live member, or only zombies) and "gives a frozen group SIGCONT when its judgement throws". All three fail.non-reaping-init.test.ts: "gc keeps an exited session while a fork chain runs behind its zombies". It runs the fork chain undernon-reaping-subreaper.pyand waits for 200 zombies. It failed 2 of 2 runs (gc removed the session). With the fix it passed 3 of 3 runs, together with the other three tests in that file.unknownread as running, or with a frozengoneread as ended,GcFreezesUnseenGroupreads FALSE. A kill added inside the freeze also turnsProvesGroupOwnershipFALSE.check.sh --update-coverageat 1c1bd1c6: 79/79 runs as expected in 1,121 s.currenthas 29,309,460 distinct states andcurrent-no-tokenhas 29,186,546 (unchanged). The new mutant violates R20.lifecycle-modelshares took 24.5, 10.8, 20.3 and 17.3 min of TLC, and 20 + 20 + 20 + 19 = 79 runs gave the expected results. The SIGCONT fix changes no model input, so ace0873 writes the record fromfacts.mjs.check.sh --drift-only,tsc --noEmit,oxlint,oxfmt --checkand the unit tests ofprogramGroup,gcandlifecycle(252 tests) pass. The unit suite passes (143 files, 2,019 tests).gc collects exited, failed, and destroyed sessionsneeds 29.1 s on main against its 30 s limit, and it timed out here at load averages of 26 to 42. gc itself takes 2 to 3 s on both trees for those three sessions. Indestroy ends a program whose killed background process stays a zombie, the 2 slistcheck read 2,471 ms at load 59, with the new test filtered out.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh