Skip to content

Don't run a memoize's producer on a model torn down mid re-evaluation - #88

Merged
mansbernhardt merged 1 commit into
mainfrom
claude/memoize-on-destructed-model
Sep 26, 2026
Merged

mansbernhardt merged 1 commit into
mainfrom
claude/memoize-on-destructed-model

Conversation

@mansbernhardt

Copy link
Copy Markdown
Collaborator

Fixes the unattributed memoize(...) on an unanchored model node is not allowed issues reported from parallel-apple's ParallelEditorTests. That suite records 57 of them per run, so xcodebuild exits nonzero even though the log says TEST SUCCEEDED, which blocks their PR #2407.

Mechanism

  • A teardown cuts a model's parent links before it destructs the model and cancels its memoizes, and that parent-link change wakes memoizes that read ancestors.
  • On the withObservationTracking path the re-evaluation runs on the background queue. One already past its hasBeenCancelled check blocks on the context lock until the cascade finishes, then runs produce() on a model with no ancestors.
  • The downstream accessor node.memoize { node.member(in: .ancestors) ?? TimelineModel() } then falls back to a fresh, never-anchored TimelineModel. A memoize on that model (mediaModel(for:)) reports from the background queue after the test has finished, so the issue is attributed to no test.

Fix

Narrow and local. Once inside the context lock, the memoize update closure checks whether the model has been destructed. If it has, it serves the last produced value instead of running the producer. The result would have been discarded anyway, because teardown clears the cache.

Evidence

MemoizeDuringTeardownTests reproduces the downstream shape: a child memoize resolves an ancestor, falls back to a fresh model, and calls a memoize on the result.

  • Without the fix: over 200 iterations the producer ran on a torn-down model 293 times and recorded 186 unattributed issues (ModelNode.swift:551, the downstream signature).
  • With the fix: 0 and 0. The test now runs 60 iterations, where the failure without the fix would still show roughly 90 fallbacks.
  • Full suite: 3 parallel runs and 1 serial, all green, with no compiler warnings. Its only remaining ModelNode.swift:551 hits are the deliberate withKnownIssue tests of memoize on a never-anchored model.

CHANGELOG entry added under [Unreleased].

🤖 Generated with Claude Code

Teardown cuts parent links before it destructs a model and cancels its
memoizes; the parent-link change wakes memoizes that read ancestors. A
withObservationTracking re-evaluation 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) ?? Fallback() }` then called
memoize on the never-anchored fallback, reporting 'unanchored model node'
from the background queue, attributed to no test (xcodebuild exits
nonzero; 57 per downstream ParallelEditorTests run).

The update closure now checks the model's lifetime under the context lock
and serves the last produced value once it has been destructed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mansbernhardt
mansbernhardt merged commit 70a0d8b into main Sep 26, 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