From 608fc0c70a6b1976da650c999f9f4ffa91a30ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Mon, 7 Sep 2026 18:54:42 +0200 Subject: [PATCH 1/3] Reproduce the cross-tree dependency AB-BA deadlock (known issue, not 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 --- .../DependencyLockInversionTests.swift | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 Tests/SwiftModelTests/DependencyLockInversionTests.swift diff --git a/Tests/SwiftModelTests/DependencyLockInversionTests.swift b/Tests/SwiftModelTests/DependencyLockInversionTests.swift new file mode 100644 index 0000000..29f7a55 --- /dev/null +++ b/Tests/SwiftModelTests/DependencyLockInversionTests.swift @@ -0,0 +1,103 @@ +import Testing +import Foundation +@testable import SwiftModel +import SwiftModel +import Dependencies + +/// 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() } } +} + +struct DependencyLockInversionTests { + /// KNOWN ISSUE — this deadlocks on `main` today. It is the reproduction for the + /// production bug diagnosed on 2026-09-07 from a live `sample` of a wedged CI run. + /// Wrapped in `withKnownIssue` so it documents the bug without reddening CI; delete the + /// wrapper when the fix lands. Bounded at 30 s so a regression reports instead of hanging. + /// + /// A first fix attempt — dropping the hierarchy lock from `AnyContext.dependency(for:)` + /// and making the dependency-cache install first-wins — DOES make this pass (0.15 s + /// instead of a 30 s deadlock) but crashes the full parallel suite deterministically + /// (3/3 runs) inside Swift-runtime generic-metadata instantiation, reached from a + /// generic `ModelAccess.willAccess` override in `ObservedModelDebugIsolationTests`. So + /// that lock is doing more than guarding the check-then-act, and the real fix has to + /// narrow what runs unlocked rather than remove the lock wholesale. + @Test func concurrentDependencyResolutionAcrossTreesDoesNotDeadlock() async { + let iterations = 200 + let done = Flag() + let progressed = Flag() + + await withKnownIssue("AB-BA between two contexts' hierarchy locks — see the doc comment") { + let worker = Thread { + for _ in 0.. Void] { + Thread { + ready.signal(); go.wait() + model() + finished.signal() + }.start() + } + ready.wait(); ready.wait() + go.signal(); go.signal() + finished.wait(); finished.wait() + progressed.value = true + withExtendedLifetime((anchorOne, anchorTwo)) {} + } + done.value = true + } + worker.start() + + let deadline = Date().addingTimeInterval(30) + while !done.value && Date() < deadline { try? await Task.sleep(nanoseconds: 20_000_000) } + #expect(done.value, "concurrent cross-tree dependency resolution deadlocked (progressed at least once: \(progressed.value))") + } + } +} From 8abbb4e8188d72600bf62013d230dd7f105bfbfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Mon, 7 Sep 2026 20:46:11 +0200 Subject: [PATCH 2/3] Fix a cross-tree deadlock when resolving a shared model dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 6 ++ .../Internal/ModelContainer+Internals.swift | 65 ++++++++++----- .../DependencyLockInversionTests.swift | 79 +++++++++++++++---- 3 files changed, 113 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d91f85..88cbb23 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Sources/SwiftModel/Internal/ModelContainer+Internals.swift b/Sources/SwiftModel/Internal/ModelContainer+Internals.swift index 452c6af..067dd18 100644 --- a/Sources/SwiftModel/Internal/ModelContainer+Internals.swift +++ b/Sources/SwiftModel/Internal/ModelContainer+Internals.swift @@ -78,31 +78,56 @@ private struct MakeInitialTransformer: ModelTransformer { private struct MakeInitialDependencyCopyTransformer: ModelTransformer { func transform(_ 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.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.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.Reference(modelID: .generate(), state: srcRef.state) model.modelContext.setReference(newRef) } } diff --git a/Tests/SwiftModelTests/DependencyLockInversionTests.swift b/Tests/SwiftModelTests/DependencyLockInversionTests.swift index 29f7a55..d5e1a95 100644 --- a/Tests/SwiftModelTests/DependencyLockInversionTests.swift +++ b/Tests/SwiftModelTests/DependencyLockInversionTests.swift @@ -52,27 +52,37 @@ private final class Flag: @unchecked Sendable { 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 { - /// KNOWN ISSUE — this deadlocks on `main` today. It is the reproduction for the - /// production bug diagnosed on 2026-09-07 from a live `sample` of a wedged CI run. - /// Wrapped in `withKnownIssue` so it documents the bug without reddening CI; delete the - /// wrapper when the fix lands. Bounded at 30 s so a regression reports instead of hanging. + /// 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. /// - /// A first fix attempt — dropping the hierarchy lock from `AnyContext.dependency(for:)` - /// and making the dependency-cache install first-wins — DOES make this pass (0.15 s - /// instead of a 30 s deadlock) but crashes the full parallel suite deterministically - /// (3/3 runs) inside Swift-runtime generic-metadata instantiation, reached from a - /// generic `ModelAccess.willAccess` override in `ObservedModelDebugIsolationTests`. So - /// that lock is doing more than guarding the check-then-act, and the real fix has to - /// narrow what runs unlocked rather than remove the lock wholesale. + /// 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 progressed = Flag() + let stop = Flag() + let progress = Counter() - await withKnownIssue("AB-BA between two contexts' hierarchy locks — see the doc comment") { let worker = Thread { for _ in 0.. 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) } } } From 6426b2ec632d2a092959ef49d3d6a55c61c1ad9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Tue, 8 Sep 2026 08:18:41 +0200 Subject: [PATCH 3/3] Guard the deadlock regression test out on WASI (no Thread / DispatchSemaphore) Co-Authored-By: Claude Fable 5.1 --- Tests/SwiftModelTests/DependencyLockInversionTests.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/SwiftModelTests/DependencyLockInversionTests.swift b/Tests/SwiftModelTests/DependencyLockInversionTests.swift index d5e1a95..5ec65d2 100644 --- a/Tests/SwiftModelTests/DependencyLockInversionTests.swift +++ b/Tests/SwiftModelTests/DependencyLockInversionTests.swift @@ -4,6 +4,10 @@ import Foundation 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 @@ -146,3 +150,6 @@ struct DependencyLockInversionTests { } } } + + +#endif \ No newline at end of file