From 1393cb3378d55539791a411cf0c393ff0ed89441 Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Thu, 11 Apr 2024 09:05:02 -0700 Subject: [PATCH] Fix race condition in native module invalidation (#44048) Summary: This is an attempt to fix a couple of similar memory corruption crashes that happen during the deallocation of RCTHost. **Hypotheis:** there is a race condition between [this](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm#L1038-L1058) and [this](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm#L1062) chunks of work. I.e. let's say we are invalidating 10 modules. Because of the race condition it is possible that we call `dispatch_group_enter`/`dispatch_group_enter` for the first five before we call `dispatch_group_enter` for the sixth one. In that case we would resume at [dispatch_group_wait](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm#L1079), not waiting for the remaining modules to invalidate. That would lead to `RCTInstance` potentially being invalidated prematurely, deallocating all its ivars including `_turboModuleManager` and `_jsThreadManager`. That in turn would lead to memory access error during the invalidation of remaining modules. This diff is trying to solve this problem by calling `dispatch_group_enter` for all modules before any other work is performed. Changelog: [iOS][Fixed] - Fixed race condition in native module invalidation. Differential Revision: D55965290 --- .../ios/ReactCommon/RCTTurboModuleManager.mm | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm index 9a34ab1ab7d3..6599c1295fe5 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm @@ -178,6 +178,11 @@ static Class getFallbackClassFromName(const char *name) return moduleClass; } +typedef struct { + id module; + dispatch_queue_t methodQueue; +} ModuleQueuePair; + @implementation RCTTurboModuleManager { std::shared_ptr _jsInvoker; __weak id _delegate; @@ -1033,7 +1038,7 @@ - (void)_invalidateModules { // Backward-compatibility: RCTInvalidating handling. dispatch_group_t moduleInvalidationGroup = dispatch_group_create(); - + std::vector modulesToInvalidate; for (auto &pair : _moduleHolders) { std::string moduleName = pair.first; ModuleHolder *moduleHolder = &pair.second; @@ -1056,22 +1061,31 @@ - (void)_invalidateModules [module class]); continue; } + modulesToInvalidate.push_back({module, methodQueue}); + } + } - dispatch_group_enter(moduleInvalidationGroup); - dispatch_block_t invalidateModule = ^{ - [((id)module) invalidate]; - dispatch_group_leave(moduleInvalidationGroup); - }; + for (auto unused : modulesToInvalidate) { + dispatch_group_enter(moduleInvalidationGroup); + } - if (_bridge) { - [_bridge dispatchBlock:invalidateModule queue:methodQueue]; + for (auto &moduleQueuePair : modulesToInvalidate) { + id module = moduleQueuePair.module; + dispatch_queue_t methodQueue = moduleQueuePair.methodQueue; + + dispatch_block_t invalidateModule = ^{ + [((id)module) invalidate]; + dispatch_group_leave(moduleInvalidationGroup); + }; + + if (_bridge) { + [_bridge dispatchBlock:invalidateModule queue:methodQueue]; + } else { + // Bridgeless mode + if (methodQueue == RCTJSThread) { + invalidateModule(); } else { - // Bridgeless mode - if (methodQueue == RCTJSThread) { - invalidateModule(); - } else { - dispatch_async(methodQueue, invalidateModule); - } + dispatch_async(methodQueue, invalidateModule); } } }