Fix concurrency/file-presence findings from Stage 1 fact-finding - #1283
Merged
Merged
Conversation
Implements the four findings confirmed by the prior fact-finding investigation (memory: concurrency-file-presence-factfinding), plus fixes the release-pipeline mutex issue identified alongside it: - Finding 1: release pipeline's `skipped` counter race -- ReleaseState gains a lock field, the increment is synchronized, exactly matching the test pipeline's own established pattern. No validation experiment needed (already a well-understood pattern). - Finding 3 (prime suspect for sporadic Windows file-miss reports): new FileWrapper#open_with_retry/#exist_with_retry? ride out a transient failure to read/see a file this same build just produced via a shell-out, instead of crashing or silently skipping on the first attempt. Applied at all ten preprocessing call sites that read gcc output. Both exhaustion paths leave a clear trace: a permanent open failure raises a filepath-naming CeedlingException; a permanent absence logs a NOTICE before falling back to the caller's existing false-branch behavior. - Finding 2: exist_with_retry? applied at all eight .d-dependency-file presence checks in the test and release pipelines, closing most of the "silently registers one recompile late" gap directly. - Finding 4: documented the two-pass compile barrier invariant that makes apply_sibling_isolation's unguarded search_paths writes safe today, plus a committed integration-tier canary spec (the right-sized version of Stage 1's Experiment D) that will fail if the barrier is ever merged away or reordered. New spec/integration/preprocessing_transient_io_retry_spec.rb and spec/integration/test_build_parallel_barrier_spec.rb commit Stage 1's Experiments C and D as permanent regression coverage. Every existing spec touching a changed call site was updated to track the renamed collaborator method and stays green. Verified: full unit + integration suites green on host (macOS) and the madsciencelab-plugins Docker image (Linux); targeted preprocessing/release system specs green on both. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Implements fixes for the four concurrency/file-presence findings identified in a prior fact-finding investigation of Ceedling's test (and, more lightly, release) build pipelines, plus the release-pipeline mutex issue found alongside them. Findings and experimental evidence are recorded in this session's memory (
concurrency-file-presence-factfinding); this PR is the follow-on fix-and-validate stage.Finding 1 — release pipeline counter race.
ReleaseBuildExecutor#compile_objects'sskipped += 1was an unguarded compound read-modify-write across worker threads.ReleaseStategains a:lockfield, defaulted automatically; the increment is now synchronized — exactly matching the test pipeline's own established pattern. No validation experiment needed (well-understood pattern, direct fix).Finding 3 — no-guard preprocessing read (prime suspect for sporadic Windows file-miss reports). A file a shell-out (gcc) just wrote wasn't guaranteed visible to the very next read — surfacing as an uncaught, unretried crash. New
FileWrapper#open_with_retry/#exist_with_retry?(four retries, ~0.37s worst case, scoped toSystemCallErroronly so a real programming bug is never masked) ride out a transient failure instead of crashing on the first attempt. Applied at all ten preprocessing call sites that read gcc output. A permanently-failing open now raises a filepath-namingCeedlingException; a permanently-missing file logs a NOTICE before falling back to prior behavior.Finding 2 — silent dependency-loss on a missed
.dfile. Same retry mechanism (exist_with_retry?) applied at all eight.d-dependency-file presence checks in the test and release pipelines, closing most of the "self-corrects into one extra recompile" gap directly.Finding 4 — two-pass compile barrier invariant. Confirmed safe today and load-bearing, not incidental. No code change — documented the invariant with explicit comments at
stage_build_objects/apply_sibling_isolation, plus a committed integration-tier canary spec that will fail if the barrier is ever merged away or reordered.Tests
spec/integration/preprocessing_transient_io_retry_spec.rbandspec/integration/test_build_parallel_barrier_spec.rbcommit the fact-finding phase's deterministic fault-injection experiments as permanent regression coverage.file_wrapper_spec.rb(stub-based, no real filesystem/timing) prove the retry mechanism itself; call-site specs updated to track the renamed collaborator methods and stay green.Verification
madsciencelab-pluginsDocker image (Linux).delta_builds_spec.rbconfirmed present on unmodifiednext_versiontoo, and passes cleanly in Docker).🤖 Generated with Claude Code