Skip to content

Fix the cross-tree deadlock when resolving a shared model dependency - #73

Merged
mansbernhardt merged 3 commits into
mainfrom
fix/dependency-deadlock-minimal
Sep 8, 2026
Merged

mansbernhardt merged 3 commits into
mainfrom
fix/dependency-deadlock-minimal

Conversation

@mansbernhardt

Copy link
Copy Markdown
Collaborator

Fixes the chronic full-test-plan wedge that has been hitting parallel-apple at roughly 3% of runs, on 1.0.16 and 1.0.17 alike.

The bug. AnyContext.dependency(for:) resolves a model dependency while holding its own tree's hierarchy lock. The copy it performs went MakeInitialDependencyCopyTransformerModel.shallowCopyModelContext.makeFrozen, which reads the model's state under that model's hierarchy lock. A dependency declared as a static let is shared, so the model being copied is routinely anchored in a different tree. Two threads resolving two such dependencies in opposite order take the two locks in opposite order and deadlock. Everything else that then wants either lock queues behind them, including the executor the .modelTesting wait verbs drain, which is what turns a two-thread deadlock into a whole-process hang where every expect rides to its ceiling.

Diagnosed from a live sample of a hung CI run: four threads in __psynch_mutexwait, none holding-and-running, which is what an AB-BA looks like. Two of them had byte-identical stacks through dependency(for:)setupModelDependencyinitialDependencyCopyshallowCopy → the foreign lock. It is the same hazard the dependenciesLock comment in this file already documents for nearestDependencyContext, on a path that was missed.

The fix, one file. The transformer resolves genesis state before copying instead of after, which removes the foreign-lock acquisition — and the frozen state that acquisition existed for was never used on this path:

  • Reference.setContext captures genesis on the very first anchor, so every Reference that has, or ever had, a live context has genesis. That is exactly the case in which shallowCopy would freeze under a foreign lock (it only freezes when ref.context != nil).
  • Contrapositive: no genesis implies never anchored, hence no context, hence withHierarchyLockIfLive finds no lock. The retained fallback is foreign-lock-free by construction.
  • The one observable side effect of the skipped shallowCopy — clearing modelContext.access when it freezes an anchored model — is preserved under the identical condition.

No lock scope changes. dependency(for:) still holds its lock across setupModelDependency, which is what serialises the cache check-then-act and the dependency-context bookkeeping.

Regression test. DependencyLockInversionTests: two trees, two @Model-typed static let dependencies resolved concurrently in opposite order. Without the fix it deadlocks for its full bound; with it, 0.15 s. The verdict is progress-based rather than wall-clock, so a loaded or TSan-slowed machine cannot fail it — a real AB-BA freezes the iteration counter, a slow run keeps incrementing.

Gate. Parallel suite 4/4 clean with an explicit grep for host crashes rather than the summary line; serial clean; --loop 10 clean; TSan per ci.yml clean with zero warnings; release build warning-free.

Two approaches rejected, with evidence. Removing the lock from dependency(for:) and making the cache install first-wins fixes the deadlock but crashes the parallel suite 4/4 on a quiet machine inside Swift-runtime generic-metadata instantiation. Giving modeLifeTime its own leaf lock, so AnyContext.lifetime never takes the hierarchy lock, does the same: bisected to 2/2 crashes for that change alone, while the transformer change alone is 4/4 clean and sufficient. Both are recorded so they are not retried.

🤖 Generated with Claude Code

mansbernhardt and others added 3 commits September 7, 2026 18:54
…yet fixed)

Diagnosed from a live sample of a wedged CI run on 2026-09-07: four threads in
__psynch_mutexwait, none holding-and-running. AnyContext.dependency(for:) holds
its OWN hierarchy lock while resolving a model dependency, and that resolution
copies the dependency model (setupModelDependency -> initialDependencyCopy ->
Model.shallowCopy), which reads Reference.lifetime and may makeFrozen it - both
of which take the hierarchy lock of whatever context owns that model. A
dependency declared as a static let is shared across trees, so that is routinely
a DIFFERENT tree's lock, and two threads resolving two such dependencies in
opposite order deadlock. The test drive's executor queuing behind one of the two
locks is what turns it into a whole-process hang with every expect riding to its
1500 s ceiling.

Same hazard the dependenciesLock comment already documents for
nearestDependencyContext, on a path that was missed. Explains every measured
property: version-independent, unaffected by the QoS floor, not raised by extra
cores, ~3% per full-plan run, full-plan only, varying victim test.

The test deadlocks on main and is wrapped in withKnownIssue so it documents the
bug without reddening CI. A first fix attempt is recorded in its doc comment.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
AnyContext.dependency(for:) resolves a model dependency while holding its own
tree's hierarchy lock, and the copy it performs reached into the hierarchy lock
of whatever tree currently owns that model — routinely a different one, since a
static let dependency value is shared. Two threads resolving two such
dependencies in opposite order deadlocked, and everything else queued behind
them, including the executor the test wait verbs drain.

The transformer now resolves genesis before copying instead of after, which
removes the foreign-lock acquisition. The frozen state that acquisition existed
for was never used on this path: genesis is captured on the first anchor, so
every Reference that has ever had a context has it, which is exactly when the
copy would have frozen under a foreign lock; no genesis implies never anchored,
hence no context and no lock. The access-clearing side effect is preserved
under the identical condition.

Diagnosed from a live sample of a hung CI run; reproduced deterministically by
DependencyLockInversionTests, which deadlocks for its full bound without this
change and passes in 0.15 s with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…emaphore)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mansbernhardt
mansbernhardt merged commit 733eb26 into main Sep 8, 2026
7 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.

1 participant