Repository navigation
mkdir: do not ignore EEXIST on non-recursive creation - #13986
Merged
sylvestre merged 1 commit intoAug 28, 2026
Merged
Conversation
|
GNU testsuite comparison: |
sylvestre
force-pushed
the
fix-13970-mkdir-concurrent-eexist
branch
from
August 18, 2026 20:44
4e4ed7b to
eb057b9
Compare
sylvestre
reviewed
Aug 25, 2026
| } | ||
|
|
||
| Err(_) if path.is_dir() => { | ||
| Err(_) if (config.recursive || is_parent) && path.is_dir() => { |
Contributor
There was a problem hiding this comment.
is_parent is dead here - parents are only created inside the config.recursive branch, so this is just config.recursive && path.is_dir().
Contributor
Author
There was a problem hiding this comment.
Updated, simplified the guard to config.recursive && path.is_dir().
Fixes uutils#13970 Signed-off-by: Alex Chen <[email protected]>
alexchen-sys
force-pushed
the
fix-13970-mkdir-concurrent-eexist
branch
from
August 28, 2026 07:05
eb057b9 to
c2c9cc6
Compare
Contributor
|
Clippy seems unhappy on openbsd |
Contributor
Author
|
That's the OpenBSD infra flake — |
jxy
added a commit
to jcosborn/qex
that referenced
this pull request
Oct 6, 2026
Create the claim file with set -C, which the shell opens with O_EXCL. Ubuntu 26.04 ships uutils coreutils 0.8.0, whose mkdir can exit 0 when the kernel returns EEXIST build. uutils/coreutils#13970 uutils/coreutils#13986 The Rust kids rewrote a 50-year-old wheel and it came back square; mkdir as a mutex is older than most of them.
2 tasks done
This was referenced Oct 24, 2026
martinfrancois
added a commit
to martin-francois/symphony-trello
that referenced
this pull request
Oct 24, 2026
Three CodexAppServerClientTest fixtures start two app-server processes from one script, and each process picked its role with a mkdir lock. That assumes exactly one of two racing mkdir calls succeeds. The CI test job moved to Ubuntu 26.04, whose uutils coreutils 0.10.0 mkdir reports success when the directory exists by the time mkdir(2) fails. Both sessions then took the rate-limit role, no session sent the usage-limit failure, and usageDeadlineWaitsForAnInFlightRateLimitUpdateDecision timed out waiting for it. uutils fixed this in 0.11.0 (uutils/coreutils#13986). The fixture now sets the role from the session's workspace directory, which the test chooses, so the in-flight test also knows which future is the usage-limit run instead of finding out from the published event. Closes #1016
This was referenced Oct 25, 2026
martinfrancois
added a commit
to martin-francois/symphony-trello
that referenced
this pull request
Oct 25, 2026
Three CodexAppServerClientTest fixtures start two app-server processes from one script, and each process picked its role with a mkdir lock. That assumes exactly one of two racing mkdir calls succeeds. The CI test job moved to Ubuntu 26.04, whose uutils coreutils 0.10.0 mkdir reports success when the directory exists by the time mkdir(2) fails. Both sessions then took the rate-limit role, no session sent the usage-limit failure, and usageDeadlineWaitsForAnInFlightRateLimitUpdateDecision timed out waiting for it. uutils fixed this in 0.11.0 (uutils/coreutils#13986). The fixture now sets the role from the session's workspace directory, which the test chooses, so the in-flight test also knows which future is the usage-limit run instead of finding out from the published event. Closes #1016
martinfrancois
added a commit
to martin-francois/symphony-trello
that referenced
this pull request
Oct 25, 2026
Three CodexAppServerClientTest fixtures start two app-server processes from one script, and each process picked its role with a mkdir lock. That assumes exactly one of two racing mkdir calls succeeds. The CI test job moved to Ubuntu 26.04, whose uutils coreutils 0.10.0 mkdir reports success when the directory exists by the time mkdir(2) fails. Both sessions then took the rate-limit role, no session sent the usage-limit failure, and usageDeadlineWaitsForAnInFlightRateLimitUpdateDecision timed out waiting for it. uutils fixed this in 0.11.0 (uutils/coreutils#13986). The fixture now sets the role from the session's workspace directory, which the test chooses, so the in-flight test also knows which future is the usage-limit run instead of finding out from the published event. Closes #1016
nrjdalal
added a commit
to nrjdalal/playground
that referenced
this pull request
Nov 5, 2026
…buntu-26.04 (#1076) ## TL;DR - 🧪 The run-slot lock tests stop failing now and then on GitHub's Linux runner. One such failure held up a ship. - 🔍 The cause was the runner's own `mkdir` command, which can tell two programs that each made the same folder. The tests now use the runner's other `mkdir`, which tells only one, as the Mac's does. ## Summary ```diff tests/scripts/ops/run-slot-fake.ts, machine() slowChild("python3", bin) + if (GNU_MKDIR) // Bun.which("gnumkdir"): ubuntu-26.04 only + bin/mkdir = exec gnumkdir "$@" ``` - GitHub's `ubuntu-latest` is ubuntu-26.04, whose `/usr/bin/mkdir` is uutils coreutils 0.10.0 (`rust-coreutils` 0.10.0-1ubuntu2~26.04.1). Its `create_dir` checks `path.exists()`, then maps a failed `mkdir(2)` to success with `Err(_) if path.is_dir() => Ok(())`, even without `-p`. So when two processes race, both exit 0. - run-slot.sh takes every lock with a plain `mkdir`: the gauges refresh lock, the run slots and the model slots. On that runner, two looks can each hold the gauges lock and both run `top`: "eight looks with no reading yet run top once" got 6. - The image keeps GNU's mkdir as `gnumkdir`, and each fake machine's bin now runs that one wherever it exists. On the Mac and on GNU images nothing changes. - `.agents/testing.md`, Traps: one new trap, the racing `mkdir` and how a test avoids it. **Decisions:** - **The fix is in the test, not in run-slot.sh.** run-slot.sh runs for real only on the Mac, whose BSD `mkdir` is atomic: 60 rounds of the eight-look case there each ran one `top`. Only CI runs the script on Linux. - **The dead-lock takeover case stays out of this PR.** It is a separate race, not yet reproduced alone (Evidence, #959's row), now #1077, so this PR keeps to the two "run top once" cases #956 names. - **The reported lead is refuted.** The lead was that a reading stamped with the look's start time goes stale after a refresh slower than 10 s. The red run took 1.6 s, and six one-second tops in 1.6 s means they ran side by side. A stale stamp cannot do that, while two lock holders can. - **Nothing is posted upstream.** uutils fixed this in 0.11.0 (uutils/coreutils#13970, uutils/coreutils#13986), and Ubuntu's request to bring that fix to 26.04 (LP #2171333) already has a patch waiting for a sponsor. A comment would add no new facts. Once 26.04 takes that patch, the shim is harmless and can go. ## Evidence - **Cause, in an `ubuntu:26.04` container:** 8 racing `mkdir L`, 200 rounds each: | mkdir | rounds with more than one winner | | --- | --- | | `/usr/bin/mkdir` (uutils 0.10.0) | 33 of 200, and 38 of 200 on a second run | | `gnumkdir` (GNU 9.7) | 0 of 200 | - **No repro without uutils:** the eight-look case ran exactly one `top` in 60 of 60 rounds on the Mac (BSD mkdir) and 80 of 80 on `oven/bun:debian` (GNU mkdir, bash 5.2). - **Before / after in `ubuntu:26.04`** (`--cpus 2`, Linux bun 1.4.4): | run | canary's fake | this head's fake | | --- | --- | --- | | probe: 60 rounds of 8 looks with no reading, rounds that ran `top` more than once | 10 of 60 (2 or 3 tops) | 0 of 60 | | `run-slot-gauges-lock.test.ts`, 20 runs, the two "run top once" cases | failed in 6 of 20: the stale-reading case (line 46) in runs 1, 14 and 15, the no-reading case (line 52) in runs 5, 7 and 13 | failed in 0 of 20 | - **At this head on the Mac:** 20 runs in a row of `bun run test:file tests/scripts/ops/run-slot-gauges-lock.test.ts`: 20 of 20 passed, each 10 pass, 0 fail. `bun run check-types:tests`: no errors. `bun run test:skills`: all 13 passed. - **Sightings on CI:** - canary at d216f23, Suite (8): "eight looks with no reading yet run top once", expected 1, received 6, in 1631 ms. - #1063 at 1af2e05, Suite (7) (job 127580442051), ubuntu-26.04: "eight looks at a stale reading run top once", expected 1, received 2, in 1526 ms. The same case fails 3 of 20 on canary's fake above and 0 of 20 at this head. - #959 at 3006e0a, attempt 1, Suite (5) (job 127180040487): line 124 of the same file, "eight looks at a dead lock together each read the stale or the new reading", expected true, received false, in 1627 ms. **Not this race, and not fixed here.** That case still failed 1 of 20 runs at this head in `ubuntu:26.04` (run 12, line 125), with GNU's `mkdir` in the fake. A probe of the case alone found 0 bad rounds in 60 with GNU's `mkdir` and 0 in 60 with all 104 `gnu*` tools on PATH, so no tool swap is shown to fix it. It is #1077. - **doc-sync:** the change touches the test fake alone. `git grep -n -iE "mkdir"` over `.agents`, `scripts/README.md`, `scripts/ops/README.md`, `scripts/hooks/README.md`, `docs` and the core skills finds no line that calls `mkdir` atomic. The new trap in `.agents/testing.md` is the one place that says it is not on ubuntu-26.04. ## Merge Danger **Door:** two-way A revert brings back the uutils `mkdir` in the fake, and the flake with it. **Blast Radius:** small Tests only: every `run-slot*.test.ts` machine on a runner that has `gnumkdir`. run-slot.sh and every other script are unchanged. ## Links - Closes #956 - uutils/coreutils#13970 (the report) and uutils/coreutils#13986 (the fix, in 0.11.0) - #1077: the dead-lock takeover race, left out of this PR - Ubuntu: https://bugs.launchpad.net/ubuntu/+source/rust-coreutils/+bug/2171333 🤖 Generated with [Claude Code](https://claude.com/claude-code)
4 tasks
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.
Fixes #13970.
mkdirwithout-pincorrectly swallowsEEXISTwhen the target path already exists as a directory. Increate_single_dir, the error branchErr(_) if path.is_dir()returnsOk(())unconditionally, without checking whether recursive (-p) mode was requested.This is a logic bug: a non-recursive
mkdiron an existing directory must fail per POSIX, but currently exits 0. Under concurrent invocation, this also allows multiple racing processes to all "succeed" in creating the same directory.Fix: require
config.recursive || is_parentbefore allowing theEEXIST→Ok(())fallback. This matches GNU mkdir behavior and the existing test expectations.Regression tests:
test_mkdir_concurrent_creation: concurrentmkdir -pon the same path — all processes must succeed (valid for-p).test_mkdir_concurrent_non_recursive: concurrentmkdirwithout-pon the same path — exactly one process must win per round.Verified on bare-metal Linux and in a sandboxed environment: 500 concurrent rounds on main showed double-wins (both processes exiting 0), while this branch showed 0 double-wins. All existing tests pass.