Fix CollectionViewLayout delegate lifetime#611
Merged
RoyalPineapple merged 1 commit intomainfrom May 2, 2026
Merged
Conversation
2fcf7ae to
4703f72
Compare
Hold the layout delegate weakly instead of unowned so the layout no longer crashes if the delegate is deallocated before the layout. prepare() and the deferred reorder flush now snapshot the delegate locally; LayoutManager's layout-swap path uses listableInternalFatal if the previous layout's delegate is missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4703f72 to
99bfb40
Compare
RoyalPineapple
added a commit
that referenced
this pull request
May 2, 2026
This reverts commit c3245d3.
RoyalPineapple
added a commit
that referenced
this pull request
May 2, 2026
Reverts #611 Which was accidentally automerged
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.
Problem
CollectionViewLayoutheld its delegate asunowned let. In normal use theListViewownership graph keeps the delegate alive for the layout's whole lifetime, but a layout swap (LayoutManager.set(layout:)) or test setup that constructs a layout in isolation can deallocate the delegate first, leaving a danglingunownedreference and a hard crash on next access.Fix
Change
delegatefromunowned lettoprivate(set) weak var, and tighten the surrounding call sites:prepare()reads the delegate once at the top, returns early if it's gone, and threads the unwrapped reference throughperformLayout,performRebuild, andperformLayoutUpdateto avoid per-call weak loads.initkeeps a non-optional delegate parameter; the weak storage already conveys "may vanish later," and the parameter keeps construction honest.LayoutManager.set(layout:)useslistableInternalFatalif the outgoing layout's delegate has been deallocated, since silently dropping a layout swap is worse than crashing on a "should never happen" state.sendEndQueuingEditsAfterDelay()snapshots the delegate locally and weak-captures it, so the closure neither pinsselfnor extends the delegate's lifetime past its natural end.Test
Adds
CollectionViewLayoutTests.test_prepare_whenDelegateHasBeenDeallocated, which constructs a layout with a mock delegate, drops the strong reference, asserts the weak references are nil, and callsprepare()to verify it doesn't crash.Verified locally that the test fails against
origin/main's source and passes with this PR's fix.Before (against
origin/main'sunowned let delegate):After (this PR):
Risk
Low.
ListViewretains itsDelegatefor its entire lifetime, so in normal flows the delegate is alive every time the layout uses it. The fix is defense-in-depth for layout swaps and isolated-construction paths.Checklist
Mainsection.