From 976f51abd907cb2ff8c5fa8fffa1d0abd7cc7222 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Tue, 2 Jun 2020 22:58:04 -0700
Subject: [PATCH] Execute sync methods on JS thread

Summary:
There are two ways to make a NativeModule method execute synchronously:
- Declare the NativeModule method to be synchronous (i.e: use `RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD`).
- Make the NativeModule synchronous (i.e: make its method queue `RCTJSThread`). This way, all its methods are synchronous.

`RCTNativeModule` executes all synchronous methods on the JS thread:
- Executing an async methods on a sync module: https://git.io/JfRPj
- Executing a sync method: https://git.io/JfRXe

However, in TurboModules we block the JS thread, execute the method on the NativeModule's method queue, and then unblock the JS thread. While this approach is thread-safe, and arguably the correct way to dispatch sync methods, it's also much slower than the alternative. Therefore, this diff migrates the legacy behaviour to the TurboModule system.

## Special Case: getConstants()
When an ObjC NativeModule requires main queue setup, and it exports constants, we execute its `constantsToExport` method on the main queue (see: [RCTModuleData gatherConstants](https://github.com/facebook/react-native/blob/c8d678abcf93fd3f6daf4bebfdf25937995c1fdf/React/Base/RCTModuleData.mm#L392-L402)). I replicated this behaviour with TurboModules.

Changelog:
[iOS][Fixed] - Execute ObjC TurboModule async method calls on JS thread for sync modules

Reviewed By: fkgozali

Differential Revision: D21602096

fbshipit-source-id: 42d07b7ad000abeac27091dc3ec440e3836d2eae
---
 .../turbomodule/core/platform/ios/RCTTurboModule.h | 14 +++++++-------
 .../core/platform/ios/RCTTurboModule.mm            |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h
index 42dfd285928..abf072a2b78 100644
--- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h
+++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h
@@ -40,7 +40,6 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule {
     id<RCTTurboModule> instance;
     std::shared_ptr<CallInvoker> jsInvoker;
     std::shared_ptr<CallInvoker> nativeInvoker;
-    // Does the NativeModule dispatch async methods to the JS thread?
     bool isSyncModule;
   };
 
@@ -61,16 +60,20 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule {
   void setMethodArgConversionSelector(NSString *methodName, int argIndex, NSString *fnName);
 
  private:
+  // Does the NativeModule dispatch async methods to the JS thread?
+  const bool isSyncModule_;
+
   /**
    * TODO(ramanpreet):
    * Investigate an optimization that'll let us get rid of this NSMutableDictionary.
    */
   NSMutableDictionary<NSString *, NSMutableArray *> *methodArgConversionSelectors_;
   NSDictionary<NSString *, NSArray<NSString *> *> *methodArgumentTypeNames_;
-  const bool isSyncModule_;
-  bool isMethodSync(TurboModuleMethodValueKind returnType);
-  NSString *getArgumentTypeName(NSString *methodName, int argIndex);
 
+  bool isMethodSync(TurboModuleMethodValueKind returnType);
+  BOOL hasMethodArgConversionSelector(NSString *methodName, int argIndex);
+  SEL getMethodArgConversionSelector(NSString *methodName, int argIndex);
+  NSString *getArgumentTypeName(NSString *methodName, int argIndex);
   NSInvocation *getMethodInvocation(
       jsi::Runtime &runtime,
       TurboModuleMethodValueKind returnType,
@@ -86,9 +89,6 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule {
       NSInvocation *inv,
       NSMutableArray *retainedObjectsForInvocation);
 
-  BOOL hasMethodArgConversionSelector(NSString *methodName, int argIndex);
-  SEL getMethodArgConversionSelector(NSString *methodName, int argIndex);
-
   using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper);
   jsi::Value
   createPromise(jsi::Runtime &runtime, std::shared_ptr<react::CallInvoker> jsInvoker, PromiseInvocationBlock invoke);
diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
index b44010d0304..4e5cdc8a4ee 100644
--- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
+++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
@@ -361,7 +361,7 @@ jsi::Value ObjCTurboModule::performMethodInvocation(
   };
 
   if (wasMethodSync) {
-    nativeInvoker_->invokeSync([block]() -> void { block(); });
+    block();
   } else {
     asyncCallCounter = getUniqueId();
     TurboModulePerfLogger::asyncMethodCallDispatch(moduleName, methodName);