Skip to content

Commit b7812a8

Browse files
dmytrorykunfacebook-github-bot
authored andcommitted
Fix race condition in native module invalidation (#44048)
Summary: Pull Request resolved: #44048 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_leave` 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. Reviewed By: cipolleschi Differential Revision: D55965290 fbshipit-source-id: 0c2b5957371bf3573155cee687c661603da162de
1 parent 91d3bc5 commit b7812a8

1 file changed

Lines changed: 28 additions & 14 deletions

File tree

packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModuleManager.mm

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ static Class getFallbackClassFromName(const char *name)
178178
return moduleClass;
179179
}
180180

181+
typedef struct {
182+
id<RCTBridgeModule> module;
183+
dispatch_queue_t methodQueue;
184+
} ModuleQueuePair;
185+
181186
@implementation RCTTurboModuleManager {
182187
std::shared_ptr<CallInvoker> _jsInvoker;
183188
__weak id<RCTTurboModuleManagerDelegate> _delegate;
@@ -1033,7 +1038,7 @@ - (void)_invalidateModules
10331038
{
10341039
// Backward-compatibility: RCTInvalidating handling.
10351040
dispatch_group_t moduleInvalidationGroup = dispatch_group_create();
1036-
1041+
std::vector<ModuleQueuePair> modulesToInvalidate;
10371042
for (auto &pair : _moduleHolders) {
10381043
std::string moduleName = pair.first;
10391044
ModuleHolder *moduleHolder = &pair.second;
@@ -1056,22 +1061,31 @@ - (void)_invalidateModules
10561061
[module class]);
10571062
continue;
10581063
}
1064+
modulesToInvalidate.push_back({module, methodQueue});
1065+
}
1066+
}
10591067

1060-
dispatch_group_enter(moduleInvalidationGroup);
1061-
dispatch_block_t invalidateModule = ^{
1062-
[((id<RCTInvalidating>)module) invalidate];
1063-
dispatch_group_leave(moduleInvalidationGroup);
1064-
};
1068+
for (auto unused : modulesToInvalidate) {
1069+
dispatch_group_enter(moduleInvalidationGroup);
1070+
}
10651071

1066-
if (_bridge) {
1067-
[_bridge dispatchBlock:invalidateModule queue:methodQueue];
1072+
for (auto &moduleQueuePair : modulesToInvalidate) {
1073+
id<RCTBridgeModule> module = moduleQueuePair.module;
1074+
dispatch_queue_t methodQueue = moduleQueuePair.methodQueue;
1075+
1076+
dispatch_block_t invalidateModule = ^{
1077+
[((id<RCTInvalidating>)module) invalidate];
1078+
dispatch_group_leave(moduleInvalidationGroup);
1079+
};
1080+
1081+
if (_bridge) {
1082+
[_bridge dispatchBlock:invalidateModule queue:methodQueue];
1083+
} else {
1084+
// Bridgeless mode
1085+
if (methodQueue == RCTJSThread) {
1086+
invalidateModule();
10681087
} else {
1069-
// Bridgeless mode
1070-
if (methodQueue == RCTJSThread) {
1071-
invalidateModule();
1072-
} else {
1073-
dispatch_async(methodQueue, invalidateModule);
1074-
}
1088+
dispatch_async(methodQueue, invalidateModule);
10751089
}
10761090
}
10771091
}

0 commit comments

Comments
 (0)