Don't run a memoize's producer on a model torn down mid re-evaluation - #88
Merged
Merged
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the unattributed
memoize(...) on an unanchored model node is not allowedissues reported from parallel-apple'sParallelEditorTests. That suite records 57 of them per run, soxcodebuildexits nonzero even though the log saysTEST SUCCEEDED, which blocks their PR #2407.Mechanism
withObservationTrackingpath the re-evaluation runs on the background queue. One already past itshasBeenCancelledcheck blocks on the context lock until the cascade finishes, then runsproduce()on a model with no ancestors.node.memoize { node.member(in: .ancestors) ?? TimelineModel() }then falls back to a fresh, never-anchoredTimelineModel. 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
MemoizeDuringTeardownTestsreproduces the downstream shape: a child memoize resolves an ancestor, falls back to a fresh model, and calls a memoize on the result.ModelNode.swift:551, the downstream signature).ModelNode.swift:551hits are the deliberatewithKnownIssuetests of memoize on a never-anchored model.CHANGELOG entry added under
[Unreleased].🤖 Generated with Claude Code