Skip to content

fix(graph): re-probe the cache after taking an action lock - #428

Merged
fortmarek merged 1 commit into
mainfrom
fix/action-lock-reprobe
Nov 9, 2026
Merged

fortmarek merged 1 commit into
mainfrom
fix/action-lock-reprobe

Conversation

@fortmarek

Copy link
Copy Markdown
Member

Summary

Two targets that declare the same cacheable action could both run it. This is what made spec/apple_explicit_modules_spec.sh:75 fail intermittently on shellspec (macos-latest) (for example once run 42596832361 on main), where Left and Right compile the same Clang modules and the spec expects one of them to reuse the other's result.

The race:

  1. Target B probes the cache and misses, because target A is still running the action.
  2. B waits for an execution slot, which A holds.
  3. A stores its result, then releases the slot and the action lock together.
  4. B takes the lock without contention. The re-probe only ran on contention, so B ran the action again.

The fix always re-probes after taking the lock and drops the now-unused contention flag from with_action_lock_and_permit. The cost is one extra cache lookup per action that actually executes; with a remote cache, a true miss asks the remote twice.

Verification

  • Reproduced on macOS 27 under load (6 parallel loops of the spec's steps with --cpu-limit 3): 18 of 36 runs failed, all at the shared .pcm check, with both targets recording miss for the same NativeValue.pcm, SwiftShims.pcm, and _SwiftConcurrencyShims.pcm digests. With the fix, 36 of 36 passed.
  • shellspec spec/apple_explicit_modules_spec.sh: 3 examples, 0 failures.
  • New regression test an_action_queued_behind_its_own_run_reuses_that_result runs two builds of one action on a single slot. It fails on the old code (the action runs twice) and passes with the fix.
  • cargo test -p once-cli --bin once commands::graph::analysis, cargo fmt --check, and cargo clippy -p once-cli --all-targets -D warnings pass.

Two targets that declare the same cacheable action could both run it.
The second probed the cache while the first was running, then waited for
an execution slot. The first released the slot and the action lock
together once it stored its result, so the second took the lock without
contention and skipped the re-probe that only ran on contention.

This flaked spec/apple_explicit_modules_spec.sh on macOS CI, where Left
and Right compile the same Clang modules and neither recorded a hit.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1c12d-ae66-702f-b51a-5ab9df400da2
Co-authored-by: Marek Fort <marek@tuist.dev>
@fortmarek
fortmarek force-pushed the fix/action-lock-reprobe branch from 4880c97 to b9a35de Compare November 9, 2026 21:50
@fortmarek
fortmarek merged commit 56d4f49 into main Nov 9, 2026
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants