Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔥 Performance Monitoring module crashing the app on iOS #3736

Closed
2 of 10 tasks
MrSltunUxbert opened this issue Jun 3, 2020 · 11 comments · Fixed by #3978
Closed
2 of 10 tasks

🔥 Performance Monitoring module crashing the app on iOS #3736

MrSltunUxbert opened this issue Jun 3, 2020 · 11 comments · Fixed by #3978
Labels
impact: crash Behaviour causing app to crash. platform: ios plugin: performance Firebase Performance Monitoring

Comments

@MrSltunUxbert
Copy link

Issue

I installed the performance monitoring module into our application, it works fine and reports to firebase, but sometimes it the app crashes.
And the issue was Collection <__NSDictionaryM: 0x600002b7e1c0> was mutated while being enumerated

I looked into it and read the error reports, it seemed like the function dealloc in RNFBPerfModule.m was causing the issue.

image

What I did was, I changed the dealloc function to this:

   - (void)dealloc {
     @synchronized ([self class]) {
-      for (NSString *key in traces) {
+      for (NSString *key in [traces allKeys]) {
         [traces removeObjectForKey:key];
       }
 
-      for (NSString *key in httpMetrics) {
+      for (NSString *key in [httpMetrics allKeys]) {
         [httpMetrics removeObjectForKey:key];
       }
     }

reference: https://coderwall.com/p/qsty1w/safely-remove-nsmutabledictionary-objects-in-a-loop

I'm not an experienced iOS developer, so I don't know if that's a good solution or not, but it worked for me and since I changed the code, the app didn't crash.

If that's a good solution, I'd be happy to send a PR.


Project Files

Javascript

Click To Expand

package.json:

{
  "name": "XYZ",
  "version": "1.0.0",
  "private": true,
  "scripts": {
    "android": "react-native run-android",
    "ios": "react-native run-ios",
    "start": "react-native start",
    "test": "jest",
    "lint": "eslint .",
  },
  "dependencies": {
    "@expo/react-native-action-sheet": "^3.6.0",
    "@react-native-community/async-storage": "^1.8.1",
    "@react-native-community/datetimepicker": "^2.3.0",
    "@react-native-community/masked-view": "^0.1.7",
    "@react-native-community/netinfo": "^5.8.0",
    "@react-native-firebase/analytics": "^6.3.4",
    "@react-native-firebase/app": "^6.3.4",
    "@react-native-firebase/crashlytics": "^6.3.4",
    "@react-native-firebase/perf": "6.3.4",
    "@react-navigation/bottom-tabs": "^5.1.1",
    "@react-navigation/native": "^5.0.9",
    "@react-navigation/stack": "^5.2.0",
    "axios": "^0.19.2",
    "date-fns": "^2.10.0",
    "date-fns-tz": "^1.0.10",
    "mobx": "^5.15.4",
    "mobx-react": "^6.1.8",
    "moment": "^2.24.0",
    "react": "16.9.0",
    "react-hook-form": "^5.1.3",
    "react-native": "0.61.5",
    "react-native-android-keyboard-adjust": "^1.2.0",
    "react-native-animatable": "^1.3.3",
    "react-native-calendar-events": "^1.7.3",
    "react-native-collapsible": "^1.5.1",
    "react-native-gesture-handler": "^1.6.0",
    "react-native-image-zoom-viewer": "^2.2.27",
    "react-native-invertible-scroll-view": "^2.0.0",
    "react-native-iphone-x-helper": "^1.2.1",
    "react-native-keyboard-aware-scroll-view": "^0.9.1",
    "react-native-linear-gradient": "^2.5.6",
    "react-native-modal-datetime-picker": "^8.5.1",
    "react-native-network-info": "^5.2.1",
    "react-native-pdf": "^6.1.1",
    "react-native-permissions": "^2.0.9",
    "react-native-picker-select": "^6.6.0",
    "react-native-radio-buttons-ext": "^2.0.0",
    "react-native-read-more-text": "^1.1.2",
    "react-native-reanimated": "^1.7.0",
    "react-native-restart": "^0.0.14",
    "react-native-safe-area-context": "^0.7.3",
    "react-native-screens": "^2.3.0",
    "react-native-share": "^3.1.2",
    "react-native-swiper": "^1.6.0",
    "react-native-vector-icons": "^6.6.0",
    "react-native-webview": "^9.0.1",
    "rn-fetch-blob": "^0.12.0",
    "validator": "^12.2.0"
  },
  "devDependencies": {
    "@babel/core": "^7.8.7",
    "@babel/runtime": "^7.8.7",
    "@react-native-community/eslint-config": "^0.0.7",
    "babel-jest": "^25.1.0",
    "eslint": "^6.8.0",
    "jest": "^25.1.0",
    "metro-react-native-babel-preset": "^0.58.0",
    "patch-package": "^6.2.2",
    "pre-commit": "^1.2.2",
    "react-test-renderer": "16.9.0"
  },
  "jest": {
    "preset": "react-native"
  }
}

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:
platform :ios, '9.0'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

target 'XYZ' do
  # Harpy
  pod 'Harpy'

  # Pods for XYZ
  pod 'FBLazyVector', :path => "../node_modules/react-native/Libraries/FBLazyVector"
  pod 'FBReactNativeSpec', :path => "../node_modules/react-native/Libraries/FBReactNativeSpec"
  pod 'RCTRequired', :path => "../node_modules/react-native/Libraries/RCTRequired"
  pod 'RCTTypeSafety', :path => "../node_modules/react-native/Libraries/TypeSafety"
  pod 'React', :path => '../node_modules/react-native/'
  pod 'React-Core', :path => '../node_modules/react-native/'
  pod 'React-CoreModules', :path => '../node_modules/react-native/React/CoreModules'
  pod 'React-Core/DevSupport', :path => '../node_modules/react-native/'
  pod 'React-RCTActionSheet', :path => '../node_modules/react-native/Libraries/ActionSheetIOS'
  pod 'React-RCTAnimation', :path => '../node_modules/react-native/Libraries/NativeAnimation'
  pod 'React-RCTBlob', :path => '../node_modules/react-native/Libraries/Blob'
  pod 'React-RCTImage', :path => '../node_modules/react-native/Libraries/Image'
  pod 'React-RCTLinking', :path => '../node_modules/react-native/Libraries/LinkingIOS'
  pod 'React-RCTNetwork', :path => '../node_modules/react-native/Libraries/Network'
  pod 'React-RCTSettings', :path => '../node_modules/react-native/Libraries/Settings'
  pod 'React-RCTText', :path => '../node_modules/react-native/Libraries/Text'
  pod 'React-RCTVibration', :path => '../node_modules/react-native/Libraries/Vibration'
  pod 'React-Core/RCTWebSocket', :path => '../node_modules/react-native/'

  pod 'React-cxxreact', :path => '../node_modules/react-native/ReactCommon/cxxreact'
  pod 'React-jsi', :path => '../node_modules/react-native/ReactCommon/jsi'
  pod 'React-jsiexecutor', :path => '../node_modules/react-native/ReactCommon/jsiexecutor'
  pod 'React-jsinspector', :path => '../node_modules/react-native/ReactCommon/jsinspector'
  pod 'ReactCommon/jscallinvoker', :path => "../node_modules/react-native/ReactCommon"
  pod 'ReactCommon/turbomodule/core', :path => "../node_modules/react-native/ReactCommon"
  pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga'

  pod 'DoubleConversion', :podspec => '../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec'
  pod 'glog', :podspec => '../node_modules/react-native/third-party-podspecs/glog.podspec'
  pod 'Folly', :podspec => '../node_modules/react-native/third-party-podspecs/Folly.podspec'

  # react-native-permissions
  pod 'Permission-Calendars', :path => '../node_modules/react-native-permissions/ios/Calendars.podspec'
  
  use_native_modules!
end

AppDelegate.m:

#import "AppDelegate.h"

#import <React/RCTBridge.h>
#import <React/RCTBundleURLProvider.h>
#import <React/RCTRootView.h>
#import <React/RCTI18nUtil.h>
#import <Firebase.h>
#if !DEVELOPMENT
#import "Harpy.h"
#endif


@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  if ([FIRApp defaultApp] == nil) {
      [FIRApp configure];
  }
  
  [[RCTI18nUtil sharedInstance] allowRTL:YES];
  BOOL isRTL = [[RCTI18nUtil sharedInstance] isRTL];
  
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
  RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge
                                                   moduleName:@"XYZ"
                                            initialProperties:nil];

  rootView.backgroundColor = [[UIColor alloc] initWithRed:1.0f green:1.0f blue:1.0f alpha:1];

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  self.window.tintColor = [UIColor colorWithRed:0.15 green:0.31 blue:0.91 alpha:1.00];
  [self.window makeKeyAndVisible];
  #if !DEVELOPMENT
  if (isRTL) {
    // Sets the language of the message of the new update
    [[Harpy sharedInstance] setForceLanguageLocalization:HarpyLanguageArabic];
  }

  // Present Window before calling Harpy
  // Set the UIViewController that will present an instance of UIAlertController
  [[Harpy sharedInstance] setAlertType:HarpyAlertTypeForce];
  [[Harpy sharedInstance] setPresentingViewController:_window.rootViewController];
  #endif

  return YES;
}

- (void)applicationDidBecomeActive:(UIApplication *)application {
#if !DEVELOPMENT
  /*
   Perform daily check for new version of your app
   Useful if user returns to you app from background after extended period of time
   Place in applicationDidBecomeActive:

   Also, performs version check on first launch.
   */
  [[Harpy sharedInstance] checkVersionDaily];
#endif
}

- (void)applicationWillEnterForeground:(UIApplication *)application {
  /*
   Perform check for new version of your app
   Useful if user returns to you app from background after being sent tot he App Store,
   but doesn't update their app before coming back to your app.

    ONLY USE THIS IF YOU ARE USING *HarpyAlertTypeForce*

    Also, performs version check on first launch.
   */
  [[Harpy sharedInstance] checkVersion];
}

- (NSURL *)sourceURLForBridge:(RCTBridge *)bridge
{
#if DEBUG
  return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
#else
  return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"];
#endif
}

@end


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • I am using android/gradle.settings jetifier=true for Android compatibility?
  • I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->


Environment

Click To Expand

react-native info output:

System:
    OS: macOS 10.15.4
    CPU: (4) x64 Intel(R) Core(TM) i5-6360U CPU @ 2.00GHz
    Memory: 256.90 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 14.3.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.4, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
    Android SDK:
      API Levels: 23, 25, 26, 27, 28, 29
      Build Tools: 23.0.1, 23.0.3, 25.0.0, 25.0.2, 25.0.3, 26.0.1, 26.0.2, 26.0.3, 27.0.3, 28.0.3, 29.0.2, 29.0.3
      System Images: android-21 | Google APIs ARM EABI v7a, android-27 | Google APIs Intel x86 Atom, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom
  IDEs:
    Android Studio: 3.6 AI-192.7142.36.36.6241897
    Xcode: 11.4/11E146 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.9.0 => 16.9.0
    react-native: 0.61.5 => 0.61.5
  npmGlobalPackages:
    react-native-cli: 2.0.1
    react-native-create-bridge: 2.0.1
    react-native-create-library: 3.1.2
    react-native-git-upgrade: 0.2.7

  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • 6.3.4
  • Firebase module(s) you're using that has the issue:
    • Performance Monitoring
  • Are you using TypeScript?
    • No

@mikehardy
Copy link
Collaborator

@Salakar / @Ehesp crash report with workaround and offer for PR, look good?
@MrSltunUxbert I'd pre-emptively make the PR if I were you, I know those two are really busy and a reply might take a while
You may have seen this already but if not look into 'patch-package' as the best way to keep your local change integrated reproducibly while waiting for resolution here

@mikehardy mikehardy added impact: crash Behaviour causing app to crash. platform: ios plugin: performance Firebase Performance Monitoring labels Jun 3, 2020
@mikehardy
Copy link
Collaborator

Oh - @MrSltunUxbert make sure it still applies on new versions, I just checked yours and you look like you are a little bit behind current

@MrSltunUxbert
Copy link
Author

@mikehardy I will update the package to the latest version on npm and test if it still crashes

@Ehesp
Copy link
Member

Ehesp commented Jun 4, 2020

Yeah that looks correct, I think we had this problem on a few of the other modules.

@patrickschmelter
Copy link

@MrSltunUxbert After a little bit of testing it seems to be working with the current release too (7.1.4)

@gilbertl
Copy link
Contributor

Having the same problem. When will the fix make it into the official release? Alternatively, @MrSltunUxbert do you have a version that we can point our package.json towards?

@mikehardy
Copy link
Collaborator

@gilbertl -> https://github.com/ds300/patch-package - don't develop with react-native modules without it...

@dburdan
Copy link

dburdan commented Jun 18, 2020

In the meantime, if you use patch-package you can use this patch to fix it until a new release is out.

@react-native-firebase+perf+7.1.3.patch

--- a/node_modules/@react-native-firebase/perf/ios/RNFBPerf/RNFBPerfModule.m
+++ b/node_modules/@react-native-firebase/perf/ios/RNFBPerf/RNFBPerfModule.m
@@ -44,11 +44,11 @@

   - (void)dealloc {
     @synchronized ([self class]) {
-      for (NSString *key in traces) {
+      for (NSString *key in [traces allKeys]) {
         [traces removeObjectForKey:key];
       }

-      for (NSString *key in httpMetrics) {
+      for (NSString *key in [httpMetrics allKeys]) {
         [httpMetrics removeObjectForKey:key];
       }
     }

@gilbertl
Copy link
Contributor

@gilbertl -> https://github.com/ds300/patch-package - don't develop with react-native modules without it...

This is great. Didn't know about it. Thanks so much.

@stale
Copy link

stale bot commented Jul 18, 2020

Hello 👋, to help manage issues we automatically close stale issues.
This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Jul 18, 2020
@gilbertl
Copy link
Contributor

I don't think this issue has been fixed yet?

@stale stale bot removed the Type: Stale Issue has become stale - automatically added by Stale bot label Jul 20, 2020
andersonaddo added a commit that referenced this issue Jul 21, 2020
Salakar pushed a commit that referenced this issue Jul 23, 2020
Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>
[publish]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: crash Behaviour causing app to crash. platform: ios plugin: performance Firebase Performance Monitoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants