From e36bfef0f1cadba0e9d5d647ead8d011cc0ea351 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 8 Sep 2021 12:49:15 -0700 Subject: [PATCH] Conditionally disable TurboModuleManager delegate locking Summary: ## Context Whenever the TurboModuleManager calls into its delegate, it [acquires a lock](https://www.internalfb.com/code/fbsource/[f14548634e72009989c844a2ef025915ef74159e]/xplat/js/react-native-github/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm?lines=429%2C513). We initially introduced this mutex (in D21170099 (https://github.com/facebook/react-native/commit/2c473e1a38c35957fe80b6c334a1983c034c2bbc)). It serializes access to the TurboModuleManager delegate during TurboModule create. ## Problems - When we call into the delegate, we acquire a lock, and call into arbitrary product code: getModuleClassFromName, getModuleInstanceFromClass. If any of these two product methods create another TurboModule, the application will deadlock, because we'll acquire the same std::mutex twice. ## Fix The delegate methods of TurboModuleManager are usually implemented as [switch cases over the NativeModule names or NativeModule classes](https://www.internalfb.com/code/fbsource/[f015e461de4e7a18d0d52a697a53086fe6a3b91c]/fbobjc/Apps/Wilde/FBReactModule2/FBReactModuleAPI/FBReactModuleAPI/Exported/FBReactModule.mm?lines=1481-1488%2C1490-1537%2C1539-1577). So, it should be safe to call into them concurrently for two different modules. So, while we could fix the problem by migrating the TurboModuleManager to an std::recursive_mutex, one could make an argument that this locking shouldn't even be necessary in the first place. We don't have this locking in the Android TurboModule system. ## Changes This diff introduces a flag in React Native that allows to to safely remove this TurboModuleManager delegate locking in production. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D30754875 fbshipit-source-id: d04a831c18a2a8b46e9bc07ddf690d8e4d0be8e0 --- React/Base/RCTBridge.h | 4 ++++ React/Base/RCTBridge.m | 12 ++++++++++++ .../core/platform/ios/RCTTurboModuleManager.mm | 18 ++++++++++++------ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index 7e8494ebdad..8eef1cece61 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -160,6 +160,10 @@ RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void); RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled); +// Turn off TurboModule delegate locking +RCT_EXTERN BOOL RCTTurboModuleManagerDelegateLockingDisabled(void); +RCT_EXTERN void RCTDisableTurboModuleManagerDelegateLocking(BOOL enabled); + typedef enum { kRCTGlobalScope, kRCTGlobalScopeUsingRetainJSCallback, diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 10503261d72..85533c04dfe 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -149,6 +149,18 @@ void RCTSetTurboModuleCleanupMode(RCTTurboModuleCleanupMode mode) turboModuleCleanupMode = mode; } +// Turn off TurboModule delegate locking +static BOOL turboModuleManagerDelegateLockingDisabled = NO; +BOOL RCTTurboModuleManagerDelegateLockingDisabled(void) +{ + return turboModuleManagerDelegateLockingDisabled; +} + +void RCTDisableTurboModuleManagerDelegateLocking(BOOL disabled) +{ + turboModuleManagerDelegateLockingDisabled = disabled; +} + @interface RCTBridge () @end diff --git a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm index 6b26168f465..c740faf1ba3 100644 --- a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm +++ b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm @@ -422,9 +422,12 @@ static Class getFallbackClassFromName(const char *name) */ if ([_delegate respondsToSelector:@selector(getModuleClassFromName:)]) { - std::lock_guard delegateGuard(_turboModuleManagerDelegateMutex); - - moduleClass = [_delegate getModuleClassFromName:moduleName]; + if (RCTTurboModuleManagerDelegateLockingDisabled()) { + moduleClass = [_delegate getModuleClassFromName:moduleName]; + } else { + std::lock_guard delegateGuard(_turboModuleManagerDelegateMutex); + moduleClass = [_delegate getModuleClassFromName:moduleName]; + } } if (!moduleClass) { @@ -506,9 +509,12 @@ static Class getFallbackClassFromName(const char *name) TurboModulePerfLogger::moduleCreateConstructStart(moduleName, moduleId); if ([_delegate respondsToSelector:@selector(getModuleInstanceFromClass:)]) { - std::lock_guard delegateGuard(_turboModuleManagerDelegateMutex); - - module = [_delegate getModuleInstanceFromClass:moduleClass]; + if (RCTTurboModuleManagerDelegateLockingDisabled()) { + module = [_delegate getModuleInstanceFromClass:moduleClass]; + } else { + std::lock_guard delegateGuard(_turboModuleManagerDelegateMutex); + module = [_delegate getModuleInstanceFromClass:moduleClass]; + } } else { module = [moduleClass new]; }