Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ All notable changes are documented here. The format follows [Keep a Changelog](h

## [Unreleased]

### Fixed

- **A memoize re-evaluated during its model's teardown ran its producer on the torn-down model, recording unattributed "memoize(...) on an unanchored model node" issues.** A teardown cuts a model's parent links before it destructs the model and cancels its memoizes, and the parent-link change wakes memoizes that read ancestors. On the `withObservationTracking` path the re-evaluation is scheduled on the background queue; one already past its cancellation check blocked on the context lock until the cascade finished, then ran `produce()` on a model with no ancestors. A producer like `node.memoize { node.member(in: .ancestors) ?? TimelineModel() }` then fell back to a fresh, never-anchored model, and the memoize it called on that model reported. The report was raised on the background queue after the test had finished, so it was attributed to no test and made `xcodebuild` exit nonzero despite `TEST SUCCEEDED` (57 per `ParallelEditorTests` run downstream).
- Once inside the context lock, the memoize's update closure now checks whether the model has been destructed, and if so serves its last produced value instead of running the producer. The result would have been discarded anyway, because teardown clears the cache.
- `MemoizeDuringTeardownTests` reproduces the downstream shape. Without the fix, 200 iterations ran the producer on a torn-down model 293 times and recorded 186 unattributed issues; with it, zero and zero.

---

## [1.1.0] — Signals (`onSignal` / `signal` / `onTeardown`) + `TestPredicate` `==` no longer leaks into app code
Expand Down
25 changes: 22 additions & 3 deletions Sources/SwiftModel/Model+Changes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ private extension ModelNode {
let useWithObservationTracking = context.hasObservationRegistrar && useCoalescing
let usesAsyncTracking = useWithObservationTracking // Capture for cache entry

// The last value `produce()` returned, served instead of re-producing once
// this model has been torn down (see the destructed check below).
let lastProduced = LockIsolated<T?>(nil)

return update(
initial: true,
isSame: nil,
Expand Down Expand Up @@ -684,14 +688,29 @@ private extension ModelNode {
// performUpdate's access() call. This avoids spurious extra computes from
// other access() call sites (forceObserver setup, initial registration,
// dirty-path sync reads).
let (entry, version): (AnyContext.MemoizeCacheEntry?, UInt64) = context.lock {
let (entry, version, isTornDown): (AnyContext.MemoizeCacheEntry?, UInt64, Bool) = context.lock {
let entry = context._memoizeCache[key]
return (entry, entry?.dirtyVersion ?? 0)
return (entry, entry?.dirtyVersion ?? 0, context.unprotectedIsDestructed)
}
// Torn down: never run the producer on a removed model. A teardown
// cuts parent links before it destructs a model and cancels its
// memoizes, and the parent-link change wakes memoizes that read
// ancestors. A re-evaluation already scheduled past the cancellation
// check blocks on `context.lock` above until the cascade finishes,
// and would then run `produce()` on a model with no ancestors — e.g.
// falling back to a fresh, never-anchored model and calling memoize on
// it, reporting "unanchored model node" unattributed to any test. The
// result would be discarded anyway (the cache is gone), so serve the
// last produced value.
if isTornDown, let last = lastProduced.value {
return (last, version)
}
if !threadLocals.isInsideAsyncPerformUpdate, let entry = entry, !entry.isDirty {
return (entry.value as! T, version)
}
return (produce(), version)
let value = produce()
lastProduced.setValue(value)
return (value, version)
} onUpdate: { @Sendable (produced: (value: T, version: UInt64)) in
let value = produced.value
var postLockCallbacks: [() -> Void] = []
Expand Down
56 changes: 56 additions & 0 deletions Tests/SwiftModelTests/MemoizeDuringTeardownTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import Testing
@testable import SwiftModel
import ConcurrencyExtras
import IssueReporting

// A memoize whose producer resolves an ancestor, falling back to a fresh (never-anchored)
// model when there is none, then calls a memoize on the result — the shape of a
// real-world `timelineModel` accessor (`node.memoize { node.member(in: .ancestors) ??
// TimelineModel() }`). Tearing the tree down cuts the child's parent link first; that
// wakes the child's memoizes, and a re-evaluation racing the teardown cascade ran the
// producer on a model being torn down: the ancestor was gone, the fallback was used, and
// memoize on the never-anchored fallback reported "memoize(...) on an unanchored model
// node is not allowed" — unattributed to any test, after the test had finished.
struct MemoizeDuringTeardownTests {
@Test func reevaluationRacingTeardownDoesNotRunTheProducer() async {
// The reports are raised on the background queue, outside any task-local issue
// reporter (they surface as unattributed issues), so assert on their cause: the
// producer running on a torn-down model, i.e. the ancestor fallback being taken.
let fallbacks = LockIsolated(0)
for _ in 0..<60 {
await withModelTesting(exhaustivity: .off) {
let root = Timeline(segments: (0..<8).map { _ in Segment(fallbacks: fallbacks) }).withAnchor()
for segment in root.segments { _ = segment.visibleCount }
}
}
await backgroundCall.waitUntilIdle(deadline: _monotonicNs() + 5_000_000_000)
#expect(fallbacks.value == 0)
}
}

@Model private struct Timeline {
var media: [Int] = [1, 2, 3]
var segments: [Segment]

var mediaCount: Int {
node.memoize { media.count }
}
}

@Model private struct Segment {
let fallbacks: LockIsolated<Int>

var timeline: Timeline {
node.memoize {
if let timeline = node.mapHierarchy(for: .ancestors, transform: { $0 as? Timeline }).first {
return timeline
}
fallbacks.withValue { $0 += 1 }
return Timeline(segments: [])
}
}

var visibleCount: Int {
node.memoize { timeline.mediaCount }
}
}
Loading