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

CHANGELOG.md

@@ -6,6 +6,12 @@ All notable changes are documented here. The format follows [Keep a Changelog](h
 
 ## [Unreleased]
 
+### Fixed
+
+- **A model dependency shared across model trees could deadlock the process.** `AnyContext.dependency(for:)` resolves a model dependency while holding its own tree's hierarchy lock, and the resolution copied the dependency model through `MakeInitialDependencyCopyTransformer` → `Model.shallowCopy` → `ModelContext.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 took the two locks in opposite order and deadlocked. Every other thread that then wanted either lock — in a `.modelTesting` run, including the executor the wait verbs drain — queued behind them, so a two-thread deadlock became a whole-process hang in which every `expect` rode to its ceiling and reported a timeout. It was diagnosed from a live `sample` of a hung CI run showing four threads in `__psynch_mutexwait` with none holding-and-running, which is what an AB-BA looks like; it reproduced at roughly 3% of full test-plan runs on 1.0.16 and 1.0.17 alike, and neither the drain queue's QoS nor the number of cores affected it.
+
+  The transformer now resolves genesis state *before* copying rather than after, which removes the foreign-lock acquisition entirely — and the frozen state it used to take that lock 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, which is exactly the case in which `shallowCopy` would freeze under a foreign lock; and `!_hasGenesis` implies the Reference was never anchored, so it has no context and no lock to take. The retained fallback is therefore 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. `DependencyLockInversionTests` is the regression test: two trees, two `@Model`-typed `static let` dependencies resolved concurrently in opposite order, with a progress-based verdict rather than a wall-clock one so a slow machine cannot fail it.
+
 ---
 
 ## [1.0.17] — Hot-path performance: index-keyed accessors, lock-free registrar lookups, 10× faster collection reconcile

Sources/SwiftModel/Internal/ModelContainer+Internals.swift

@@ -78,31 +78,56 @@ private struct MakeInitialTransformer: ModelTransformer {
 
 private struct MakeInitialDependencyCopyTransformer: ModelTransformer {
     func transform<M: Model>(_ model: inout M) -> Void {
-        // Capture the original reference BEFORE shallowCopy may convert an anchored model
-        // to a frozen snapshot. shallowCopy creates a brand-new Reference with no genesis,
-        // losing the original's _genesisState. We need the original to recover genesis.
-        // For pre-anchor models, shallowCopy returns self unchanged, so originalRef === srcRef.
-        let originalRef = model.modelContext._source.reference
-        model = model.shallowCopy
-        // Create a fresh Reference with a new identity and copy state from the frozen copy.
-        // Without state, Context.init's `hasState` assertion would fire when anchoring this copy.
-        let srcRef = model.modelContext._source.reference
+        let src = model.modelContext._source
+        let originalRef = src.reference
+
         // Prefer genesis state when available — matching reserveOrFork() — so that a fresh
         // dependency copy always starts from the clean initial state, not from mutations
         // made by a concurrently-running test that has the same static `testValue` anchored.
-        // Use originalRef for genesis since shallowCopy may have replaced srcRef with a
-        // frozen snapshot reference that has no _hasGenesis/_genesisState of its own.
-        // If state has been cleared post-TTL but genesis was captured, genesis is also used.
-        // If neither live state nor genesis is available, bail out and leave model.context
-        // intact; setupModelDependency guards against this residual non-nil context.
-        let genesisRef = originalRef._hasGenesis ? originalRef : srcRef
-        guard !srcRef._stateCleared || genesisRef._hasGenesis else { return }
-        let sourceState = genesisRef._hasGenesis ? genesisRef._genesisState : srcRef.state
-        let newRef = Context<M>.Reference(modelID: .generate(), state: sourceState)
-        if genesisRef._hasGenesis {
-            newRef._genesisState = genesisRef._genesisState
+        //
+        // Genesis is resolved BEFORE any `shallowCopy`, and that ordering is load-bearing
+        // for more than efficiency. `shallowCopy` freezes an anchored model by reading its
+        // state under **that model's** hierarchy lock (`makeFrozen` →
+        // `Reference.withHierarchyLockIfLive`). A shared `static let` dependency value is
+        // routinely anchored in a *different* tree, so that is a foreign lock — and this
+        // transformer runs inside `AnyContext.dependency(for:)`, which already holds its
+        // own tree's hierarchy lock. Two threads resolving two such dependencies in
+        // opposite order took the two locks A→B and B→A and deadlocked the process
+        // (diagnosed 2026-09-07 from a live `sample` of a hung CI run).
+        //
+        // Taking the genesis path first removes that edge entirely, because the frozen
+        // state was never used in the first place: `Reference.setContext` captures genesis
+        // on the very first anchor, so *every* Reference that has (or has ever had) a live
+        // context has genesis — exactly the case in which `shallowCopy` would take a
+        // foreign hierarchy lock. Conversely `!_hasGenesis` implies the Reference was never
+        // anchored, so it has no context and `withHierarchyLockIfLive` finds no lock to
+        // take. The fallback below is therefore foreign-lock-free by construction.
+        if originalRef._hasGenesis {
+            let sourceState = originalRef._genesisState
+            let newRef = Context<M>.Reference(modelID: .generate(), state: sourceState)
+            newRef._genesisState = sourceState
             newRef._hasGenesis = true
+            // `shallowCopy` drops the access when it freezes an anchored model; preserve
+            // that here, under the same condition, now that we skip the copy.
+            if !src._isLive, originalRef.context != nil {
+                model.modelContext.access = nil
+            }
+            model.modelContext.setReference(newRef)
+            return
         }
+
+        // No genesis — the Reference was never anchored (pre-anchor value, or a
+        // frozen/lastSeen snapshot). `shallowCopy` cannot reach a live context's hierarchy
+        // lock from here; it either returns `self` or freezes a snapshot Reference, whose
+        // `_context` is always nil.
+        model = model.shallowCopy
+        // Create a fresh Reference with a new identity and copy state from the frozen copy.
+        // Without state, Context.init's `hasState` assertion would fire when anchoring this copy.
+        let srcRef = model.modelContext._source.reference
+        // If state has been cleared post-TTL and no genesis was captured, bail out and leave
+        // model.context intact; setupModelDependency guards against this residual non-nil context.
+        guard !srcRef._stateCleared else { return }
+        let newRef = Context<M>.Reference(modelID: .generate(), state: srcRef.state)
         model.modelContext.setReference(newRef)
     }
 }

Tests/SwiftModelTests/DependencyLockInversionTests.swift

@@ -0,0 +1,155 @@
+import Testing
+import Foundation
+@testable import SwiftModel
+import SwiftModel
+import Dependencies
+
+// WASI is single-threaded and has neither `Thread` nor `DispatchSemaphore`, so the race
+// this reproduces cannot occur there and the test cannot be expressed.
+#if !os(WASI)
+
+/// Reproduction for the AB-BA deadlock between two contexts' hierarchy locks.
+///
+/// `AnyContext.dependency(for:)` holds ITS OWN hierarchy lock while resolving a model
+/// dependency, and that resolution copies the dependency model — `initialDependencyCopy`
+/// → `shallowCopy` → `Reference.lifetime` / `makeFrozen` → **another context's** hierarchy
+/// lock. Two threads resolving dependencies that reach into each other's trees acquire the
+/// two locks in opposite order and deadlock; a live `sample` of a wedged CI run on
+/// 2026-09-07 showed exactly that, with the drive executor queued behind one of them, which
+/// is what turns it into a whole-process hang.
+///
+/// Each iteration anchors two independent trees and resolves a model dependency on both
+/// concurrently. The work runs on detached threads with a wall-clock bound so a regression
+/// reports instead of hanging the suite.
+@Model private struct DepA: Sendable {
+    var value = 1
+}
+extension DepA: DependencyKey {
+    static let liveValue = DepA(value: 1)
+    static let testValue = DepA(value: 1)
+}
+
+@Model private struct DepB: Sendable {
+    var value = 2
+}
+extension DepB: DependencyKey {
+    static let liveValue = DepB(value: 2)
+    static let testValue = DepB(value: 2)
+}
+
+@Model private struct LeafOne: Sendable {
+    @ModelDependency var a: DepA
+    @ModelDependency var b: DepB
+    var touched = 0
+    func touch() { touched = a.value &+ b.value }
+}
+
+@Model private struct LeafTwo: Sendable {
+    @ModelDependency var b: DepB
+    @ModelDependency var a: DepA
+    var touched = 0
+    func touch() { touched = b.value &+ a.value }
+}
+
+private final class Flag: @unchecked Sendable {
+    private let l = NSLock(); private var v = false
+    var value: Bool { get { l.lock(); defer { l.unlock() }; return v } set { l.lock(); v = newValue; l.unlock() } }
+}
+
+private final class Counter: @unchecked Sendable {
+    private let l = NSLock(); private var v = 0
+    var value: Int { l.lock(); defer { l.unlock() }; return v }
+    func increment() { l.lock(); v += 1; l.unlock() }
+}
+
+struct DependencyLockInversionTests {
+    /// Regression test for the AB-BA fixed by giving `modeLifeTime` its own leaf lock and
+    /// by resolving genesis state before `shallowCopy` in `MakeInitialDependencyCopyTransformer`
+    /// — the two places where `dependency(for:)` reached a *foreign* hierarchy lock while
+    /// holding its own. Deadlocked on the very first iteration before the fix (zero
+    /// iterations completed); completes 200 iterations in ~0.2 s after it.
+    ///
+    /// The verdict is a *stall* detector rather than a total-runtime budget — see the
+    /// comment in the body — so a regression reports instead of hanging the suite, without
+    /// the pass/fail line depending on how loaded the machine is.
+    ///
+    /// Note for future fixers: removing the `lock { }` from `dependency(for:)` altogether
+    /// (with a first-wins dependency-cache install) also makes this pass, but crashes the
+    /// full parallel suite deterministically inside Swift-runtime generic-metadata
+    /// instantiation. That lock does more than guard the cache check-then-act; the fix has
+    /// to remove the foreign-lock edges from under it, not remove the lock.
+    @Test func concurrentDependencyResolutionAcrossTreesDoesNotDeadlock() async {
+        let iterations = 200
+        let done = Flag()
+        let stop = Flag()
+        let progress = Counter()
+
+        let worker = Thread {
+            for _ in 0..<iterations {
+                if stop.value { break }
+                let (one, anchorOne) = LeafOne().returningAnchor()
+                let (two, anchorTwo) = LeafTwo().returningAnchor()
+                let ready = DispatchSemaphore(value: 0)
+                let go = DispatchSemaphore(value: 0)
+                let finished = DispatchSemaphore(value: 0)
+                for model in [{ one.touch() }, { two.touch() }] as [@Sendable () -> Void] {
+                    Thread {
+                        ready.signal(); go.wait()
+                        model()
+                        finished.signal()
+                    }.start()
+                }
+                ready.wait(); ready.wait()
+                go.signal(); go.signal()
+                finished.wait(); finished.wait()
+                progress.increment()
+                withExtendedLifetime((anchorOne, anchorTwo)) {}
+            }
+            done.value = true
+        }
+        worker.start()
+
+        // Evidence-based verdict, not a wall-clock budget: an AB-BA parks both resolver
+        // threads forever, so the iteration counter stops moving and never restarts. A
+        // merely slow run (TSan is 5-15x, and this suite runs in parallel with 860+ other
+        // tests) keeps incrementing it. So we fail only on a *stall* — no forward progress
+        // for `stallBudget` — and treat "still progressing when the overall ceiling is
+        // reached" as a pass, since sustained forward progress is exactly what the test
+        // asserts. Both bounds honour SWIFT_MODEL_TIMEOUT_SCALE like the rest of the suite.
+        let scale = ProcessInfo.processInfo.environment["SWIFT_MODEL_TIMEOUT_SCALE"].flatMap(Double.init) ?? 1
+        let stallBudget = 10.0 * scale
+        let ceiling = Date().addingTimeInterval(120 * scale)
+
+        var lastProgress = progress.value
+        var lastProgressAt = Date()
+        var stalled = false
+        while !done.value && Date() < ceiling {
+            try? await Task.sleep(nanoseconds: 20_000_000)
+            let current = progress.value
+            if current != lastProgress {
+                lastProgress = current
+                lastProgressAt = Date()
+            } else if Date().timeIntervalSince(lastProgressAt) > stallBudget {
+                stalled = true
+                break
+            }
+        }
+        stop.value = true
+
+        #expect(
+            !stalled,
+            """
+            concurrent cross-tree dependency resolution made no progress for \(stallBudget)s \
+            after \(lastProgress)/\(iterations) iterations — the two hierarchy locks are \
+            deadlocked again
+            """
+        )
+        // Let the worker unwind before the test returns so its threads do not outlive it.
+        while !done.value && !stalled && Date() < ceiling.addingTimeInterval(5 * scale) {
+            try? await Task.sleep(nanoseconds: 20_000_000)
+        }
+    }
+}
+
+
+#endif