Repository navigation
Conversation
…Area additionalInsets(toReach:) assumes UIKit applies the value written to additionalSafeAreaInsets. A page re-hosted by an iOS 18+ zoom transition can keep a fixed safeAreaInsets instead, so every layout pass subtracts the same gap again, the > 0.5 change check never settles and the main thread loops forever until the watchdog kills the app (0x8BADF00D). Ignore our own nested viewSafeAreaInsetsDidChange callback and write again only when the target or the page's actual safe area moved since the last write. In the normal case behavior is unchanged; in the broken state the loop writes once and stops. Fixes #1232
Apex SentinelVerdict: Since the last review: 0 new · 0 still open · 1 no longer raised No findings. 6 findings below Reviewed with this repository's own Sentinel configuration ( Feedback goes in the review comment threads, as a reply under the inline comment of a finding: Reply Useful? Reply Comment Earlier reviewsReview of
|
| Where | Finding |
|---|---|
ios/Extensions.swift:91 |
iOS 18 zoom-transition safe-area loop (#1232) fix ships without a repro or regression test |
3 findings below minor not shown (user_settings.min_severity).
Reviewed with this repository's own Sentinel configuration (d7d1c3b47dd7).
![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: 1 new finding on da70d53. The verdict and the full list are in the summary comment.
| /// | ||
| /// `additionalInsets(toReach:)` assumes UIKit applies the value written | ||
| /// here. A page re-hosted by an iOS 18+ zoom transition can keep a fixed | ||
| /// `safeAreaInsets` instead, so every pass subtracts the same gap again and | ||
| /// the main thread loops forever (#1232). Ignore our own nested callback | ||
| /// and write again only when the target or the page's actual safe area | ||
| /// moved since the last write. | ||
| private func propagateSafeArea() { | ||
| let insets = additionalInsets(toReach: targetSafeAreaInsets()) | ||
| if abs(additionalSafeAreaInsets.top - insets.top) > 0.5 | ||
| || abs(additionalSafeAreaInsets.left - insets.left) > 0.5 | ||
| || abs(additionalSafeAreaInsets.bottom - insets.bottom) > 0.5 | ||
| || abs(additionalSafeAreaInsets.right - insets.right) > 0.5 { | ||
| additionalSafeAreaInsets = insets | ||
| guard !isPropagatingSafeArea else { | ||
| return | ||
| } | ||
| let target = targetSafeAreaInsets() | ||
| let insets = additionalInsets(toReach: target) | ||
| if Self.insetsMatch(additionalSafeAreaInsets, insets) { | ||
| return | ||
| } | ||
| if let lastWrite = lastSafeAreaWrite, | ||
| Self.insetsMatch(lastWrite.target, target), | ||
| Self.insetsMatch(lastWrite.safeArea, view.safeAreaInsets) { | ||
| return | ||
| } | ||
| isPropagatingSafeArea = true | ||
| additionalSafeAreaInsets = insets | ||
| isPropagatingSafeArea = false | ||
| lastSafeAreaWrite = (target: target, safeArea: view.safeAreaInsets) | ||
| } | ||
|
|
||
| private static func insetsMatch(_ lhs: UIEdgeInsets, _ rhs: UIEdgeInsets) -> Bool { | ||
| abs(lhs.top - rhs.top) <= 0.5 | ||
| && abs(lhs.left - rhs.left) <= 0.5 | ||
| && abs(lhs.bottom - rhs.bottom) <= 0.5 | ||
| && abs(lhs.right - rhs.right) <= 0.5 | ||
| } |
There was a problem hiding this comment.
No longer raised on c5f772f.
🟡 Minor · iOS 18 zoom-transition safe-area loop (#1232) fix ships without a repro or regression test
No example screen in example/src/gh-issues or e2e case in e2e/tests/issues/ covers this fix. Neighbouring commits 457a73c and its revert d7d1c3b show this code regressing silently, and the hung-main-thread failure mode would go undetected by the existing issue-1142-search-bar-inset.e2e.ts.
Suggested change
Add an example repro screen for #1232 plus an e2e regression test that drives the iOS 18+ zoom-transition re-hosting and asserts the main thread stays responsive / the page's measured insets stay stable.
Details
persona: senior-developer
There was a problem hiding this comment.
🟡 Changes recommended
The guard can suppress further corrections after a negative inset clamps the effective safe area to zero, preventing legitimate convergence to the target.
1 open issue
What changed in this PR
This PR addresses the iOS main-thread hang reported in #1232 by preventing repeated safe-area corrections when UIKit stops applying them after a zoom transition.
Changes:
- Adds a re-entrancy guard to ignore callbacks triggered by safe-area writes.
- Records the last target and actual safe-area insets to suppress unchanged retries.
- Centralizes inset comparison and documents the failure scenario.
| File | Description |
|---|---|
ios/Extensions.swift |
Adds guards and inset tracking to prevent repeated safe-area writes in PageChildViewController. |
Review effort: Balanced
|
@apex-sentinel-staging review |
There was a problem hiding this comment.
🟡 Changes recommended
A critical inset-rebasing bug in ios/Extensions.swift repeatedly changes already-correct edges, bypassing the unchanged-sample guard and causing correction drift on every layout pass.
1 open issue
1 resolved since last review
Review effort: Balanced
| /// `additionalSafeAreaInsets` with every clamped edge (negative correction, | ||
| /// 0 actual inset) set to 0, or nil when no edge is clamped. | ||
| private func additionalInsetsWithoutClampedEdges() -> UIEdgeInsets? { | ||
| func unclamped(_ additional: CGFloat, _ actual: CGFloat) -> CGFloat { | ||
| additional < -0.5 && actual <= 0.5 ? 0 : additional | ||
| } | ||
| let current = additionalSafeAreaInsets | ||
| let actual = view.safeAreaInsets | ||
| let rebased = UIEdgeInsets( | ||
| top: unclamped(current.top, actual.top), | ||
| left: unclamped(current.left, actual.left), | ||
| bottom: unclamped(current.bottom, actual.bottom), | ||
| right: unclamped(current.right, actual.right) | ||
| ) | ||
| return rebased == current ? nil : rebased | ||
| } |
🟢 e2e: 2 flaky, 38 passed
|
| Test | Time | |
|---|---|---|
| tests/issues/issue-1154-page-change-focus.e2e.ts · 2 flaky, 11 passed | 4m 17s | |
| 🟢 | leaving a page with setPage blurs its input | 18.4s |
| 🟢 | setPage with an input focused lands on the target page | 15.9s |
| 🟢 | a setPage two pages away lands on the far page | 15.2s |
| 🟢 | a setPage repeated during the slide changes nothing | 16.3s |
| 🟢 | leaving a page with a swipe blurs its input | 15.6s |
| 🟢 | an input focused on the new page keeps focus | 24.0s |
| 🟢 | a swipe keeps the focus the new page takes from onPageSelected | 21.2s |
| 🟢 | a setPage that interrupts another lands on the page asked for last | 22.3s |
| a setPage that interrupts another still lands on the later page (1 failed attempt first) | 22.9s | |
| a setPage redirected once the focus has landed blurs it (1 failed attempt first) | 18.5s | |
| 🟢 | two setPage calls that cancel out land on the first page | 16.4s |
| 🟢 | right to left › leaving a page with a swipe blurs its input | 33.9s |
| 🟢 | a page that grows with the keyboard still lands on the target page | 16.3s |
| 🟢 | tests/basic-pager.e2e.ts · 3 passed | 2m 10s |
| 🟢 | basic pager › in LTR › pages horizontally | 51.2s |
| 🟢 | basic pager › in LTR › pages vertically | 28.0s |
| 🟢 | basic pager › in RTL › pages horizontally with the reversed gesture | 50.7s |
| 🟢 | tests/issues/issue-1050-back-gesture.e2e.ts · 8 passed | 2m 23s |
| 🟢 | back gesture over the pager › in LTR › a back swipe on the first page pops the screen | 15.1s |
| 🟢 | back gesture over the pager › in LTR › a back swipe on a later page goes to the previous page | 13.7s |
| 🟢 | back gesture over the pager › in LTR › a forward swipe on the first page still goes to the next page | 10.3s |
| 🟢 | back gesture over the pager › in RTL › a back swipe on the first page pops the screen | 27.3s |
| 🟢 | back gesture over the pager › in RTL › a back swipe on a later page goes to the previous page | 30.0s |
| 🟢 | back gesture over the pager › in RTL › a forward swipe on the first page still goes to the next page | 25.6s |
| 🟢 | back gesture over a pager in a nested stack › a back swipe on the first page pops the outer stack | 11.7s |
| 🟢 | back gesture over a pager in a nested stack › a forward swipe on the first page still goes to the next page | 9.1s |
| 🟢 | tests/issues/issue-1083-modal-set-page.e2e.ts · 1 passed | 17.9s |
| 🟢 | setPage called behind a native-stack modal lands on the requested page | 17.9s |
| 🟢 | tests/issues/issue-1096-keyboard-shrink.e2e.ts · 1 passed | 33.9s |
| 🟢 | keyboard avoidance does not shrink the page under the keyboard | 33.9s |
| 🟢 | tests/issues/issue-1098-nested-pager.e2e.ts · 1 passed | 17.5s |
| 🟢 | outer pager stays responsive after its page count changes around a nested pager | 17.5s |
| 🟢 | tests/issues/issue-1142-search-bar-inset.e2e.ts · 1 passed | 16.1s |
| 🟢 | FlatList inside the pager gets the search bar safe-area insets | 16.1s |
| 🟢 | tests/issues/pr-1194-remount-initial-page.e2e.ts · 1 passed | 19.0s |
| 🟢 | a pager remounted after its screen was covered starts on its own initialPage | 19.0s |
| 🟢 | tests/keyboard-dismiss-mode.e2e.ts · 3 passed | 40.1s |
| 🟢 | on-drag: dragging the pager blurs the input | 14.6s |
| 🟢 | on-drag: setPage keeps the input focused | 13.3s |
| 🟢 | none: dragging the pager keeps the input focused | 12.3s |
| 🟢 | tests/material-top-bar.e2e.ts · 1 passed | 20.8s |
| 🟢 | material top tabs survive a native-stack push and pop | 20.8s |
| 🟢 | tests/native-driver-events.e2e.ts · 2 passed | 38.3s |
| 🟢 | native driver events › setPage through the Animated ref drives the native value | 22.0s |
| 🟢 | native driver events › a swipe drives the native value | 16.3s |
| 🟢 | tests/nested-pager-view.e2e.ts · 1 passed | 20.0s |
| 🟢 | nested horizontal and vertical pagers scroll independently | 20.0s |
| 🟢 | tests/on-page-selected.e2e.ts · 1 passed | 18.0s |
| 🟢 | onPageSelected fires an alert for every page change | 18.0s |
| 🟢 | tests/scrollable-pager-view.e2e.ts · 1 passed | 24.6s |
| 🟢 | pager inside a ScrollView keeps its page across vertical scrolls | 24.6s |
| 🟢 | tests/smoke.e2e.ts · 1 passed | 10.6s |
| 🟢 | horizontal pager pages with the panel buttons | 10.6s |
| 🟢 | tests/tab-view-inside-scroll-view.e2e.ts · 1 passed | 18.5s |
| 🟢 | TabView inside a ScrollView swipes tabs and scrolls the page | 18.5s |
e2e 0.21.3 · 14m 52s · ios · run artifacts
Summary
Fixes the main-thread hang reported in #1232:
PageChildViewController.propagateSafeArea()can loop forever on iOS, pinning a core at 100% until the watchdog kills the app (0x8BADF00D).Root cause
additionalInsets(toReach:)derives the inset this controller already inherits as:which assumes UIKit applied the previous write to
additionalSafeAreaInsets. When a page is re-hosted by an iOS 18+ zoom transition (e.g. expo-routerLink.AppleZoom), UIKit can stop applyingadditionalSafeAreaInsetsfor that page:view.safeAreaInsetsstays fixed at 62 while the written value drifts by the same ~-36.9pt every pass.Because of that, the
> 0.5"did it change?" check is always true, so each write re-entersviewSafeAreaInsetsDidChange->propagateSafeArea-> another write, and the next layout pass starts it over again:Fix
propagateSafeAreanow:isPropagatingSafeAreare-entrancy guard.view.safeAreaInsetsmust differ from the ones recorded at the last write (lastSafeAreaWrite).In the normal case the first write lands and later passes have nothing to change, so behaviour is unchanged. In the broken state it writes once and stops.
Test plan
Credit to @jakzaizzat for the diagnosis and the patch in #1232.