[Android] Fix handlers cancelled while awaiting leaking in the orchestrator - #4402
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Android gesture orchestrator now clears ChangesAndroid awaiting handler cleanup
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes an Android-side leak in GestureHandlerOrchestrator where a handler that gets cancelled/failed while isAwaiting could remain recorded indefinitely, eventually preventing RNGH-based touchables/gestures from beginning until app restart (as described in #4401).
Changes:
- Clear
handler.isAwaitingwhen the handler transitions toSTATE_CANCELLEDorSTATE_FAILEDinsideonHandlerStateChange. - Allow the existing
cleanupFinishedHandlers()logic to reset/remove these terminal-state handlers now that they’re no longer excluded by the!handler.isAwaitingguard.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…867) Bumps \[react-native-gesture-handler\](https://github.com/software-mansion/react-native-gesture-handler) from 2.32.0 to 3.2.1. Release notes _Sourced from [react-native-gesture-handler's releases](https://github.com/software-mansion/react-native-gesture-handler/releases)._ > v3.2.1 > ------ > > 🐛 Bug fixes > ------------ > > * Forward press handlers as `testOnly_*` in `PressableWithTouchable` by [`@huextrat`](https://github.com/huextrat) in [software-mansion/react-native-gesture-handler#4416](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4416) > > 🔢 Miscellaneous > ---------------- > > * Update `Pressable` props by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4421](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4421) > > **Full Changelog**: [https://github.com/software-mansion/react-native-gesture-handler/compare/v3.2.0...v3.2.1](https://github.com/software-mansion/react-native-gesture-handler/compare/v3.2.0...v3.2.1) > > v3.2.0 > ------ > > ❗ Important changes > ------------------- > > * feat: Adopt AGP v9 by [`@hurali97`](https://github.com/hurali97) in [software-mansion/react-native-gesture-handler#4263](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4263) > * Implement `Pressable` based on `Touchable` by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4411](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4411) > * \[Android\] Add hover callbacks to Touchable by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4396](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4396) > * \[iOS\] Add hover callbacks to Touchable by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4397](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4397) > * \[Web\] Add hover callbacks to Touchable by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4398](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4398) > * \[Web\] Refactor `Touchable` not to rely on `GestureDetector` by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4344](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4344) > * \[iOS\] Refactor `Touchable` not to rely on `GestureDetector` by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4343](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4343) > * \[Android\] Refactor `Touchable` not to rely on `GestureDetector` by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4342](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4342) > * Fix fatal crash `Cannot read property 'translationX' of undefined` when a touch event is serialized without `allTouches` by [`@huextrat`](https://github.com/huextrat) in [software-mansion/react-native-gesture-handler#4316](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4316) > > 👍 Improvements > --------------- > > * \[Android\] Skip the underlay drawable when it can never be visible by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4359](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4359) > * \[Android\] Apply the button's managed handler config once per prop transaction by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4357](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4357) > * \[Android\] Configure the button's handler directly instead of through a `ReadableMap` by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4358](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4358) > > 🐛 Bug fixes > ------------ > > * \[Android\] Guard update events to only be dispatched in ACTIVE state by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4332](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4332) > * Pass empty callbacks to UI when `runOnJS` is `true` by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4326](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4326) > * \[Web\] Fix incorrectly calculated `timeDelta` by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4329](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4329) > * \[Web\] Fix incorrect `Tap` offset by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4330](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4330) > * Fix `minVelocity` props behavior by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4327](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4327) > * \[Android\] Fix `minDistance` being reset by partial config updates by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4347](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4347) > * Move Interceptor on `ScrollView`, not its content by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4331](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4331) > * \[iOS\] Re-sync layer opacity and transform from retained props when recycling buttons by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4360](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4360) > * \[Android\] Properly handle `requestDisallowInterceptTouchEvent` for v3 by [`@j-piasecki`](https://github.com/j-piasecki) in [software-mansion/react-native-gesture-handler#4367](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4367) > * Fix `Touchable` not respecting `keyboardShouldPersistTaps="handled"` by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4372](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4372) > * \[macOS\] Fix touch events never being delivered by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4390](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4390) > * fix: crash when mount listener fires after GestureDetector unmount by [`@kosmydel`](https://github.com/kosmydel) in [software-mansion/react-native-gesture-handler#4268](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4268) > * \[macOS\] Fix `Pan` activation criteria being ignored by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4387](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4387) > * \[macOS\] Fix `manualActivation` never blocking gesture activation by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4389](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4389) > * \[iOS\] Fix touch events never being delivered to `VirtualDetector` handlers by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4392](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4392) > * \[iOS\] Fix gestures attached via `VirtualGestureDetector` never recognizing continuous gestures by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4393](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4393) > * \[macOS\] Fix `Fling` not sending touch events and begin/end states consistently by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4395](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4395) > * \[Android\] Fix handlers cancelled while awaiting leaking in the orchestrator by [`@m-bert`](https://github.com/m-bert) in [software-mansion/react-native-gesture-handler#4402](https://redirect.github.com/software-mansion/react-native-gesture-handler/pull/4402) ... (truncated) Commits * [`62f0f7d`](software-mansion/react-native-gesture-handler@62f0f7d) Release v3.2.1 * [`4716425`](software-mansion/react-native-gesture-handler@4716425) Merge branch '3.2-stable' of github.com:software-mansion/react-native-gesture... * [`f0ae48c`](software-mansion/react-native-gesture-handler@f0ae48c) Update `Pressable` props ([#4421](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4421)) * [`5f0f0d8`](software-mansion/react-native-gesture-handler@5f0f0d8) Forward press handlers as `testOnly_*` in `PressableWithTouchable` ([#4416](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4416)) * [`0a91db7`](software-mansion/react-native-gesture-handler@0a91db7) Release v3.2.0 * [`44046a6`](software-mansion/react-native-gesture-handler@44046a6) \[Android\] Resolve the button event dispatcher by react tag ([#4415](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4415)) * [`2469c1d`](software-mansion/react-native-gesture-handler@2469c1d) Derive `Pressable` pressed state from `testOnly_pressed` ([#4414](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4414)) * [`3f1bf74`](software-mansion/react-native-gesture-handler@3f1bf74) Clear pending timers on unmount in StatefulPressable ([#4413](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4413)) * [`50ae6a1`](software-mansion/react-native-gesture-handler@50ae6a1) \[General\] Default GestureDetector moduleId to -1 ([#4412](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4412)) * [`8b661c9`](software-mansion/react-native-gesture-handler@8b661c9) Implement `Pressable` based on `Touchable` ([#4411](https://redirect.github.com/software-mansion/react-native-gesture-handler/issues/4411)) * Additional commits viewable in [compare view](software-mansion/react-native-gesture-handler@v2.32.0...v3.2.1)
Description
On Android, cancelling a handler while it is awaiting another one (e.g. the single tap in
Exclusive(doubleTap, singleTap)waiting for the double tap to fail) leaves it in the orchestrator forever. Both cleanup paths incleanupFinishedHandlersskip handlers withisAwaitingset, and the rescue loop inonHandlerStateChangenever reaches it becausedropGestureHandlerdrops interaction relations on the JS thread before the posted cancel runs on the UI thread, soshouldHandlerWaitForOtherno longer matches.The leaked handler stays in
gestureHandlers, which makesButtonViewGroup.shouldBeginWithRecordedHandlersreturnfalseon every subsequent touch. As a result all button-based touchables (Pressable,RectButton,BaseButton,Touchables) stop responding app-wide until the app process is restarted. The most common trigger is unmounting aGestureDetectorduring the wait window.This change clears
isAwaitingwhen a handler reachesSTATE_CANCELLEDorSTATE_FAILED, since such a handler can never be resolved by the one it was waiting for, letting the existing cleanup collect it.STATE_ENDstays pinned, asmakeActiverelies on it to send synthetic events. Going throughonHandlerStateChangealso covers cancel paths that never touch the registry, e.g.tryActivatecancelling an awaiting handler viashouldBeCancelledByFinishedHandler.Fixes #4401
Test plan
Tested on the following code