iOS: gate CodePush-triggered reloads on a Fabric readiness signal

Author: ofalvai

ios/CodePush/CodePush.m

@@ -1,6 +1,7 @@
 #if __has_include(<React/RCTAssert.h>)
 #import <React/RCTAssert.h>
 #import <React/RCTBridgeModule.h>
+#import <React/RCTConstants.h>
 #import <React/RCTConvert.h>
 #import <React/RCTEventDispatcher.h>
 #import <React/RCTRootView.h>
@@ -36,6 +37,11 @@ @implementation CodePush {
     BOOL _allowed;
     BOOL _restartInProgress;
     NSMutableArray *_restartQueue;
+
+    // Reload readiness tracking. See -registerSettleObservers for details.
+    BOOL _instanceSettled;
+    // YES exactly while a reload is parked waiting for the instance to settle.
+    BOOL _reloadPending;
 }
 
 RCT_EXPORT_MODULE()
@@ -45,6 +51,11 @@ @implementation CodePush {
 // These constants represent emitted events
 static NSString *const DownloadProgressEvent = @"CodePushDownloadProgress";
 
+// How long a parked reload waits for a settle signal before reloading anyway.
+// See -registerSettleObservers for why this exists and why the exact value is
+// not critical.
+static const NSTimeInterval SettleTimeout = 5.0;
+
 // These constants represent valid deployment statuses
 static NSString *const DeploymentFailed = @"DeploymentFailed";
 static NSString *const DeploymentSucceeded = @"DeploymentSucceeded";
@@ -395,15 +406,88 @@ - (instancetype)init
     _allowed = YES;
     _restartInProgress = NO;
     _restartQueue = [NSMutableArray arrayWithCapacity:1];
-    
+
     self = [super init];
     if (self) {
+        [self registerSettleObservers];
         [self initializeUpdateAfterRestart];
     }
 
     return self;
 }
 
+#pragma mark - Immediate update + reload readiness tracking
+
+/*
+ * In case of an immediate bundle update, we need to tear down the current
+ * RCTInstance at the right time.
+ *
+ * Right after bundle evaluation, RN enqueues -[RCTFabricSurface start] on a
+ * background queue for that same instance. Tearing the instance down while
+ * that block is in flight crashes inside RN's mounting layer. This module is
+ * initialized *during* bundle evaluation, so in case of immediate update
+ * mode, -loadBundle needs to park the reload until we see the surface get
+ * past its startup.
+ *
+ * The two signals below are the possible outcomes of an instance's startup:
+ *   - RCTContentDidAppearNotification: Fabric posts this from
+ *     RCTRootComponentView on the first child mount. That needs a JS render
+ *     and commit, so by that time we are good to tear things down.
+ *   - RCTJavaScriptDidFailToLoadNotification: the bundle never evaluated, so
+ *     no surface startup will follow.
+ *
+ * Note: neither signal is guaranteed, so we also need a fallback timeout. If
+ * the first render returns nil and mounts no child, RCTContentDidAppear
+ * never arrives. Think of a splash gate or a fonts/auth loader. That kind of
+ * app could run codepush.sync() with an IMMEDIATE install before it shows
+ * any UI.
+ *
+ * Running the teardown after the timeout fires is safe because the delay is
+ * large enough that the teardown already happened by that time.
+ */
+- (void)registerSettleObservers
+{
+    NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
+    [center addObserver:self
+               selector:@selector(instanceDidSettle)
+                   name:RCTContentDidAppearNotification
+                 object:nil];
+    [center addObserver:self
+               selector:@selector(instanceDidSettle)
+                   name:RCTJavaScriptDidFailToLoadNotification
+                 object:nil];
+}
+
+- (void)instanceDidSettle
+{
+    // Note: A reload tears the instance down synchronously.
+    // Calling -releasePendingReload directly would destroy the surface partway through
+    // the mount that just notified us.
+    dispatch_async(dispatch_get_main_queue(), ^{
+        self->_instanceSettled = YES;
+
+        if (self->_reloadPending) {
+            CPLog(@"Instance settled. Restarting app.");
+            [self releasePendingReload];
+        }
+    });
+}
+
+/*
+ * Fires a parked reload, if one is still parked. Called both by the settle
+ * signal and by the timeout in -loadBundle, so whichever arrives first wins and
+ * the other becomes a no-op. Main queue only.
+ */
+- (void)releasePendingReload
+{
+    if (!_reloadPending) {
+        return;
+    }
+
+    _reloadPending = NO;
+    [self performBundleReload];
+}
+
 /*
  * This method is used when the app is started to either
  * initialize a pending update or rollback a faulty update
@@ -547,18 +631,45 @@ - (void)loadBundle
     // This needs to be async dispatched because the bridge is not set on init
     // when the app first starts, therefore rollbacks will not take effect.
     dispatch_async(dispatch_get_main_queue(), ^{
-        // If the current bundle URL is using http(s), then assume the dev
-        // is debugging and therefore, shouldn't be redirected to a local
-        // file (since Chrome wouldn't support it). Otherwise, update
-        // the current bundle URL to point at the latest update
-        if ([CodePush isUsingTestConfiguration] || ![super.bridge.bundleURL.scheme hasPrefix:@"http"]) {
-            [super.bridge setValue:[CodePush bundleURL] forKey:@"bundleURL"];
+        if (!self->_instanceSettled) {
+            CPLog(@"Restart deferred until the current instance has settled.");
+            self->_reloadPending = YES;
+
+            // Weak, so that a module outliving the teardown of its own instance (because of the timer)
+            // cannot reload against a bridge that no longer belongs to it.
+            __weak __typeof(self) weakSelf = self;
+            dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(SettleTimeout * NSEC_PER_SEC)),
+                           dispatch_get_main_queue(), ^{
+                __typeof(self) strongSelf = weakSelf;
+                if (strongSelf && strongSelf->_reloadPending) {
+                    CPLog(@"Timed out waiting for the current instance to settle. Restarting app anyway.");
+                    [strongSelf releasePendingReload];
+                }
+            });
+            return;
         }
 
-        RCTTriggerReloadCommandListeners(@"react-native-code-push: Restart");
+        [self performBundleReload];
     });
 }
 
+/*
+ * Performs the actual reload. Must be called on the main queue, and only once the
+ * current instance has settled or the wait for it timed out. See -loadBundle.
+ */
+- (void)performBundleReload
+{
+    // If the current bundle URL is using http(s), then assume the dev
+    // is debugging and therefore, shouldn't be redirected to a local
+    // file (since Chrome wouldn't support it). Otherwise, update
+    // the current bundle URL to point at the latest update
+    if ([CodePush isUsingTestConfiguration] || ![super.bridge.bundleURL.scheme hasPrefix:@"http"]) {
+        [super.bridge setValue:[CodePush bundleURL] forKey:@"bundleURL"];
+    }
+
+    RCTTriggerReloadCommandListeners(@"react-native-code-push: Restart");
+}
+
 /*
  * This method is used when a pending update never finished loading (i.e. it
  * crashed before calling notifyApplicationReady) and needs to be rolled back
@@ -804,7 +915,7 @@ - (void)restartAppInternal:(BOOL)onlyIfUpdateIsPending
 
     _restartInProgress = NO;
     if ([_restartQueue count] > 0) {
-        BOOL buf = [_restartQueue valueForKey: @"@firstObject"];
+        BOOL buf = [[_restartQueue firstObject] boolValue];
         [_restartQueue removeObjectAtIndex:0];
         [self restartAppInternal:buf];
     }
@@ -1010,7 +1121,7 @@ - (void)restartAppInternal:(BOOL)onlyIfUpdateIsPending
 
     if ([_restartQueue count] > 0) {
         CPLog(@"Executing pending restart.");
-        BOOL buf = [_restartQueue valueForKey: @"@firstObject"];
+        BOOL buf = [[_restartQueue firstObject] boolValue];
         [_restartQueue removeObjectAtIndex:0];
         [self restartAppInternal:buf];
     }