From d7ac21cec5492e180fbf3817af7be64ab121cb75 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Tue, 2 Jun 2020 22:58:04 -0700
Subject: [PATCH] Remove main queue execution of constantsToExport

Summary:
## Context
- If a NativeModule requires main queue setup, its `constantsToExport` method is executed on the main queue.
- In the TurboModule system, `constantsToExport` or `getConstants` is treated like a regular synchronous NativeModule method. Therefore, it's always executed on the JS thread.

This difference in behaviour is dangerous when we're A/B testing the TurboModule infra: One could write a NativeModule that requires main queue setup, and have it expose constants that access objects/state only accessible on the UI thread. This NativeModule would work fine in the legacy infra, which could be the case if the NativeModule author is testing locally. But once it ships to prod, it may run with the TurboModule system, and crash the application. To mitigate this risk, I'm removing this special main queue execution of `constantsToExport` from the legacy infrastructure.

## Consequences
- If a NativeModule's `constantsToExport` method accesses objects/state only accessible on the UI thread, it must do so by explicitly scheduling work on the main thread. I wrote up a codemod to fix this for our OSS modules: D21797048.
- Eagerly initialized NativeModules that required main queue setup had their constants calculated eagerly. After the changes in this diff, those NativeModules will have their constants calculated lazily. I don't think this is a big deal because only a handful of NativeModules are eagerly initialized, and eagerly initialized NativeModules are going away anyway.

Changelog:
[iOS][Removed] - Main queue execution of constantsToExport in NativeModules requiring main queue setup

Reviewed By: fkgozali

Differential Revision: D21829091

fbshipit-source-id: df21fd5fd2ef45a291c07400f360bba801ae290f
---
 React/Base/RCTModuleData.h      |  4 ++++
 React/Base/RCTModuleData.mm     | 15 ++++++++++++++-
 React/CxxBridge/RCTCxxBridge.mm |  4 +++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/React/Base/RCTModuleData.h b/React/Base/RCTModuleData.h
index 398c0a2b9f8d44..04d6271eeedd5b 100644
--- a/React/Base/RCTModuleData.h
+++ b/React/Base/RCTModuleData.h
@@ -8,6 +8,7 @@
 #import <Foundation/Foundation.h>
 
 #import <React/RCTInvalidating.h>
+#import "RCTDefines.h"
 
 @protocol RCTBridgeMethod;
 @protocol RCTBridgeModule;
@@ -94,3 +95,6 @@ typedef id<RCTBridgeModule> (^RCTBridgeModuleProvider)(void);
 @property (nonatomic, assign, readonly) BOOL implementsPartialBatchDidFlush;
 
 @end
+
+RCT_EXTERN void RCTSetIsMainQueueExecutionOfConstantsToExportDisabled(BOOL val);
+RCT_EXTERN BOOL RCTIsMainQueueExecutionOfConstantsToExportDisabled();
diff --git a/React/Base/RCTModuleData.mm b/React/Base/RCTModuleData.mm
index 14251ee6d590e6..c9c2ac79490ec9 100644
--- a/React/Base/RCTModuleData.mm
+++ b/React/Base/RCTModuleData.mm
@@ -29,6 +29,17 @@ int32_t getUniqueId()
   return counter++;
 }
 }
+static BOOL isMainQueueExecutionOfConstantToExportDisabled = NO;
+
+void RCTSetIsMainQueueExecutionOfConstantsToExportDisabled(BOOL val)
+{
+  isMainQueueExecutionOfConstantToExportDisabled = val;
+}
+
+BOOL RCTIsMainQueueExecutionOfConstantsToExportDisabled()
+{
+  return isMainQueueExecutionOfConstantToExportDisabled;
+}
 
 @implementation RCTModuleData {
   NSDictionary<NSString *, id> *_constantsToExport;
@@ -389,7 +400,8 @@ - (void)gatherConstants
      *    require.
      */
     BridgeNativeModulePerfLogger::moduleJSRequireEndingStart([moduleName UTF8String]);
-    if (_requiresMainQueueSetup) {
+
+    if (!RCTIsMainQueueExecutionOfConstantsToExportDisabled() && _requiresMainQueueSetup) {
       if (!RCTIsMainQueue()) {
         RCTLogWarn(@"Required dispatch_sync to load constants for %@. This may lead to deadlocks", _moduleClass);
       }
@@ -400,6 +412,7 @@ - (void)gatherConstants
     } else {
       _constantsToExport = [_instance constantsToExport] ?: @{};
     }
+
     RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"");
   } else {
     /**
diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm
index a8e2ae2a9a3a10..7b2fc3808bce16 100644
--- a/React/CxxBridge/RCTCxxBridge.mm
+++ b/React/CxxBridge/RCTCxxBridge.mm
@@ -907,7 +907,9 @@ - (void)_prepareModulesWithDispatchGroup:(dispatch_group_t)dispatchGroup
         if (self.valid && ![moduleData.moduleClass isSubclassOfClass:[RCTCxxModule class]]) {
           [self->_performanceLogger appendStartForTag:RCTPLNativeModuleMainThread];
           (void)[moduleData instance];
-          [moduleData gatherConstants];
+          if (!RCTIsMainQueueExecutionOfConstantsToExportDisabled()) {
+            [moduleData gatherConstants];
+          }
           [self->_performanceLogger appendStopForTag:RCTPLNativeModuleMainThread];
         }
       };