Experiencing crashes when implementing RCTEventEmitter on Turbo Modules

[jerielng]

Hi, I'm following this guide here to port my RCTEventEmitter logic to support both the old architecture and the New Architecture. This is really helpful, @cipolleschi, thank you so much!

It seems to work pretty well, and when compiling with the old architecture, everything works as before. It's also registering and observing events when compiled as a Turbo Module as well, but I'm noticing about 50% of the time in the Turbo Module case, I'm getting crashes when calling sendEvent because _callableJSModules is nil in my RCTEventEmitter.

Specifically, it appears to be triggered by this assert here:

RCTAssert(
      _callableJSModules != nil,
      @"Error when sending event: %@ with body: %@. "
       "RCTCallableJSModules is not set. This is probably because you've "
       "explicitly synthesized the RCTCallableJSModules in %@, even though it's inherited "
       "from RCTEventEmitter.",
      eventName,
      body,
      [self class]);

I haven't overridden the synthesized callableJSModules in my Turbo Module, but noticed that if I did, the crash happens 100% of the time.

Do we have any thoughts on what might be causing this to happen some of the time?

[cipolleschi]

Hi @jerielng, thanks for the question and I am happy that you are finding the guide useful. 😄

I don't have a specific pointer right now. When you say 50% of the time, you mean that it systematically sends one event and the next one make the app crash or that every time you launch the app there is a 50% chance that the _callableJSModules is nil?

Can you share a PR or a repro we can use to investigate the issue?

[jerielng]

Hey @robinheinze, in your case, by any chance, are you able to reproduce the crash if you remove the initialization for RCTBridge:

RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge
                                                  moduleName:@"ManualTestApp"
                                            initialProperties:nil];

if (@available(iOS 13.0, *)) {
  rootView.backgroundColor = [UIColor systemBackgroundColor];
} else {
  rootView.backgroundColor = [UIColor whiteColor];
}

self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
UIViewController *rootViewController = [UIViewController new];
rootViewController.view = rootView;
self.window.rootViewController = rootViewController;
[self.window makeKeyAndVisible];

I've noticed in my testing scenario, removing the initialization to RCTBridge seems to resolve the issue, but this may not be ideal in a situation where someone might be relying on it, although it does point to something, I hope.

@cipolleschi I've tried to recreate a separate repo, which crashes when backgrounding the app, but unfortunately, not for the same reason as my live scenario. I'll continue to work on making it as accurate as possible, but might the above insight on RCTBridge mean anything?

[cipolleschi]

removing the initialization to RCTBridge seems to resolve the issue,

Out of curiosity, was the app working at all once removed the Bridge initialization? 🤔

As far as I know, we still need the bridge in the New Architecture because not everything is actually migrated to the New Arch.
But, it could be that something in React Native is actually creating a second instance of the bridge. By removing the bridge init, we delegate the creation of the bridge to this second mechanism. This could also explain the randomicity: the double init could be a race condition and there is 50% chance that the wrong bridge is init after the right one...

These are just speculation, though, I haven't had time to dig into that code path, unfortunately.

[jerielng]

Out of curiosity, was the app working at all once removed the Bridge initialization? 🤔

Surprisingly, yes - it still works as expected on both the old architecture and the New Architecture.

For more context, this is our currently published sample AppDelegate, although what's published in this sample app and our module have not been migrated yet.

Apart from a call to enable Turbo Modules, the only thing I've changed during the migration is updating the initialization method for RCTBridge from:

NSURL *jsCodeLocation =
      [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index"];
  RCTRootView *rootView = [[RCTRootView alloc] initWithBundleURL:jsCodeLocation
                                                      moduleName:self.moduleName
                                               initialProperties:self.initialProps
                                                   launchOptions:launchOptions];

to

RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self
                                              launchOptions:launchOptions];

That being said, the part I commented out to encounter a case with no crashes in the event emitter was lines 27-37.

[cipolleschi]

ah... of course. If you extend RCTAppDelegate, you can remove a lot of code.
RCTAppDelegate does a lot of things for you: initialize the bridge, for example, and prepare a default UIView and UIViewController.

You can apply these changes to simplify your code this:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  self.moduleName = @"BrazeProject";
  self.initialProps = @{};

// 👇 This can be moved in a template method, see below 👇 
-  // Setup Braze bridge
-  NSURL *jsCodeLocation =
-      [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index"];
- RCTRootView *rootView = [[RCTRootView alloc] initWithBundleURL:jsCodeLocation
                                                      moduleName:self.moduleName
                                               initialProperties:self.initialProps
                                                   launchOptions:launchOptions];
// 👇🏻 You don't need this, RCTAppDelegate prepare this for you 👇🏻
-  // Configure views in the application
-  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
-  UIViewController *rootViewController = [UIViewController new];
-  rootViewController.view = rootView;
-  self.window.rootViewController = rootViewController;
-  [self.window makeKeyAndVisible];

  // Setup Braze
  BRZConfiguration *configuration = [[BRZConfiguration alloc] initWithApiKey:apiKey endpoint:endpoint];
  configuration.triggerMinimumTimeInterval = 1;
  configuration.logger.level = BRZLoggerLevelInfo;
  Braze *braze = [BrazeReactBridge initBraze:configuration];
  AppDelegate.braze = braze;

  [self registerForPushNotifications];
  [[BrazeReactUtils sharedInstance] populateInitialUrlFromLaunchOptions:launchOptions];

  return [super application:application didFinishLaunchingWithOptions:launchOptions];
}
 // ...

// 👇 This is a [Template Method pattern](https://refactoring.guru/design-patterns/template-method) to customize the UIView, hooking into the RCTAppDelegate life cycle 👇 
+- (UIView *)createRootViewWithBridge:(RCTBridge *)bridge
+                          moduleName:(NSString *)moduleName
+                           initProps:(NSDictionary *)initProps
+{
+  return [[RCTRootView alloc] initWithBundleURL:[self sourceURLForBridge:bridge]
+                                                      moduleName:self.moduleName
+                                               initialProperties:self.initialProps
+                                                   launchOptions:launchOptions];;
+}

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

The reason why we introduced the RCTAppDelegate was to extremely simplify the amount of code required by your app. You should not care about initializing React Native in the app delegate, but only, and eventually, customizing it if needed.

We prepared several methods in the RCTAppDelegate you can hook into to customize the behaviour.

[jerielng]

Thanks @cipolleschi - this was really insightful!

Digging through the source was really helpful! It turns out that the default implementation of createRootViewWithBridge has a conditional based on whether Fabric is enabled. If so, it uses a completely different initializer for the root view, using RCTFabricSurfaceHostingProxyRootView instead of RCTRootView.

What was happening here is the creation and assignation of two separate UIView classes:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
  ...
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self
                                                launchOptions:launchOptions];
  RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge
                                                    moduleName:self.moduleName
                                             initialProperties:self.initialProps];
  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];
}

But in the default implementation of RCTAppDelegate, the above happens again and overrides rootViewController.view with a difference instance of RCTFabricSurfaceHostingProxyRootView instead of RCTRootView:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
   ...
  NSDictionary *initProps = [self prepareInitialProps];
  UIView *rootView = [self createRootViewWithBridge:self.bridge moduleName:self.moduleName initProps:initProps];

  if (@available(iOS 13.0, *)) {
    rootView.backgroundColor = [UIColor systemBackgroundColor];
  } else {
    rootView.backgroundColor = [UIColor whiteColor];
  }

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [self createRootViewController];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];
}

What I imagine was happening here was since these are two separate classes, we would have two different instances maintained by the app, which is why in a New Architecture context, you could potentially receive either the RCTRootView or the RCTFabricSurfaceHostingProxyRootView instance, hence the unpredictable crashes.