iOS: prevent nativemodule access from JS if bridge is no longer valid

Summary: This helps prevent race condition where JS calls to NativeModules got queued and executed while the bridge is invalidating itself, causing assertion failures in test setup (for example). It won't prevent it 100% of the time, due to threading (and adding lock is expensive for each nativemodule call).

Reviewed By: yungsters

Differential Revision: D9231636

fbshipit-source-id: 298eaf52ffa4b84108184124e75b206b9ca7a41d

Author: fkgozali

Libraries/RCTTest/RCTTestRunner.m

@@ -115,20 +115,20 @@ - (void)runTest:(SEL)test module:(NSString *)moduleName
 {
   __weak RCTBridge *batchedBridge;
   NSNumber *rootTag;
-
-  @autoreleasepool {
-    __block NSMutableArray<NSString *> *errors = nil;
-    RCTLogFunction defaultLogFunction = RCTGetLogFunction();
-    RCTSetLogFunction(^(RCTLogLevel level, RCTLogSource source, NSString *fileName, NSNumber *lineNumber, NSString *message) {
-      defaultLogFunction(level, source, fileName, lineNumber, message);
-      if (level >= RCTLogLevelError) {
-        if (errors == nil) {
-          errors = [NSMutableArray new];
-        }
-        [errors addObject:message];
+  RCTLogFunction defaultLogFunction = RCTGetLogFunction();
+  // Catch all error logs, that are equivalent to redboxes in dev mode.
+  __block NSMutableArray<NSString *> *errors = nil;
+  RCTSetLogFunction(^(RCTLogLevel level, RCTLogSource source, NSString *fileName, NSNumber *lineNumber, NSString *message) {
+    defaultLogFunction(level, source, fileName, lineNumber, message);
+    if (level >= RCTLogLevelError) {
+      if (errors == nil) {
+        errors = [NSMutableArray new];
       }
-    });
+      [errors addObject:message];
+    }
+  });
 
+  @autoreleasepool {
     RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:_scriptURL
                                               moduleProvider:_moduleProvider
                                                launchOptions:nil];
@@ -172,7 +172,16 @@ - (void)runTest:(SEL)test module:(NSString *)moduleName
       testModule.view = nil;
     }
 
-    RCTSetLogFunction(defaultLogFunction);
+    // From this point on catch only fatal errors.
+    RCTSetLogFunction(^(RCTLogLevel level, RCTLogSource source, NSString *fileName, NSNumber *lineNumber, NSString *message) {
+      defaultLogFunction(level, source, fileName, lineNumber, message);
+      if (level >= RCTLogLevelFatal) {
+        if (errors == nil) {
+          errors = [NSMutableArray new];
+        }
+        [errors addObject:message];
+      }
+    });
 
 #if RCT_DEV
     NSArray<UIView *> *nonLayoutSubviews = [vc.view.subviews filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(id subview, NSDictionary *bindings) {
@@ -208,8 +217,10 @@ - (void)runTest:(SEL)test module:(NSString *)moduleName
     [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
     [[NSRunLoop mainRunLoop] runMode:NSRunLoopCommonModes beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
   }
-  // Note: this deallocation isn't consistently working in test setup, so disable the assertion.
-  // RCTAssert(batchedBridge == nil, @"Bridge should be deallocated after the test");
+  RCTAssert(errors == nil, @"RedBox errors during bridge invalidation: %@", errors);
+  RCTAssert(batchedBridge == nil, @"Bridge should be deallocated after the test");
+
+  RCTSetLogFunction(defaultLogFunction);
 }
 
 @end

React/CxxModule/RCTNativeModule.mm

@@ -71,11 +71,16 @@
     invokeInner(weakBridge, weakModuleData, methodId, std::move(params));
   };
 
-  dispatch_queue_t queue = m_moduleData.methodQueue;
-  if (queue == RCTJSThread) {
-    block();
-  } else if (queue) {
-    dispatch_async(queue, block);
+  if (m_bridge.valid) {
+    dispatch_queue_t queue = m_moduleData.methodQueue;
+    if (queue == RCTJSThread) {
+      block();
+    } else if (queue) {
+      dispatch_async(queue, block);
+    }
+  } else {
+    RCTLogError(@"Attempted to invoke `%u` (method ID) on `%@` (NativeModule name) with an invalid bridge.",
+                methodId, m_moduleData.name);
   }
 }