Repository navigation
test: a pipe end leaked into a sweep no longer fails the concurrent-sweeps capacity check on macOS - #208
Merged
Conversation
This was referenced Oct 8, 2026
… end On macOS std makes a pipe with pipe() and only then sets FD_CLOEXEC, so a sweep spawned while another test thread spawns can inherit a write end of a pipe the test reads. The fake runner's backgrounded lock holder redirects only fds 0-2 and keeps that end, so output() returns only once the holder has exited and released its lock, and the live-root check reads 0. The new case runs the concurrent-sweeps scenario with each sweep started through sh, holding a write end of its own stdout at fd 9, which puts every platform in that state on purpose. It fails today at the live-root check, after the holder's 20 seconds. The existing case keeps its plain reading of the shared ceiling.
Each fake detached runner left a backgrounded python holding the root's run lock for 20s, with stdout and stderr redirected but every other inherited descriptor kept. A sweep that carried a leaked pipe end - which macOS std leaves when another thread spawns between pipe() and the close-on-exec mark - handed it down to that holder, so the test's output() waited for the holder to exit and then read the lock as released (#203). The three holders become one shared RUN_LOCK_HOLDER that closes every descriptor above stderr, listed from /dev/fd, before it takes the lock.
vjovanov
force-pushed
the
fix/issue-203
branch
from
October 8, 2026 08:05
64ce551 to
97443f3
Compare
vjovanov
marked this pull request as ready for review
October 8, 2026 08:05
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.
Closes #203
review_repro_concurrent_sweeps_share_the_global_ceilingfailed intermittently on macOS at its last check, the one that reads whether the started root's run lock is still held. The defect was in the test's fake runner, not in ephor. The lock holder that the fakerheileaves behind kept a pipe end it had inherited by accident. This pull request changes only tests.Before this change, the race can be forced on any platform by handing each sweep a write end of its own stdout at fd 9, and the new test does exactly that. At
cba2f56d5e, where the test exists but the fix does not, the test sits out the lock holder's 20s sleep and then fails with the panic CI reported (at:195onmain; the assertion is now at:235):With this change, the holder closes the leaked pipe end before it takes the lock, and both tests pass in half a second:
Why it failed
The cause was not a slow runner, which is what the issue guessed. The CI log rules that out. In the failing job, the other 13 tests in the binary kept the green run's pace and were all done 6.7s in. This test alone finished at 25.54s, about 20s late, and 20s is exactly the lock holder's
time.sleep(20). The test was waiting for the holder to exit.On macOS, Rust std has no
pipe2.Command::output()makes each pipe withpipe()and only then marks itFD_CLOEXEC, in two separate steps. The test starts its twowork run --duesweeps from two threads at once, so one sweep can be spawned between those two steps of the other and inherit a write end of a pipe the test reads. That descriptor then passes from the sweep'sephortosh -c, then to the bash fakerhei, then to the backgroundedpython … time.sleep(20)that holds.rhei/run.lock. The holder redirected fds 0-2 and nothing else, so it kept the write end for its whole 20s.output()reached EOF only after the holder had exited and dropped its flock, and thelivecheck read 0.The behaviour the test pins holds. Filtered sweeps in separate processes share the global ceiling, so exactly one starts a root and the other is passed over (§FS-005-dispatch.24). In each of triage's 21 control runs, the started root's lock was still held after both sweeps returned. Nothing in ephor ends the holder early.
What changed
capacity_runnerandoverlapping_capacity_runnerintests/integration/work_capacity_test.rs, andholding_runnerintests/integration/work_checkout_test.rs. All three had the same hazard. They are now oneRUN_LOCK_HOLDERconst intests/integration/common/mod.rs, and each fixture'sformat!interpolates it as{holder}./dev/fd, closes each descriptor numbered above 2, and only then opens and flocks the lock file. Its stdout and stderr still go to/dev/null. Its stdin stays the/dev/nullthat bash gives a background job. Itstime.sleep(20)is unchanged. A pipe end leaked into a sweep now goes away with the launcher, sooutput()returns at once and the check sees the lock still held./dev/fdrather than a fixed range. macOS has noclose_range, soos.closerange(3, 65536)would cost about 65kclose()calls per holder there./dev/fdlists exactly what is open on Linux and macOS, and it is where CPython's ownsubprocesslooks on macOS. Each listed descriptor is closed throughos.closerange(fd, fd + 1). That call ignores the one entry already closed by then, the descriptorlistdirread the directory through.concurrent_sweeps_share_the_global_ceiling_while_holding_a_leaked_pipeis new. It starts each sweep through/bin/sh -c 'exec 9>&1; exec "$0" "$@"'(holding_its_stdout), with the same program, arguments and environment. So on every platform, each sweep holds a write end of its own stdout at fd 9, which is the state the macOS race leaves a sweep in. There is no wall-clock assertion. Thelivecheck is the pin.review_repro_concurrent_sweeps_share_the_global_ceilingis now a one-line call into the shared bodyconcurrent_sweeps_share_the_global_ceiling(leak: bool)withleak = false, so the fixture setup is not duplicated.The specification. No spec point is written or moved. Both tests cite §FS-005-dispatch.24, and nothing under
src/changed. There is no changelog entry, on purpose. Section 1.1 ofdocs/changelog.mdsays no change edits that file, because the release takes each pull request's title as its line (§FS-002-release.1.3).Found and deliberately not fixed
pre_execincaptured()). It was not taken, because it would change what a runner launched by the shipped tool inherits, and this ticket does not need that.std::thread::scopeof feed refresh (src/feed/refresh.rs:183). On macOS the same race could leak one child's pipe into another, and a long-lived descendant could then hold a capture open until its timeout. Nobody has observed this, and it is left for a person to file.RUN_LOCK_HOLDERand on the fixture runners say that the holder closes "every other descriptor it inherited". In fact it closes every descriptor above stderr and keeps stdin. The finding is a nit, and the comments were left unchanged.How it was verified
The branch is test first.
cba2f56d5eholds only the new test, its helper and the shared body, and64ce551967holds the fix.cba2f56d5e, before the fix (run 37743882941). The new test failed atwork_capacity_test.rs:235:5withleft: 0on bothmacos-latestandubuntu-latest. Each time it was the only failure in its binary (14 passed; 1 failed). So the test catches the defect on macOS as well as on Linux.64ce551967, with the fix (run 37745221301).cargo test (ubuntu-latest)passed. Themacos-latestjob failed earlier, in itspython testsstep, on the known flake test_prepare_changelog_release CompletenessTests fail intermittently on macOS: release_forge.py:269 git 'unable to create temporary file: Invalid argument' #202 (release_forge.py:269,git add -A failed: error: unable to create temporary file), so itscargo teststep was skipped. The fixed tree has not yet been seen passingcargo teston macOS. The ship step reruns CI and collects that result.798f3077c1and atcba2f56d5e. At64ce551967it exits 0 (DEFECT GONE), with the forced run passing in 0.54s.64ce551967, on Linux. Five commands and the reproducer ran, 4m01s in all, and all exited 0:pre-commit run --all-files:grund checkandgrund fmtwith CI's pinned grund 0.14.0, fissile, the private-words hook and the attribution hook;check_boundary.py;check_parity.py: 25 abilities, 26 presentation keys, 54 bindings accounted for, and every command has--json;cargo test, withTMPDIRon a symlinked directory.cargo test --all-targets --lockedpassed 1821 tests with 0 failures, both plain and withTMPDIRon a symlinked directory.work_capacity_testpassed all 15 tests in 1.13s, where atcba2f56d5eit took 20.53s and failed the new test.work_checkout_testpassed all 12 tests in 0.81s.AI workflow: `rhei`, 11 agent invocations across 1 model; 6 tasks completed, 10 in progress.
github-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 1) — cld, anthropic/claude-opus-5-5 — 1m53s — 844.5k in / 11.1k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticket.triageassessing (visit 1) — cld, anthropic/claude-opus-5-5 — 14m23s — 7.3M in / 70.5k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticket.triagereproducing (visit 1) — cld, anthropic/claude-opus-5-5 — 8m12s — 3.0M in / 37.6k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 2) — cld, anthropic/claude-opus-5-5 — 3m01s — 878.4k in / 16.0k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticket.specifyspecify — cld, anthropic/claude-opus-5-5 — 6m02s — 3.1M in / 25.7k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 3) — cld, anthropic/claude-opus-5-5 — 1m24s — 702.6k in / 8.4k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticket.implementimplement — cld, anthropic/claude-opus-5-5 — 9m40s — 2.1M in / 23.7k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 4) — cld, anthropic/claude-opus-5-5 — 1m29s — 732.9k in / 8.0k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticket.review-1review — cld, anthropic/claude-opus-5-5 — 10m42s — 5.6M in / 36.7k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 5) — cld, anthropic/claude-opus-5-5 — 1m10s — 753.6k in / 5.8k outgithub-issues-agent-grounds-ephor-203-implement-2ece1d3c.ticketsupervising (visit 6) — cld, anthropic/claude-opus-5-5 — 1m32s — 1.0M in / 8.9k out