react-native-pager-view: [iOS][5.4.2...5.4.6] Crash when rapidly switching pages

Environment

System:
    OS: macOS 11.5
    CPU: (6) x64 Intel(R) Core(TM) i5-8500B CPU @ 3.00GHz
    Memory: 655.26 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 12.16.2 - ~/.nvm/versions/node/v12.16.2/bin/node
    Yarn: Not Found
    npm: 6.14.4 - ~/.nvm/versions/node/v12.16.2/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.10.1 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 14.5, DriverKit 20.4, macOS 11.3, tvOS 14.5, watchOS 7.4
    Android SDK:
      API Levels: 29, 31
      Build Tools: 29.0.3, 31.0.0
      System Images: android-29 | Google APIs Intel x86 Atom_64, android-29 | Google Play Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 2020.3 AI-203.7717.56.2031.7678000
    Xcode: 12.5/12E262 - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_292 - /usr/bin/javac
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1 => 16.13.1
    react-native: 0.63.4 => 0.63.4
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Description

Application crashes when rapidly switching pages, with the following error: Unexpected view controller: <UIViewController: 0x7f983a539850>.

As far as I can tell it dates back to version 5.4.2. Prior to that, this did not happen. It’s surprisingly easy to trigger on a real device.

https://user-images.githubusercontent.com/2244710/135704139-92179925-dcc2-4918-b0a7-f4f7ba8402b8.mov

Reproducible Demo

  • Open the example app.
  • Remove the Alert.alert() call on OnPageSelectedExample.tsx so you don’t get artificial pauses.
  • Run, and rapidly switch pages in the corresponding example.
  • 💥

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 8
  • Comments: 54 (9 by maintainers)

Commits related to this issue

Most upvoted comments

From what I’ve been reading, it’s a bug in UIPageViewController that’s been there since before iOS 8.

In scroll mode, it keeps an internal cache of the pages it is transitioning between. If you switch pages while it’s already animating to another, you can end up selecting a page that is not in its cache, leading to the “Unexpected view controller” crash.

In curl mode it doesn’t do any precaching, but the animation is ugly.

From what I’ve gathered some people:

  • Avoid using UIPageViewController altogether, instead opting for UICollectionView (and rolling their own animations?);
  • Implement workarounds to force it to clear its internal cache (e.g. setting dataSet = nil and then back to an acceptable state just before transitioning to another view controller);
  • Transition to other pages asynchronously.

I’m no iOS dev, but I tried to implement every workaround I could find to no avail. I worked around it by:

  1. only allowing one animation to run at any point in time.
  2. only animating the last page transition. So if you’re jumping from page 1 to page 5, it will instantly jump to page 4 and then animate to page 5. In my app, that isn’t terribly obvious, but your mileage may vary.

For those interested, here’s the patch I am using with patch-package and react-native-pager-view@5.4.6:


diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.m b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
index 78f266b..c8abb4f 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.m
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
@@ -20,6 +20,7 @@ @interface ReactNativePageView () <UIPageViewControllerDataSource, UIPageViewCon
 
 @property(nonatomic, strong) NSHashTable<UIViewController *> *cachedControllers;
 @property (nonatomic, assign) CGPoint lastContentOffset;
+@property(nonatomic, assign) BOOL animating;
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated;
 - (void)shouldScroll:(BOOL)scrollEnabled;
@@ -175,6 +176,10 @@ - (void)setReactViewControllers:(NSInteger)index
     __weak ReactNativePageView *weakSelf = self;
     uint16_t coalescingKey = _coalescingKey++;
 
+    if (animated == YES) {
+        self.animating = YES;
+    }
+    
     [self.reactPageViewController setViewControllers:@[controller]
                                            direction:direction
                                             animated:animated
@@ -183,6 +188,10 @@ - (void)setReactViewControllers:(NSInteger)index
         strongSelf.currentIndex = index;
         strongSelf.currentView = controller.view;
 
+        if (finished) {
+            strongSelf.animating = NO;
+        }
+        
         if (strongSelf.eventDispatcher) {
             if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
                 if (shouldCallOnPageSelected) {
@@ -240,25 +249,19 @@ - (void)goTo:(NSInteger)index animated:(BOOL)animated {
     long diff = labs(index - _currentIndex);
     
     if (isForward && diff > 0) {
-        for (NSInteger i=_currentIndex; i<=index; i++) {
-            if (i == _currentIndex) {
-                continue;
-            }
-            [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
+        for (NSInteger i=_currentIndex+1; i<=index; i++) {
+            [self goToViewController:i direction:direction animated:(!self.animating && i == index) shouldCallOnPageSelected: i == index];
         }
     }
     
     if (!isForward && diff > 0) {
-        for (NSInteger i=_currentIndex; i>=index; i--) {
-            if (index == _currentIndex) {
-                continue;
-            }
-            [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
+        for (NSInteger i=_currentIndex-1; i>=index; i--) {
+            [self goToViewController:i direction:direction animated:(!self.animating && i == index) shouldCallOnPageSelected: i == index];
         }
     }
     
     if (diff == 0) {
-        [self goToViewController:index direction:direction animated:animated shouldCallOnPageSelected:YES];
+        [self goToViewController:index direction:direction animated:NO shouldCallOnPageSelected:YES];
     }
 }

I am facing the same issue when using Material-top-tabs.

I’m trying other solutions by counting the number of animation actions. I haven’t tried in production, but it seems ok on debug mode, it can prevent a crash, but sometimes if the animation does not finish yet, then you navigate to other screens it can be a bug. Maybe it can help and someone can improve it. Thx.

diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.h b/node_modules/react-native-pager-view/ios/ReactNativePageView.h
index 9e34051..e31b290 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.h
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.h
@@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property(nonatomic) BOOL overdrag;
 @property(nonatomic) NSString* layoutDirection;
 @property(nonatomic) CGRect previousBounds;
-
+@property(nonatomic) NSInteger numberOfAction;
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated;
 - (void)shouldScroll:(BOOL)scrollEnabled;
diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.m b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
index bebfb39..e39caa1 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.m
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
@@ -48,6 +48,7 @@ - (instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher {
         _overdrag = NO;
         _layoutDirection = @"ltr";
         _previousBounds = CGRectMake(0, 0, 0, 0);
+        _numberOfAction = 0;
     }
     return self;
 }
@@ -89,6 +90,11 @@ - (void)didMoveToWindow {
         [self embed];
         [self setupInitialController];
     }
+    
+    if (self.numberOfAction > 0) {
+        self.numberOfAction = 0;
+        [self updateDataSource];
+    }
 }
 
 - (void)embed {
@@ -189,12 +195,18 @@ - (void)setReactViewControllers:(NSInteger)index
         strongSelf.currentIndex = index;
         strongSelf.currentView = controller.view;
         
-        if (strongSelf.eventDispatcher) {
-            if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
-                if (shouldCallOnPageSelected) {
-                    [strongSelf.eventDispatcher sendEvent:[[RCTOnPageSelected alloc] initWithReactTag:strongSelf.reactTag position:@(index) coalescingKey:coalescingKey]];
+        if (finished == YES) {
+            if (self.numberOfAction > 0) {
+                self.numberOfAction -= 1;
+            }
+                        
+            if (strongSelf.eventDispatcher) {
+                if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
+                    if (shouldCallOnPageSelected) {
+                        [strongSelf.eventDispatcher sendEvent:[[RCTOnPageSelected alloc] initWithReactTag:strongSelf.reactTag position:@(index) coalescingKey:coalescingKey]];
+                    }
+                    strongSelf.lastReportedIndex = strongSelf.currentIndex;
                 }
-                strongSelf.lastReportedIndex = strongSelf.currentIndex;
             }
         }
     }];
@@ -233,11 +245,9 @@ - (void)updateDataSource {
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated {
     NSInteger numberOfPages = self.reactSubviews.count;
-    
-    if (numberOfPages == 0 || index < 0 || index > numberOfPages - 1) {
+    if (numberOfPages == 0 || index < 0 || index > numberOfPages - 1 || self.numberOfAction > 0) {
         return;
     }
-    
     BOOL isForward = (index > self.currentIndex && [self isLtrLayout]) || (index < self.currentIndex && ![self isLtrLayout]);
     UIPageViewControllerNavigationDirection direction = isForward ? UIPageViewControllerNavigationDirectionForward : UIPageViewControllerNavigationDirectionReverse;
     
@@ -250,6 +260,7 @@ - (void)goTo:(NSInteger)index animated:(BOOL)animated {
             if (i == _currentIndex) {
                 continue;
             }
+            _numberOfAction += 1;
             [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
         }
     }
@@ -257,9 +268,10 @@ - (void)goTo:(NSInteger)index animated:(BOOL)animated {
     if (!isForward && diff > 0) {
         for (NSInteger i=_currentIndex; i>=index; i--) {
             // Prevent removal of one or many pages at a time
-            if (index == _currentIndex || i >= numberOfPages) {
+            if (i == _currentIndex || i >= numberOfPages) {
                 continue;
             }
+            _numberOfAction += 1;
             [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
         }
     }
@@ -267,6 +279,7 @@ - (void)goTo:(NSInteger)index animated:(BOOL)animated {
     if (diff == 0) {
         [self goToViewController:index direction:direction animated:animated shouldCallOnPageSelected:YES];
     }
+    
 }
 
 - (void)goToViewController:(NSInteger)index

New issue with the update. The new update fixed issue with app crashing when switching screens to fast however the new issue is as follow: I am using React Navigation Top Tabs which depends on this library. If I press (onPress) my tabs to fast then the top tabs stop responding. The indicator gets stuck. I can still swipe left or right to get to other screens from the tabs however the tab bar gets stuck. This only happens when I click on 3 or 4 tabs very quickly.

Is there a PR up for this? Instead of using a patch?

Same problem. Even with the patch.

@RZulfikri Also working fine in Production

There are two ways to fix it:

  1. block the button, once animation is happening (implement debounce)
  2. block setPage method, when screen are transitioning

Ad.1 - the simplest one, no lib changes requires, will not the fix the issue in the library Ad.2 - I would prefer to handle it inside the library and I suggest to implement the second approach

Due to limited time, I am not able to implement it. If anyone would like to take a look on it, I am eager to help.

#469 (comment)

@troZee I took a stab at implementing your recommended solution 2, please take a look here: https://github.com/callstack/react-native-pager-view/pull/510

For RTL + NSInternalInconsistencyException: Unexpected view controller fix react-native-pager-view+5.4.9.patch

diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.h b/node_modules/react-native-pager-view/ios/ReactNativePageView.h
index 9e34051..e31b290 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.h
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.h
@@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property(nonatomic) BOOL overdrag;
 @property(nonatomic) NSString* layoutDirection;
 @property(nonatomic) CGRect previousBounds;
-
+@property(nonatomic) NSInteger numberOfAction;
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated;
 - (void)shouldScroll:(BOOL)scrollEnabled;
diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.m b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
index bebfb39..4980b89 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.m
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
@@ -48,6 +48,7 @@
         _overdrag = NO;
         _layoutDirection = @"ltr";
         _previousBounds = CGRectMake(0, 0, 0, 0);
+        _numberOfAction = 0;
     }
     return self;
 }
@@ -89,6 +90,11 @@
         [self embed];
         [self setupInitialController];
     }
+    
+    if (self.numberOfAction > 0) {
+        self.numberOfAction = 0;
+        [self updateDataSource];
+    }
 }
 
 - (void)embed {
@@ -189,12 +195,18 @@
         strongSelf.currentIndex = index;
         strongSelf.currentView = controller.view;
         
-        if (strongSelf.eventDispatcher) {
-            if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
-                if (shouldCallOnPageSelected) {
-                    [strongSelf.eventDispatcher sendEvent:[[RCTOnPageSelected alloc] initWithReactTag:strongSelf.reactTag position:@(index) coalescingKey:coalescingKey]];
+        if (finished == YES) {
+            if (self.numberOfAction > 0) {
+                self.numberOfAction -= 1;
+            }
+                        
+            if (strongSelf.eventDispatcher) {
+                if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
+                    if (shouldCallOnPageSelected) {
+                        [strongSelf.eventDispatcher sendEvent:[[RCTOnPageSelected alloc] initWithReactTag:strongSelf.reactTag position:@(index) coalescingKey:coalescingKey]];
+                    }
+                    strongSelf.lastReportedIndex = strongSelf.currentIndex;
                 }
-                strongSelf.lastReportedIndex = strongSelf.currentIndex;
             }
         }
     }];
@@ -233,11 +245,9 @@
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated {
     NSInteger numberOfPages = self.reactSubviews.count;
-    
-    if (numberOfPages == 0 || index < 0 || index > numberOfPages - 1) {
+    if (numberOfPages == 0 || index < 0 || index > numberOfPages - 1 || self.numberOfAction > 0) {
         return;
     }
-    
     BOOL isForward = (index > self.currentIndex && [self isLtrLayout]) || (index < self.currentIndex && ![self isLtrLayout]);
     UIPageViewControllerNavigationDirection direction = isForward ? UIPageViewControllerNavigationDirectionForward : UIPageViewControllerNavigationDirectionReverse;
     
@@ -245,21 +255,23 @@
     self.reactPageIndicatorView.currentPage = index;
     long diff = labs(index - _currentIndex);
     
-    if (isForward && diff > 0) {
+    if (index > _currentIndex) {
         for (NSInteger i=_currentIndex; i<=index; i++) {
             if (i == _currentIndex) {
                 continue;
             }
+            _numberOfAction += 1;
             [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
         }
     }
     
-    if (!isForward && diff > 0) {
+    if (index < _currentIndex) {
         for (NSInteger i=_currentIndex; i>=index; i--) {
             // Prevent removal of one or many pages at a time
-            if (index == _currentIndex || i >= numberOfPages) {
+            if (i == _currentIndex || i >= numberOfPages) {
                 continue;
             }
+            _numberOfAction += 1;
             [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
         }
     }
@@ -267,6 +279,7 @@
     if (diff == 0) {
         [self goToViewController:index direction:direction animated:animated shouldCallOnPageSelected:YES];
     }
+    
 }
 
 - (void)goToViewController:(NSInteger)index

Patch works!

@troZee any news on this issue ? thanks

@elmcapp try to read this https://www.callstack.com/blog/say-goodbye-to-old-fashioned-forks-thanks-to-the-patch-package. But in this case, you can do:

  1. install patch-package (https://www.npmjs.com/package/patch-package)
  2. create a new folder in the root directory called patches
  3. copy that patch to a new file with name react-native-pager-view+5.4.9.patch (use react-native-page-view v5.4.9) and put into folder patches
  4. do yarn install to apply a patch

same issue…

I adapted @wbercx patch for react-native-pager-view@5.4.9:

diff --git a/node_modules/react-native-pager-view/ios/ReactNativePageView.m b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
index 78f266b..c8abb4f 100644
--- a/node_modules/react-native-pager-view/ios/ReactNativePageView.m
+++ b/node_modules/react-native-pager-view/ios/ReactNativePageView.m
@@ -20,6 +20,7 @@
 
 @property(nonatomic, strong) NSHashTable<UIViewController *> *cachedControllers;
 @property (nonatomic, assign) CGPoint lastContentOffset;
+@property(nonatomic, assign) BOOL animating;
 
 - (void)goTo:(NSInteger)index animated:(BOOL)animated;
 - (void)shouldScroll:(BOOL)scrollEnabled;
@@ -180,6 +181,10 @@
     }
     __weak ReactNativePageView *weakSelf = self;
     uint16_t coalescingKey = _coalescingKey++;
+
+    if (animated == YES) {
+        self.animating = YES;
+    }
     
     [self.reactPageViewController setViewControllers:@[controller]
                                            direction:direction
@@ -188,6 +193,10 @@
         __strong typeof(self) strongSelf = weakSelf;
         strongSelf.currentIndex = index;
         strongSelf.currentView = controller.view;
+
+        if (finished) {
+            strongSelf.animating = NO;
+        }
         
         if (strongSelf.eventDispatcher) {
             if (strongSelf.lastReportedIndex != strongSelf.currentIndex) {
@@ -246,26 +255,19 @@
     long diff = labs(index - _currentIndex);
     
     if (isForward && diff > 0) {
-        for (NSInteger i=_currentIndex; i<=index; i++) {
-            if (i == _currentIndex) {
-                continue;
-            }
-            [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
+        for (NSInteger i=_currentIndex+1; i<=index; i++) {
+            [self goToViewController:i direction:direction animated:(!self.animating && i == index) shouldCallOnPageSelected: i == index];
         }
     }
     
     if (!isForward && diff > 0) {
-        for (NSInteger i=_currentIndex; i>=index; i--) {
-            // Prevent removal of one or many pages at a time
-            if (index == _currentIndex || i >= numberOfPages) {
-                continue;
-            }
-            [self goToViewController:i direction:direction animated:animated shouldCallOnPageSelected: i == index];
+        for (NSInteger i=_currentIndex-1; i>=index; i--) {
+            [self goToViewController:i direction:direction animated:(!self.animating && i == index) shouldCallOnPageSelected: i == index];
         }
     }
     
     if (diff == 0) {
-        [self goToViewController:index direction:direction animated:animated shouldCallOnPageSelected:YES];
+        [self goToViewController:index direction:direction animated:NO shouldCallOnPageSelected:YES];
     }
 }

I’m having this issue as well, including on 5.4.1 and 5.4.0.