🐛 fix(runner): stop concurrent spawns inheriting capture pipes by gaborbernat · Pull Request #3553 · nextest-rs/nextest

gaborbernat · 2026-08-27T07:15:22Z

On Unix targets without pipe2(), std creates a pipe and sets FD_CLOEXEC in a second call, so a process spawned from another thread in between inherits both ends (rust-lang/rust#95584). Nextest spawns tests and list commands from several threads, so a test can inherit a sibling's capture writer, keep it open after the sibling exits, and get that sibling reported as leaked. I hit this on macOS in tox-dev/peryx#1629; sunshowers suspected the same race in #1469.

The fix is a process-wide RwLock on the Unix targets outside std's pipe2 list in sys/pipe/unix.rs. create_pipe takes the write lock around std::io::pipe() and spawn_process takes the read lock around the spawn, so spawns run in parallel and wait for pipe creation alone. Nextest creates the capture pipes itself instead of using Stdio::piped(), which std would create inside its spawn. Windows needs no lock because std serializes CreateProcess there; its split pipes become CreatePipe handles, as the combined path has used since #2665, with the default buffer instead of std's 64 KiB.

clap, cargo nextest run -E 'not test(ui_tests) and not test(example_tests)', release builds on a 10-core M-series, mean of alternating runs against main:

spawn lock	main	branch	change	runs
mutex, first version	1.68s	2.04s	+21%	5
read lock, this PR	1.24s	1.24s	0%	6

std's fork-and-exec fallback creates an exec-error pipe of its own inside the spawn; leaked into a concurrent child under a read lock, it would stall that spawn and every pipe creation behind it. Spawns that can fall back take the write lock instead: on Apple a non-absolute program, and on the other locked targets any spawn. Test binaries and the double-spawn stub are absolute, so test spawns keep the read lock.

Rust creates macOS pipes before setting FD_CLOEXEC. A concurrent spawn can inherit another test's capture descriptor and produce a false leak report. Lock capture-pipe setup through successful spawn on macOS. Child execution remains concurrent. Add a regression that checks child descriptors across 1,024 synchronized spawns. Refs nextest-rs#1469 Refs rust-lang/rust#95584

codecov · 2026-08-27T07:22:59Z

Codecov Report

❌ Patch coverage is 90.00000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.77%. Comparing base (c30f773) to head (c4a4e49).
⚠️ Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
nextest-runner/src/test_command.rs	67.39%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3553      +/-   ##
==========================================
+ Coverage   87.00%   87.77%   +0.76%     
==========================================
  Files         166      168       +2     
  Lines       50553    50321     -232     
==========================================
+ Hits        43986    44171     +185     
+ Misses       6567     6150     -417

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CI harnesses keep unrelated descriptors open, so an fd-range assertion confused ambient state with capture inheritance. Compare each child's macOS pipe handles with the known Nextest capture writers from the same spawn round. A signal handshake keeps those endpoints live until the parent records them without an elapsed-time assumption.

sunshowers · 2026-08-27T15:24:05Z

How is this specific to macOS? Seems like any platform without atomic CLOEXEC would be hurt by this. I would grab the platform list from the Rust standard library here.

Have you profiled it to figure out how much slower this gets? I would recommend testing against the clap repo by running all tests except ui_tests and example_tests (iirc).

sunshowers · 2026-08-27T17:36:04Z

The specific command I use to test performance with is, in the clap repo:

cargo nextest run -E 'not test(ui_tests) and not test(example_tests)'

The spawn lock was gated on macOS, but the standard library only creates pipes with pipe2(O_CLOEXEC) on a fixed list of targets. Gate the lock on the inverse of that list instead, expressed once as a `cfg!` const so the list is not repeated at every use. Windows is excluded because the standard library serializes `CreateProcess` itself for the same reason. Also wrap the setup script spawn. Setup scripts run before tests, so nothing races them today, but every in-process spawn now goes through the same boundary. Ignore poisoning of the lock: it guards no data, so a panic while holding it leaves nothing inconsistent, and the previous `expect` would have turned every later spawn into a panic. Replace the `proc_pidinfo`-based test with one that works on every Unix: each child closes its own capture pipes and lingers, and the parent requires every reader to reach EOF while all children are alive. A sibling that inherited a writer keeps it open, so the read times out. This covers both the split and combined capture strategies. With the lock disabled, both cases fail within the first two rounds.

Serializing every spawn behind a mutex costs about 20% on clap's suite on a 10-core macOS machine (`cargo nextest run -E 'not test(ui_tests) and not test(example_tests)'`: 1.68s on main, 2.04s with the mutex). The invariant only requires that no spawn overlaps the window between `pipe()` and `FD_CLOEXEC`, so spawns need to exclude pipe creation, not each other. Replace the mutex with an `RwLock`: `create_pipe` takes the write lock for the duration of `std::io::pipe()`, and `spawn_process` takes the read lock. For that to cover every capture pipe, nextest now creates them itself instead of using `Stdio::piped()`, which would create them inside the standard library's spawn without the write lock. The new `spawn_piped` helper wires the write ends into the command and attaches the read ends to the returned child through `ChildStdout::from_std`, so the test runner, the list phase, and setup scripts keep their existing reader types. With this change the same benchmark measures 1.24s on both main and this branch. The concurrent spawn test still fails within the first two rounds when the lock is disabled.

The comment on `spawn_process` described the standard library's fork and exec fallback as an open-ended gap. Nextest does not reach that path by default: `create_command` spawns the current executable through the double-spawn stub, so the program is absolute and `posix_spawn` applies, and interceptors create processes serially. State the one configuration that still reaches it, without double-spawn and with a relative or bare wrapper program.

Every comment the spawn lock added restated what the code does before saying why. Keep the reasons and drop the restatements.

sunshowers

Thanks. Overall looks good but needs a few fixes.

Review feedback from nextest-rs#3553. A spawn that falls back from posix_spawn to fork and exec creates the standard library's exec-error pipe under what was a read lock; a concurrent child inheriting that pipe would block the spawn, and through the lock every pipe creation, until the child exited. Take the write lock unless posix_spawn is guaranteed, which means Apple with an absolute program; test binaries and the double-spawn stub are absolute, so the concurrent path is unchanged. `spawn_process` now takes the command itself to inspect the program, which also removes the closure at its call sites. Also kill the child when attaching the capture readers fails instead of leaking it, narrow `create_pipe` and `spawn_process` to private and the per-OS reader conversions to `pub(super)`, and move the attach step into `imp` so the conversions stay module-local.

gaborbernat · 2026-09-02T23:23:10Z

@sunshowers is this ready to go, or is there anything else outstanding?

sunshowers · 2026-09-03T03:14:46Z

Mostly good, dealing with personal matters which take priority.

grishy · 2026-09-07T06:36:13Z

Adding another data point from our Rust workspace on macOS 26.6.2 (arm64), using nextest 0.9.129 (just for context, as you said non-macos special). I saw intermittent LEAK reports on synchronous configuration tests that don't spawn processes.

It occurred in one of three workspace repetitions and one of five API-package repetitions aporoximately.
An lsof snapshot reported two sibling test processes holding the same pipe endpoint:

PID    FD   PIPE
20492  46   0x64532149b05d75a7
20494   2   0x64532149b05d75a7

Both had nextest as their parent. One process held the other process's stderr pipe as an extra descriptor.

Use a shell child to close capture descriptors without an ignored helper test or unsafe code. Cover the Apple fork/exec path with a relative program and working directory alongside the absolute-program cases. Increase the budget to 32 rounds: lock-disabled measurements reached round 16, beyond the original zero-based budget. The relative cases exercise the fallback but do not assert on spawn duration, so a read-lock regression can still pass after stalling.

sunshowers · 2026-09-10T16:46:38Z

+    /// Keep children alive after closing stdout and stderr so an inherited
+    /// writer in a sibling delays EOF.


I don't think this comment is in the right location.

sunshowers · 2026-09-10T16:47:12Z

+        RelativeWithCwd,
+    }
+
+    /// Kill on panic so inherited writers cannot delay runtime shutdown.


I don't know how this explains LingeringChildren.

sunshowers · 2026-09-10T16:48:30Z

+    #[derive(Clone, Copy)]
+    enum ChildProgram {
+        Absolute,
+        /// The cwd forces Apple's fork/exec fallback for a relative program.


Should this be something like:

/// Configure the test to run as a program with a cwd and a relative path. /// /// In this scenario, Apple platforms fall back to fork/exec.

sunshowers · 2026-09-10T16:50:12Z

+/// Create capture pipes with `create_pipe`; `Stdio::piped()` bypasses its lock.
+///
+/// Fork/exec creates an exec-error pipe. A sibling that inherits its writer
+/// stalls the spawn and blocks pipe creation behind its lock. Take the write
+/// lock to prevent this; Apple spawns with absolute paths use `posix_spawn`
+/// and can run under a read lock.


I really don't understand what this is trying to say -- this is incredibly hard to follow.

This is very subtle code, and I would expect a comment here (really on PROCESS_SPAWN_LOCK) to be several hundred words (possibly 1000+ words) long and be written in a narrative style with several paragraphs. There is a lot to explain here. Things to cover include:

The symptoms observed which cause us to have to introduce the spawn lock.

The protocol we follow.

There are two mechanisms Rust uses on POSIX platforms: posix_spawn and fork/exec.

When creating a command with captured output, the Rust standard library must create a pipe for each kind of output that's captured (for both posix_spawn and fork/exec). We do this pipe creation ourselves to control concurrency for the reasons listed below.

But by default, pipes will end up leaking into child processes. All POSIX platforms support marking a pipe (or other fd) close-on-exec (CLOEXEC).

But also, there is a window after a pipe is created but before CLOEXEC is set on it that it can leak into another test process.

So many POSIX platforms support atomic CLOEXEC.

But some platforms don't, such as Apple ones. So we must do our own synchronization to ensure that no child processes are created in this window.

With fork/exec, a pipe is also created to communicate status between the parent and the child process. That, too, is susceptible to this race. And the consequences are actually quite a bit worse with this since the parent waits until the write end of the pipe passed to the child is closed (why? because the child writes the errno to that pipe if exec fails, so the parent reads until EOF to learn exec succeeded), and a leaked writer stalls that process spawn until the test it is leaked to completes.

Rust doesn't support introspection to determine if a particular command invocation will use posix_spawn or fork/exec, so we must simulate what the Rust standard library does here.

This boils down to rules X Y Z (covered in the earlier review comment), as examined on Rust 1.98. (Also important to cover things like we try our best to use posix_spawn over fork/exec, e.g. not setting uid/gid/pre_exec which always forces fork/exec -- and if any of that changes we must revise the rules to account for that).

We could use a mutex or always do a write lock to be simpler but that has a real measured perf loss. So we try and solve the harder version of the problem and determine where read locks are sufficient and where we have to use a write lock.

Why this is not an issue on Windows.

Most importantly, you must write this in your own words. You can use an LLM as an editor and proofreader, but I will not accept an LLM-written comment for something this careful and subtle. It doesn't have to be perfect technical English -- I'm happy to edit and polish the comment myself -- but it needs to be much more substantive than we have today.

This isn't required, but if you need tips on how to do technical writing well, I highly recommend Style: Lessons in Clarity and Grace.

sunshowers · 2026-09-15T22:14:27Z

Hey @gaborbernat, been thinking about this. I think I want to land this and then write a larger design document about process spawning myself, not as a comment but as a top-level document published to the site. What do you think of cleaning up the comments/shortening them and then making this ready? This seems like an important enough fix that I don't want to block on.

gaborbernat · 2026-09-15T22:53:28Z

Do you have any guidance on how much to shorten?

Move the race explanation onto PROCESS_SPAWN_LOCK, drop the restatements elsewhere, and describe what `LingeringChildren` and the shell command do.

sunshowers

Looks great as is -- thanks! When I write the design doc I might update the comments a bit.

gaborbernat · 2026-09-16T00:07:10Z

Would appreciate if you could tag me when you write it for my awareness and information, thanks!

sunshowers · 2026-09-16T00:53:48Z

Absolutely -- will do!

Weekly CI pin bump for the two third-party tool pins that drifted this week. - **cargo-affected** `=0.4.0` → `=0.4.1` — `affected.yaml`, both call sites - **cargo-nextest** `=0.9.144` → `=0.9.145` — `coverage.yaml`, `.github/actions/test-setup/action.yaml`, `.github/actions/tend-setup/action.yaml`, and `.codex/cloud.sh`, which the weekly checklist keeps level with `test-setup` Toolchain compatibility against the pinned `1.97.0`: `cargo-affected` 0.4.1 declares `rust-version = 1.94`, `cargo-nextest` 0.9.145 declares `1.91`. Both build under `baptiste0928/cargo-install`. The `.codex/cloud.sh` tarball URL resolves at the new version — `cargo-nextest-0.9.145-x86_64-unknown-linux-gnu.tar.gz` is published on the `cargo-nextest-0.9.145` tag. The `worktrunk` pin also drifted this week and is in its own PR — it is the `wt` that drives `wt hook pre-merge` on all three platforms, so a red matrix there shouldn't hold back these two. <details><summary>What the two releases change, against this repo's config</summary> **cargo-affected 0.4.1** carries two `run` fixes, both landing on the advisory `affected` legs: a committed uncovered change is now reported as uncovered rather than as "no changes" ([max-sixty/cargo-affected#92](max-sixty/cargo-affected#92)), and `run` no longer hands nextest a phantom-only filterset ([#81](max-sixty/cargo-affected#81)). **cargo-nextest 0.9.145**'s headline changes are profile-inheritance fixes — overrides defined in intermediate profiles are now applied, and a profile without `default-filter` inherits from its nearest ancestor rather than always from `profile.default`. Those are no-ops here: `.config/nextest.toml` defines only `profile.default` (plus its `junit` sub-table), sets no `default-filter`, and CI passes no `--tool-config-file`. The change that does reach this suite is [nextest-rs/nextest#3553](nextest-rs/nextest#3553): on Apple platforms, where the standard library can't create pipes with `FD_CLOEXEC` atomically, a concurrently-spawned test could inherit a sibling's capture pipe and the sibling was then reported as having leaked handles after exit. Nextest now creates capture pipes itself. That is a macOS-leg flake source removed, not a behavior change the suite has to absorb. </details> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>

* ⬆️ deps: take cargo-nextest 0.9.145's macOS capture pipe fix On macOS std creates a pipe and sets FD_CLOEXEC in a second call, so a test nextest spawned in between inherited a sibling's capture pipe and held it after the sibling exited: nextest-rs/nextest#3553, fixed in 0.9.145. That, not runner load, failed install_cli on the Intel macOS runner past the one-second window, which goes back to the shared 200 ms. * 🔧 chore: build releases through the R2 build cache and main's registry Release builds restored a cache no tag could have saved and started cold. They now restore the registry main saves for the same target, start the build cache CI uses, and build without --target so Windows' /Brepro reaches proc-macros.

gaborbernat mentioned this pull request Aug 27, 2026

Identify intermittent Nextest capture leak in S3 tests tox-dev/peryx#1629

Closed

gaborbernat closed this Aug 27, 2026

gaborbernat reopened this Aug 27, 2026

sunshowers reviewed Aug 27, 2026

View reviewed changes

Comment thread nextest-runner/src/test_command/imp.rs Outdated

gaborbernat added 2 commits August 27, 2026 14:05

gaborbernat changed the title ~~🐛 fix(runner): prevent macOS capture inheritance~~ 🐛 fix(runner): stop concurrent spawns inheriting capture pipes Aug 27, 2026

gaborbernat marked this pull request as draft August 27, 2026 22:49

sunshowers reviewed Aug 27, 2026

View reviewed changes

Comment thread nextest-runner/src/test_command.rs Outdated

gaborbernat added 2 commits August 27, 2026 16:19

[nextest-runner] trim spawn comments to the why

b4c8a75

Every comment the spawn lock added restated what the code does before saying why. Keep the reasons and drop the restatements.

gaborbernat marked this pull request as ready for review August 28, 2026 00:15

gaborbernat requested a review from sunshowers August 28, 2026 00:39

sunshowers reviewed Aug 28, 2026

View reviewed changes

gaborbernat requested a review from sunshowers August 28, 2026 22:41

[nextest-runner] tighten spawn lock comments

6673115

This was referenced Aug 30, 2026

Fix bootstrap capture handle leak tox-dev/peryx#1869

Closed

📝 docs(ci): record what a nextest LEAK report means tox-dev/peryx#2159

Merged

Adopt a nextest release that fixes the capture leak tox-dev/peryx#2160

Closed

sunshowers reviewed Sep 4, 2026

View reviewed changes

gaborbernat mentioned this pull request Sep 8, 2026

👷 ci: stop a leak deciding a mutant's fate tox-dev/peryx#2235

Merged

gaborbernat added 2 commits September 9, 2026 13:40

[nextest-runner] clarify spawn comments

c050a96

gaborbernat requested a review from sunshowers September 10, 2026 04:02

sunshowers reviewed Sep 10, 2026

View reviewed changes

gaborbernat marked this pull request as draft September 10, 2026 17:32

[nextest-runner] shorten spawn comments

c4a4e49

Move the race explanation onto PROCESS_SPAWN_LOCK, drop the restatements elsewhere, and describe what `LingeringChildren` and the shell command do.

gaborbernat marked this pull request as ready for review September 15, 2026 23:03

gaborbernat requested a review from sunshowers September 15, 2026 23:03

sunshowers approved these changes Sep 15, 2026

View reviewed changes

sunshowers merged commit e4ed09c into nextest-rs:main Sep 15, 2026
42 checks passed

gaborbernat deleted the fix/macos-capture-inheritance branch September 16, 2026 00:06

gaborbernat mentioned this pull request Sep 19, 2026

⬆️ chore(deps): update nextest tox-dev/peryx#2280

Merged

worktrunk-bot mentioned this pull request Sep 20, 2026

ci: bump cargo-affected and cargo-nextest pins max-sixty/worktrunk#4196

Merged

This was referenced Sep 23, 2026

Leaky nextest passes on macOS: concurrent spawns inherit a sibling test's capture pipe (nextest < 0.9.145) withpointbreak/pointbreak#814

Closed

ci: bump cargo-nextest to 0.9.146 withpointbreak/pointbreak#816

Merged

renardeinside mentioned this pull request Sep 25, 2026

🔧 Fix macOS leak failures and cache release builds volarized/rift#390

Merged

jscott3201 mentioned this pull request Sep 30, 2026

test: investigate intermittent nextest resource leak annotations jscott3201/rusty-bacnet#751

Closed

MarcusKainth mentioned this pull request Sep 30, 2026

[task] the recommended nextest version is behind the one CI runs spate-etl/spate#839

Open

		/// Keep children alive after closing stdout and stderr so an inherited
		/// writer in a sibling delays EOF.

Uh oh!

Conversation

gaborbernat commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sunshowers commented Aug 27, 2026

Uh oh!

sunshowers commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sunshowers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaborbernat commented Sep 2, 2026

Uh oh!

sunshowers commented Sep 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

grishy commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunshowers Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

sunshowers Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

sunshowers Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

sunshowers Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

sunshowers commented Sep 15, 2026

Uh oh!

gaborbernat commented Sep 15, 2026

Uh oh!

sunshowers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaborbernat commented Sep 16, 2026

Uh oh!

sunshowers commented Sep 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gaborbernat commented Aug 27, 2026 •

edited

Loading

codecov Bot commented Aug 27, 2026 •

edited

Loading

sunshowers commented Aug 27, 2026 •

edited

Loading

grishy commented Sep 7, 2026 •

edited

Loading