Report the error that caused in_parallel_with_slice() to stop, fixing a flaky test#2708
Merged
Sebastian Thiel (Byron) merged 2 commits intoJul 11, 2026
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f22c4d632e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
Previously, when a consumer failed, worker threads were joined in thread-id order and the first error encountered was returned. Threads that merely noticed the stop signal can bail out with a reactive error like `Interrupted` though, and when such a thread had a lower id than the failing one, that reaction would mask the actual cause of the stop. Now the first consumer to fail owns the causal error, and it is preferred over reactive errors when the join results are collected. Causality is tracked on a flag private to the harness as consumers may set the exposed stop flag themselves before returning their error. This makes the error of `Tree::traverse()` in `gix-pack` deterministic, and with it the previously intermittent assertion on `OutOfMemory` in `bundle::write_to_directory::respects_alloc_limit_bytes` (GitoxideLabs#2701). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nik B (nikicat)
force-pushed
the
pr-parallel-causal-error
branch
from
July 10, 2026 07:34
f22c4d6 to
b0aea79
Compare
Contributor
Author
|
Codex (@codex) review |
Sebastian Thiel (Byron)
force-pushed
the
pr-parallel-causal-error
branch
from
July 11, 2026 15:55
dbcd828 to
4fe6250
Compare
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
2 participants
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 #2701.
The race
When a consumer in
in_parallel_with_slice()fails, the failing thread sets the shared stop flag and returns its error. Other worker threads observe the flag and bail out with a reactive error — inTree::traverse()that isError::Interrupted(resolve.rs). Threads were then joined in thread-id order and the first error encountered was returned, so whenever a merely-interrupted thread had a lower id than the actually-failing one,Interruptedmasked the real cause.That is exactly the mechanism Amey Pawar (@ameyypawar) proposed in #2701:
respects_alloc_limit_bytesruns withthread_limit: None, every worker races to fail withOutOfMemory, and sometimes the surfaced error chain only containsInterrupted, failing the assertion on "Entry too large to fit in memory". Besides the Windows runs in the issue, the same failure showed up ontest(ubuntu) andtest-32bit (i386)in the runs of #2704/#2705, e.g. this job — so it is not Windows-specific.The fix
The first consumer to fail marks its error as the cause, and when collecting join results the causal error is preferred over reactive ones. Causality is tracked on a flag private to the harness (
swapdecides the winner) rather than on the stop flag itself, since consumers receive the stop flag and may legitimately set it themselves before returning an error (asgitoxide-core's corpus engine does) — Codex rightly pointed out that an earlier revision of this PR would misclassify the causal error in that case. External interrupts are unaffected: there no consumer failure precedes the stop signal, so the first reactive error (Interrupted) is returned as before.This fixes the library-level error masking rather than pinning the test to a single thread, so the test keeps exercising the multi-threaded path — callers now deterministically see the error that actually stopped the traversal instead of a bystander's
Interrupted.Testing
gix-features, each failing deterministically without its part of the fix: one for the plain masking race behind The testbundle::write_to_directory::respects_alloc_limit_bytesfails intermittently on Windows #2701, one for a consumer that sets the stop flag itself before failing.cargo nextest run -p gix-pack --all-features(100 tests) and 60 consecutive stress runs of thealloc_limittests, all green.cargo nextest run -p gix-featureswith default/parallel/all features.🤖 Generated with Claude Code