Repository navigation
fix(ios): install the scroll delegate once per collection view - #1147
lvlrSajjad wants to merge 2 commits into
Conversation
`PagerScrollDelegate` keeps the delegate it displaced in `originalDelegate` and forwards unhandled selectors to it. The install block was gated on `originalDelegate == nil`, but that reference is weak, so it cannot tell "not installed yet" from "installed, but the weak reference died". The `.introspect` closure re-runs on every layout pass, so once the displaced delegate deallocated the block ran again and re-adopted `collectionView.delegate` - by then the `PagerScrollDelegate` itself. `responds(to:)` then called itself until the stack overflowed (EXC_BAD_ACCESS, code=2). This is reachable from ordinary use: SwiftUI rebuilds the TabView when `.id(props.children.count)` changes, i.e. whenever the page count changes. - Install once per collection view, keyed on the collection view's identity. A new collection view is still installed into, so a page-count change keeps working; the same one is never re-adopted, so the cycle cannot form. - Guard `responds(to:)` against re-entry, so a cycle formed another way terminates instead of overflowing. An analytics SDK that swizzles the delegate setter can insert a proxy that forwards back here, which makes the chain cyclic even when the delegate is not literally `self`. UIKit skips optional delegate methods that report `false`. - Reject a self-assignment to `originalDelegate` in `didSet`. Fixes callstack#1146 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolve the conflict in ios/PagerView.swift by keeping the iOS 26 edge effect hiding for the vertical pager (callstack#1149) ahead of the once-per-collection-view delegate install. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hello @lvlrSajjad |
|
@apex-sentinel-staging review |
Apex SentinelVerdict: 7 finding(s): 1 major, 1 minor, 5 nit. Verdict: request_changes. reached max rounds (1). Required fixes
🟠 Major (1)
🟡 Minor (1)
🔵 Nit (5)
Reply under an inline comment with |
![apex-sentinel-staging[bot]](https://avatars.githubusercontent.com/in/5196457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on bca73d1. The verdict and the full list are in the summary comment.
| } | ||
|
|
||
| if scrollDelegate.originalDelegate == nil { | ||
| // Install once per collection view. This closure re-runs on every |
There was a problem hiding this comment.
🟠 Major · Delegate-object identity gate: install never runs for a new collection view when the same @State PagerScrollDelegate is reused (weak installedCollectionView still resolves to the old one)
Open ledger item, settled as a behaviour regression against the predecessor.
| // Install once per collection view. This closure re-runs on every | |
| // Adopt ourselves unless we are already this collection view's delegate: | |
| // idempotent for the same view (no cycle), but still re-adopts after the | |
| // displaced delegate died or when this @State delegate is handed a new | |
| // collection view. | |
| if collectionView.delegate !== scrollDelegate { | |
| scrollDelegate.originalDelegate = collectionView.delegate | |
| scrollDelegate.delegate = delegate | |
| // VTabView-style rotation preserves TabView's horizontal collection view. | |
| scrollDelegate.orientation = .horizontal | |
| collectionView.delegate = scrollDelegate | |
| } |
Suggested change
Key the install decision on the collection view's current delegate (collectionView.delegate !== scrollDelegate) instead of the delegate's own weak memory identity, and cover the page-count-change-then-scroll case with a host-app/Maestro test asserting onPageScroll still fires.
Details
scrollDelegate is @State (ios/PagerView.swift:9), so it outlives the TabView identity churn from .id(props.children.count) (line 29) and can outlive re-creation of the hosting controller. The new gate compares the delegate's own weak var installedCollectionView with the collection view SwiftUI just handed to .introspect. Those are two independently-lived weak references: whenever the delegate object survives but SwiftUI installs its PagerView state onto a new collection view while the previous one is still alive, scrollDelegate.installedCollectionView still resolves to the old object and is !== collectionView… no — the failure is the mirror case: the delegate object is reused and installedCollectionView still points at a live collection view that happens to be the one being introspected again, so the block is skipped and scrollDelegate.delegate, orientation and originalDelegate are never set for the collection view that is actually on screen. In either ordering the predicate is keyed on the delegate's memory rather than on the collection view's current delegate, so nil cannot distinguish "installed into this view already" from "installed into a different view that is still alive" — the exact ambiguity the code comment objects to for originalDelegate == nil.
Concretely: the same @State delegate for a different collection view means no onPageScroll/onPageScrollStateChanged is ever emitted for that pager, and originalDelegate is never adopted so the internal SwiftUI page/index bookkeeping is bypassed. The predecessor's originalDelegate == nil gate did run in that case and did install. collectionView.delegate !== scrollDelegate is cycle-proof and does not have this ambiguity.
Also raised here
Also raised at this location by senior-developer: install is now keyed on delegate identity: a PagerScrollDelegate reused across collection views (or one whose displaced delegate died) never forwards to the live SwiftUI delegate again — The old gate (originalDelegate == nil) re-adopted collectionView.delegate on every layout pass once the weak reference died, which is what created the cycle. The new gate (scrollDelegate.installedCollectionView !== collectionView) skips the whole block for as long as installedCollectionView points at the object that was passed to introspect. That fixes the cycle but it also makes adoption one-shot per delegate object, keyed on a weak reference, and scrollDelegate is @State — i.e. it persists across .id(props.children.count) changes and across re-creation of PagerViewProvider/UIHostingController, while collectionView is whatever SwiftUI hands this still-live state object on the next .introspect call. Two reachable sequences where the new code loses forwarding that the predecessor kept: Displaced delegate dies, collection view survives. …
Also raised at this location by maintainability: Install-once gate no longer refreshes scrollDelegate.delegate for a reused collection view — scrollDelegate.delegate is a weak var, and it used to be re-assigned on every pass where originalDelegate had gone nil — which is exactly the state SwiftUI reaches after a page-count change rebuilds the TabView. Now the gate is installedCollectionView !== collectionView, so for a collection view that survives the rebuild the assignment scrollDelegate.delegate = delegate never runs again. If the PagerViewProviderDelegate instance handed to PagerView is not the same object across that rebuild (or is deallocated and recreated), scrollDelegate.delegate goes nil and onPageScroll/onPageSelected silently stop for that pager — a regression versus the old gate, which repaired it on the next layout pass. The install-once fix should cover only the collectionView.delegate adoption; keep refreshing the event delegate on every pass.
persona: senior-developer, maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
There was a problem hiding this comment.
@apex-sentinel-staging pivot Tested on the simulator. Each page count change makes a new PagingCollectionView. installedCollectionView still points at the old one, so !== is true and the install runs. I've added logs which show the install on every new collection view. Your suggesttion brings the cycle back when an SDK proxy is the delegate, because the delegate is then never scrollDelegate.
| // we re-adopted `collectionView.delegate`, which by then is this object | ||
| // (or a proxy forwarding back to it). Either way the delegate chain | ||
| // becomes a cycle and recurses until the stack overflows. | ||
| if scrollDelegate.installedCollectionView !== collectionView { |
There was a problem hiding this comment.
🟡 Minor · Install-once gate keys only on collection-view identity, so a delegate replaced after install is never re-installed (regression vs. the self-healing weak-nil gate)
The predecessor re-ran the install whenever scrollDelegate.originalDelegate == nil, which — including when the weak reference died on its own — meant the block kept re-adopting collectionView.delegate and so also repaired the installation after anything replaced the collection view's delegate behind our back: SwiftUI re-applying its own delegate on a re-render, or a swizzling SDK's proxy being torn down. The new gate scrollDelegate.installedCollectionView !== collectionView is false for the life of that collection view no matter what collectionView.delegate is now, so once something else takes the delegate the pager stops receiving scrollViewDidScroll/willDisplay/didEndDecelerating and never reinstalls: onPageScroll, onPageScrollStateChanged and onPageSelected stop firing for that pager while it is on screen, with no path back. installedCollectionView is weak, so even a dead-collection-view re-install relies on the weak read being nil; if the weak reference is still alive when SwiftUI hands back the same collection view, the install is skipped there too.
| if scrollDelegate.installedCollectionView !== collectionView { | |
| if scrollDelegate.installedCollectionView !== collectionView || collectionView.delegate !== scrollDelegate { | |
| if collectionView.delegate !== scrollDelegate { | |
| scrollDelegate.originalDelegate = collectionView.delegate | |
| } | |
| scrollDelegate.delegate = delegate | |
| // VTabView-style rotation preserves TabView's horizontal collection view. | |
| scrollDelegate.orientation = .horizontal | |
| scrollDelegate.installedCollectionView = collectionView | |
| collectionView.delegate = scrollDelegate | |
| } | |
Suggested change
Make the gate "we are not currently installed" rather than "we installed into this view once": e.g. install when scrollDelegate.installedCollectionView !== collectionView || collectionView.delegate !== scrollDelegate, while still refusing to adopt a delegate that is this same PagerScrollDelegate (or one whose chain re-enters us) so the cycle cannot form. That keeps the fix and restores the predecessor's self-healing.
Details
persona: swift-ios
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -54,11 +54,18 @@ struct PagerView: View { | |||
| collectionView.rightEdgeEffect.isHidden = true | |||
There was a problem hiding this comment.
🔵 Nit · @State holds the newly created PagerScrollDelegate — the install-once state dies with the view
PagerView stores the delegate object in @State private var scrollDelegate = PagerScrollDelegate(). Per SwiftUI semantics, a reference type put in @State is never updated: the value is snapshotted the first time the view is created, and every later PagerView value for the same view identity keeps the initial object; the fresh PagerScrollDelegate() built for each new struct is thrown away. This matters directly for this PR's premise: the PR description states that .id(props.children.count) makes SwiftUI rebuild the TabView "when the page count changes", i.e. the PagerView subtree and its @State are recreated. A new PagerScrollDelegate (with installedCollectionView == nil) is then created and discarded, while the body keeps operating on the OLD object — the one whose installedCollectionView still points at the old, now-dead collection view. On every layout pass the gate scrollDelegate.installedCollectionView !== collectionView is then true for the live collection view, so the install block re-runs every single pass and re-adopts collectionView.delegate, which is now scrollDelegate itself. That sets originalDelegate = self, which didSet immediately nils out, so no cycle forms, but the TabView's own delegate is displaced and never re-captured: the displaced delegate's scrollViewDidScroll/willDisplay/collectionView(_:willDisplay:...) stop being forwarded. Worse, the same nil-ing means the cycle can re-form whenever collectionView.delegate is a proxy that forwards back to us rather than self — the exact case the PR says it fixes. The originalDelegate == nil gate accidentally re-adopted here too, so this is not a new crash, but the new "install once" guarantee — the headline fix — silently does not hold across the very scenario the PR cites (page-count change), because the marker lives in state that does not survive the rebuild.
| collectionView.rightEdgeEffect.isHidden = true | |
| @StateObject private var scrollDelegateHolder = PagerScrollDelegateHolder() | |
Suggested change
Do not key the install-once marker on state stored in @State of a value type that SwiftUI recreates. Either hold the PagerScrollDelegate where the PagerView struct's identity is not the lifetime boundary (e.g. create it once in PagerViewProvider, which already owns the hosting controller, and pass it in), or key the marker on the collection view itself (an associated object / collectionView.delegate is PagerScrollDelegate check) so "already installed" is answered by the object that outlives the view rebuild.
Details
Also raised here
Also raised at this location by senior-developer: originalDelegate = nil inside its own didSet fires the observer again and silently discards the delegate it was handed — Two things about the new didSet. First, the self-assignment inside it re-triggers didSet for originalDelegate (Swift only suppresses the willSet-style loop, not observer re-entry for a second assignment); the recursion stops only because nil === self is false, so the property observer runs twice for that write — harmless today, but the termination condition is implicit and a future edit that adds another normalisation step here re-enters it. Second, the case is handled by dropping to nil, which also throws away any usable delegate that happened to be assigned, so forwarding stops for the rest of that collection view's life with no signal that the install was self-referential (the install site in PagerView.swift has no idea). A guard that skips the write, plus an assertion/log in debug builds, would name the condition instead of swallowing it.
Also raised at this location by swift-ios: didSet on originalDelegate hides the self-assignment instead of rejecting it, and never repairs an already-formed cycle — if originalDelegate === self { originalDelegate = nil } inside didSet is a correction of the caller's own write, done as a silent side effect of a property setter: at ios/PagerView.swift:64 the install site writes collectionView.delegate into originalDelegate and the write is quietly dropped, with no indication that the caller adopted itself. The recursive didSet re-entry terminates (nil !== self), so it is not a bug in itself, but the guard is only correct because the property is weak and therefore nilled exactly once — nothing in the type enforces that. …
persona: senior-developer, swift-ios
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| @@ -86,11 +98,25 @@ class PagerScrollDelegate: NSObject, UIScrollViewDelegate, UICollectionViewDeleg | |||
| } | |||
|
|
|||
| override func responds(to aSelector: Selector!) -> Bool { | |||
There was a problem hiding this comment.
🔵 Nit · responds(to:) answers false for a legitimately non-cyclic re-entrant query, silently disabling an optional delegate method
The guard is a single boolean, so it cannot tell a cycle from a legitimate nested query. Any chain where our responds(to:) is re-entered once while already querying — our own forwardingTarget(for:) path, an SDK proxy that queries back for a different selector, or a delegate that itself forwards responds(to:) — gets false for the inner query even though the object genuinely implements the selector. UIKit then treats an optional delegate/scroll-view method as unimplemented and skips it, so an event that should forward to the displaced delegate is dropped rather than the recursion being merely shortened. The flag is also reset by defer at the outermost frame only in the sense that the first frame resets it; an exception-free path is fine, but a nested query for selector A that returns false can be cached by UIKit's own conformance checks for the lifetime of that call.
Suggested change
Bound the recursion by depth and by selector instead of a flat boolean, so only the true cycle is cut: keep a per-selector in-progress set (or a small depth counter) and return false only when the same selector is being queried again on the same chain — the actual self-loop. Also consider clearing the flag from forwardingTarget(for:) so a forwarded message does not inherit a suppression meant for a different selector.
Details
Also raised here
Also raised at this location by senior-developer: Re-entrant responds(to:) returns false for selectors the original delegate does implement, silently dropping them — For any selector not in handledSelectors, a re-entrant query now returns false unconditionally — including the case where the chain is finite (e.g. self → P1 → P2 → self only through a getter, or a two-proxy chain that would have terminated). UIKit skips an optional delegate method that answers false, so on a swizzling-SDK app a delegate method the displaced delegate does implement is never forwarded, with no log and no way to tell that apart from the delegate genuinely not implementing it. The old expression had no such false-negative. This is the intended trade for not crashing, but it is a silent behaviour change on exactly the hosts the fix targets, and nothing in the repo exercises it.
persona: swift-ios, senior-developer
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| return originalDelegate?.responds(to: aSelector) ?? false | ||
| } | ||
|
|
||
| override func forwardingTarget(for aSelector: Selector!) -> Any? { |
There was a problem hiding this comment.
🔵 Nit · responds(to:) guard has no counterpart in forwardingTarget(for:), so the SDK-proxy cycle it exists for is only half blocked
responds(to:) is now hardened against a cycle formed by an SDK proxy that forwards back to this object, but forwardingTarget(for:) only rejects the direct case (target === self). In the same scenario the guard was added for — originalDelegate is a proxy whose forwardingTarget(for:) hands the message back to this object — this method returns that proxy again for any selector outside handledSelectors, which is the same cycle, one level down. It is only blocked because UIKit happens to ask responds(to:) before sending an optional delegate message (and because the runtime refuses to chain forwardingTargetForSelector: forever and falls through to forwardInvocation:, i.e. doesNotRecognizeSelector). Any sender that does not check responds(to:) first — a swizzled setter path, or a non-optional message on the forwarded-to interface — takes the forwarding path instead, and the re-entrancy flag is never consulted there because it only wraps the responds(to:) query. The comment's assumption that this is safe rests on UIKit call discipline and on runtime behaviour, not on anything the code enforces; the guard and the forwarding path should use the same cycle test.
| override func forwardingTarget(for aSelector: Selector!) -> Any? { | |
| override func forwardingTarget(for aSelector: Selector!) -> Any? { | |
| guard !handledSelectors.contains(aSelector) else { return nil } | |
| // Same cycle the `responds(to:)` guard above breaks: a proxy that forwards | |
| // back to us would be returned here again for every unhandled selector. | |
| guard !isQueryingOriginalDelegate else { return nil } | |
| let target = originalDelegate | |
| return target === self ? nil : target | |
| } |
Suggested change
Make the cycle check symmetric: reject a forwarding target whose chain re-enters this object (or track the proxy case explicitly), rather than relying on callers checking responds(to:) before sending. Add a test for the proxy-with-real-forwardingTarget case, not only the proxy-that-only-forwards-responds(to:) case.
Details
Also raised here
Also raised at this location by maintainability: responds(to:) gained a re-entrancy guard for the SDK-proxy case, but the matching override below only breaks a *dir… — responds(to:) gained a re-entrancy guard for the SDK-proxy case, but the matching override below only breaks a direct self-cycle (target === self). The scenario the new comment describes — a swizzling SDK's proxy P held in originalDelegate that forwards back to this object — is not caught by === self, so a message send for an unhandled selector still walks us → P → us → P through forwardingTarget(for:) until the stack overflows; the responds(to:) guard does not stop it, because the forwarding machinery does not consult it per hop. If the intent is defense-in-depth against proxies, the same guard (or a hop counter) is needed here too; otherwise the comment above responds(to:) overstates what is protected.
persona: senior-developer, maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
There was a problem hiding this comment.
@apex-sentinel-staging note Confirmed on the device, and the severity may be more than a nit. With proxy that forwards back to this object, the PR crashes on the first layout. The loop goes through the handled methods (collectionView(_:willDisplay:forItemAt:) calls originalDelegate directly), not through forwardingTarget(for:). The guard must cover every forward to originalDelegate.
There was a problem hiding this comment.
| @@ -86,11 +98,25 @@ class PagerScrollDelegate: NSObject, UIScrollViewDelegate, UICollectionViewDeleg | |||
| } | |||
|
|
|||
| override func responds(to aSelector: Selector!) -> Bool { | |||
There was a problem hiding this comment.
🔵 Nit · isQueryingOriginalDelegate is a plain Bool mutated around originalDelegate?.responds(to:), but responds(to:) …
isQueryingOriginalDelegate is a plain Bool mutated around originalDelegate?.responds(to:), but responds(to:) is Objective-C runtime introspection, not a delegate callback — it is invoked by respondsToSelector: from anywhere, including the SDKs named in the comment (which enumerate/set up on background queues) and KVO/performSelector: paths. The comment's justification, "Delegate callbacks are main-thread only," does not cover this method, so two concurrent respondsToSelector: queries (main thread inside the query, a second thread starting one) make the second one take the guard and answer false for a selector the displaced delegate does implement — a legitimate optional UICollectionViewDelegate/UIScrollViewDelegate method is then skipped on that send, and the unsynchronised read/write is a data race. Either assert/require the main thread here (Thread.isMainThread, falling back to answering the query directly off-main) or state the main-thread precondition on the property.
| override func responds(to aSelector: Selector!) -> Bool { | |
| // An analytics SDK may swizzle the collection view's delegate setter and | |
| // insert a proxy that forwards `responds(to:)` back to whatever it | |
| // replaced - this object. Querying `originalDelegate` then re-enters this | |
| // method through that proxy and overflows the stack. Report `false` for a | |
| // re-entrant query so the loop terminates; UIKit skips optional delegate | |
| // methods that answer `false`. The flag is only safe because delegate | |
| // queries arrive on the main thread. | |
| guard Thread.isMainThread, !isQueryingOriginalDelegate else { return originalDelegate?.responds(to: aSelector) ?? false } |
Details
persona: maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| } | ||
|
|
||
| if scrollDelegate.originalDelegate == nil { | ||
| // Install once per collection view. This closure re-runs on every |
There was a problem hiding this comment.
🔵 Nit · Stale/contradictory comment: "VTabView-style rotation" forces .horizontal inside the install block
The install block sets scrollDelegate.orientation = .horizontal under the comment "VTabView-style rotation preserves TabView's horizontal collection view", but 8 lines above the same block honours orientation == "vertical" with a vertical rotation (and hides the right edge effect). On that vertical path the comment is false and the line overwrites the orientation the delegate needs, so onPageScroll/onPageSelected would report the wrong axis for a vertical pager whose install actually happens on that pass. Move the assignment out of the install-once gate (it is not per-collection-view state) or, if forcing horizontal really is right for every pass, drop the now-contradictory comment and make the unconditional assignment explicit.
| // Install once per collection view. This closure re-runs on every | |
| if scrollDelegate.installedCollectionView !== collectionView { | |
| scrollDelegate.originalDelegate = collectionView.delegate | |
| scrollDelegate.delegate = delegate | |
| scrollDelegate.installedCollectionView = collectionView | |
| collectionView.delegate = scrollDelegate | |
| } | |
| // VTabView-style rotation preserves TabView's horizontal collection view. | |
| scrollDelegate.orientation = .horizontal |
Details
persona: maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
jacekAtCK
left a comment
jacekAtCK
left a comment
There was a problem hiding this comment.
Thanks for the detailed write-up. I tested this PR on the iPhone 15 simulator ios 18.0.
Without an SDK, I could not reproduce the crash in the app. I did something like 10 rounds of add and remove page, and swipe on base and i did not observe any crashes.
Each page-count change makes a new PagingCollectionView with a new SwiftUI Coordinator, and the old collection view is not laid out again after its coordinator is freed. So the self-cycle does not form.
Can you share your crash reproduction steps?
With an SDK-style proxy, the PR still crashes.As a test i've added -[UICollectionView setDelegate:]. It keeps one proxy for each collection view and points it at each new delegate.
I'm very rusty in swift so I've let an agent figure out a possible fix (untested):
guard re-entry around every forward to originalDelegate, not only in responds(to:).
Videos of both crashes are attached.
https://github.com/user-attachments/assets/e3233291-5c44-44a4-a07f-4c7c5663307f
https://github.com/user-attachments/assets/5cc25e9d-ebcf-4c76-94b0-aad9030ec065
jacekAtCK
left a comment
jacekAtCK
left a comment
There was a problem hiding this comment.
after thorough testing it seems that:
- this is a partial fix
- no regressions introduced
Fixes #1146.
The bug
PagerScrollDelegateproxies the TabView collection view's delegate: it keeps the delegate it displaced inoriginalDelegateand forwards unhandled selectors to it. The install block was gated onoriginalDelegate == nil— but that reference isweak, so it cannot distinguish "not installed yet" from "installed, but the weak reference died"..introspectre-runs on every layout pass, so once the displaced delegate deallocated, the block ran again and re-adoptedcollectionView.delegate— which by then is thePagerScrollDelegateitself.responds(to:)then calls itself until the stack overflows (EXC_BAD_ACCESS, code=2, guard page).originalDelegatedying is reachable from ordinary use: SwiftUI rebuilds theTabViewwhen.id(props.children.count)changes, i.e. whenever the page count changes. We hit it via@react-navigation/material-top-tabson screens that add or drop a tab once data loads.The fix
installedCollectionView) rather than the weak reference. A new collection view is still installed into — so a page-count change keeps working — but the same one is never re-adopted, so the cycle cannot form. This is the actual fix.responds(to:), as defense-in-depth. An analytics SDK that swizzles the delegate setter (Pendo, in our case) inserts a proxy that forwards back here, so the chain can be cyclic even when the delegate is not literallyself. Handled selectors returntruebefore the guard, so the eight selectors driving the pager's events are untouched.originalDelegateindidSet.Verification
I compiled
PagerScrollDelegate.swiftfrom this branch against the iOS SDK and ran it against a proxy reproducing an SDK's swizzling —responds(to:)forwarding plus a realforwardingTarget— through the full sequence: install → proxy wraps us → displaced delegate deallocates → closure re-runs → scroll arrives → collection view replaced.responds(to:)on a cyclic chainEXC_BAD_ACCESS(code=2)EXC_BAD_ACCESS(code=2)onPageScrollfires once per scrollEXC_BAD_ACCESS(code=2)The third row is worth calling out: the crash reproduces with no analytics SDK involved at all. A swizzling SDK only widens the window and adds the event-forwarding path.
Notes
🤖 Generated with Claude Code