callstack / callstack/react-native-pager-view
iOS: PagerScrollDelegate proxies to itself and overflows the stack when the weak originalDelegate dies
- Vorherrschende Sprache
- TypeScript
- Sterne
- 3.4k
- Forks
- 476
- Ø Merge
- 10 T. 21 Std.
- Gemergte PRs (30 T.)
- 2
Beschreibung
### Description
On iOS, `PagerScrollDelegate` can end up proxying to itself, after which `responds(to:)` calls itself until the stack overflows. The app dies with `EXC_BAD_ACCESS (code=2)` — a guard-page hit — and the crash is not catchable.
### Root cause
`PagerScrollDelegate` keeps the delegate it displaced in `originalDelegate` and forwards unhandled selectors to it. The install block in `PagerView.swift` is gated on:
```swift
if scrollDelegate.originalDelegate == nil {
scrollDelegate.originalDelegate = collectionView.delegate
...
collectionView.delegate = scrollDelegate
}
```
`originalDelegate` is `weak`, so `== nil` cannot distinguish **"not installed yet"** from **"installed, but the weak reference died"**. The `.introspect` closure re-runs on every layout pass, so once the displaced delegate deallocates, the block runs again and re-adopts `collectionView.delegate` — which by then is the `PagerScrollDelegate` itself.
`responds(to:)` then calls itself:
```swift
override func responds(to aSelector: Selector!) -> Bool {
handledSelectors.contains(aSelector) || (originalDelegate?.responds(to: aSelector) ?? false)
}
```
### Reproduction
Minimal, no third-party SDK involved — this is the install block's own logic:
```swift
let cv = UICollectionView(frame: .init(x: 0, y: 0, width: 400, height: 800),
collectionViewLayout: UICollectionViewFlowLayout())
let sd = PagerScrollDelegate()
var upstream: Upstream? = Upstream()
cv.delegate = upstream
if sd.originalDelegate == nil { sd.originalDelegate = cv.delegate; cv.delegate = sd }
upstream = nil // the weak reference dies
if sd.originalDelegate == nil { sd.originalDelegate = cv.delegate; cv.delegate = sd }
print(sd.originalDelegate === sd) // true
_ = sd.responds(to: #selector(UIScrollViewDelegate.scrollViewDidZoom(_:))) // SIGSEGV
```
In the app, `originalDelegate` dying is reachable from ordinary use: SwiftUI rebuilds the `TabView` whenever `.id(props.children.count)` changes, i.e. any time the number of pages changes. We hit it through `@react-navigation/material-top-tabs`, on screens that add or remove a tab once data loads or an edit mode is toggled.
An analytics SDK that swizzles the collection view's delegate setter (in our case Pendo) makes it considerably worse: its proxy forwards back to the `PagerScrollDelegate`, so the cycle forms even when `collectionView.delegate` is not literally `self`, and the loop then also runs through **event forwarding**, not just `responds(to:)` — `scrollViewDidScroll` → proxy → `scrollViewDidScroll` → … So guarding `responds(to:)` alone is not sufficient.
### Suggested fix
Key the install on the collection view's identity rather than on the weak reference, so a new collection view is still installed into but the same one is never re-adopted. A re-entrancy guard in `responds(to:)` is worth having as defense-in-depth for the swizzled-proxy case.
I have this working and will open a PR.
### Version
`master` (9.0.4). Also present in 8.0.0, which is where we hit it.
### Platform
iOS only.
Beitragsleitfaden
Rechercherichtung
Beginne in PagerView.swift beim Installationsblock von .introspect und untersuche dann originalDelegate, responds(to:) und die Weiterleitung von Ereignissen in PagerScrollDelegate. Reproduziere den im Issue gezeigten Lebenszyklus eines schwachen Delegaten, einschließlich eines erneut ausgeführten Introspektionsdurchlaufs. Als erledigt gilt die Aufgabe, wenn dieselbe Collection View nicht erneut übernommen wird und die Weiterleitung von Delegaten nicht durch sich selbst oder einen Proxy rekursiv werden kann.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- react-native, swift
- Bereich
- mobile
- Issue-Typ
- Bug
- Schwierigkeit
- 4/5
- Geschätzter Aufwand
- 3-5 Tage
- Aktivitätsstatus
- Aktiv
- Klarheit
- Klar beschrieben
- Anfängerfreundlichkeit
- 25/100