callstack / callstack/react-native-pager-view

iOS: PagerScrollDelegate proxies to itself and overflows the stack when the weak originalDelegate dies

Open
#1,146 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
3.4k
Forks
476
Avg merge
10d 21h
Merged PRs (30d)
2

Description

### 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.