From 8ad810717ee1769aa5ff6c73e0c9bfa8c43a3bac Mon Sep 17 00:00:00 2001
From: Kevin Gozali <fkg@fb.com>
Date: Sun, 3 May 2020 09:30:31 -0700
Subject: [PATCH] iOS: when the bridge has been invalidated, NativeModule
 lookup should just return nil

Summary:
There's a corner case where:
* The bridge gets invalidated, and native modules are cleaning up themselves (but not done yet)
* Something asks for a NativeModule instance - ideally it should just get nil at this point
* If TM Manager is invalidating, we get nil correctly, but we continue thru the old NativeModule lookup logic
* The latter will fail anyway, and would throw a redbox (RCTLogError).

So, if the bridge is invalidated, if TM Manager returns nil, we should just return nil for old NativeModule lookup.

The module of interest is RCTImageLoader, which was requested by RCTImageView on deallocation. The problem is RCTImageView got dealloc'ed **after** the bridge has been invalidated, so the lookup would always fail...

Bonus: RCTImageView should just keep a weak ref to the RCTImageLoader, so that:
* if the imageLoader is still alive on image dealloc, it can still access them (last minute "meaningless" cleanup)
* if the imageLoader is gone, then the image deallocation doesn't do anything

Changelog: [iOS] [Fixed] - Fix module lookup race condition on bridge invalidation.

Reviewed By: p-sun, sammy-SC

Differential Revision: D21371845

fbshipit-source-id: 862dc07de18ddbfb90e87e24b8dbd001147ddce4
---
 Libraries/Image/RCTImageView.mm | 19 +++++++++++--------
 React/CxxBridge/RCTCxxBridge.mm |  5 +++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Libraries/Image/RCTImageView.mm b/Libraries/Image/RCTImageView.mm
index 6dac02d5ea8014..ad2f97592b5989 100644
--- a/Libraries/Image/RCTImageView.mm
+++ b/Libraries/Image/RCTImageView.mm
@@ -64,6 +64,9 @@ @implementation RCTImageView
   // Weak reference back to the bridge, for image loading
   __weak RCTBridge *_bridge;
 
+  // Weak reference back to the active image loader.
+  __weak id<RCTImageLoaderWithAttributionProtocol> _imageLoader;
+
   // The image source that's currently displayed
   RCTImageSource *_imageSource;
 
@@ -327,11 +330,13 @@ - (void)reloadImage
       [weakSelf imageLoaderLoadedImage:loadedImage error:error forImageSource:source partial:NO];
     };
 
-    id<RCTImageLoaderWithAttributionProtocol> imageLoader = [_bridge moduleForName:@"ImageLoader"
-                                                             lazilyLoadIfNecessary:YES];
-    RCTImageURLLoaderRequest *loaderRequest = [imageLoader loadImageWithURLRequest:source.request
-                                                                              size:imageSize
-                                                                             scale:imageScale
+    if (!_imageLoader) {
+      _imageLoader = [_bridge moduleForName:@"ImageLoader" lazilyLoadIfNecessary:YES];
+    }
+
+    RCTImageURLLoaderRequest *loaderRequest = [_imageLoader loadImageWithURLRequest:source.request
+                                                                               size:imageSize
+                                                                              scale:imageScale
                                                                            clipped:NO
                                                                         resizeMode:_resizeMode
                                                                        attribution:{
@@ -470,9 +475,7 @@ - (void)didMoveToWindow
 }
 
 - (void)dealloc {
-  id<RCTImageLoaderWithAttributionProtocol> imageLoader = [_bridge moduleForName:@"ImageLoader"
-                                                           lazilyLoadIfNecessary:YES];
-  [imageLoader trackURLImageDidDestroy:_loaderRequest];
+  [_imageLoader trackURLImageDidDestroy:_loaderRequest];
 }
 
 @end
diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm
index d8938d7033a24e..ec4090a862c3d1 100644
--- a/React/CxxBridge/RCTCxxBridge.mm
+++ b/React/CxxBridge/RCTCxxBridge.mm
@@ -488,6 +488,11 @@ - (id)moduleForName:(NSString *)moduleName lazilyLoadIfNecessary:(BOOL)lazilyLoa
   }
 
   // Module may not be loaded yet, so attempt to force load it here.
+  // Do this only if the bridge is still valid.
+  if (_didInvalidate) {
+    return nil;
+  }
+
   const BOOL result = [self.delegate respondsToSelector:@selector(bridge:didNotFindModule:)] &&
       [self.delegate bridge:self didNotFindModule:moduleName];
   if (result) {