callstack / callstack/react-native-pager-view
iOS: PagerScrollDelegate proxies to itself and overflows the stack when the weak originalDelegate dies
- Lingua principale
- TypeScript
- Stelle
- 3.4k
- Fork
- 476
- Merge medio
- 10g 21h
- PR unite (30g)
- 2
Descrizione
### 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.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia in PagerView.swift, nel blocco di installazione di .introspect, quindi esamina originalDelegate, responds(to:) e l’inoltro degli eventi di PagerScrollDelegate. Riproduci il ciclo di vita del delegato debole mostrato nell’issue, inclusa una nuova esecuzione del passaggio di introspezione. Il lavoro è completato quando la stessa vista di raccolta non viene adottata nuovamente e l’inoltro del delegato non può ricorrere attraverso sé stesso o un proxy.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- react-native, swift
- Ambito
- mobile
- Tipo di issue
- Bug
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Attiva
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 25/100