Skip to content

Commit 42b1e31

Browse files
Autopilot Botfacebook-github-bot
authored andcommitted
Fix use-after-free race in RCTInstance callFunctionOnJSModule
Summary: WARNING: Generated by Autopilot (alpha) — review carefully, verify the underlying claim before accepting. Agent: Bugs Agent | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/4b82dbca-6e63-49ae-828e-b8401b428572/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/4503602613718376/ --- Twilight iOS build 391 crashes with `EXC_BAD_ACCESS` / `KERN_INVALID_ADDRESS at 0x30` in `BufferedRuntimeExecutor::execute`, from `-[RCTSettingsManager userDefaultsDidChange:]` -> `-[RCTInstance callFunctionOnJSModule:method:args:]` -> `ReactInstance::callFunctionOnModule`, on a background `FBAppJobs` queue during cold start. `callFunctionOnJSModule` is called from arbitrary threads (here, an off-main NSUserDefaults write by the code-usage app job). It checked `_valid` and then dereferenced `_reactInstance` with nothing stopping `-invalidate` from tearing down the `ReactInstance` in between. Before 7c6ccb595d69, the `bufferedRuntimeExecutor_ == nullptr` early return in `ReactInstance::callFunctionOnModule` happened to hide this, because freed memory read back as zero. Removing that check turned the latent use-after-free into a crash. The fix takes `_invalidationMutex`, the lock `-_loadScriptFromSource` already uses for this purpose and that `-invalidate` holds while setting `_valid = false`. The check and the dereference now happen together relative to teardown. The `folly::dynamic` conversion stays outside the lock because the `settingsUpdated` payload is the whole NSUserDefaults dictionary. `callFunctionOnModule` only enqueues work on the runtime executor and never runs JS inline, so holding the lock adds no re-entrancy or deadlock risk. Not restoring the removed null check is deliberate: it relies on allocator behavior and only masks the use-after-free. Other methods in this file (`registerSegmentWithId:`, `_handleMemoryWarning`, the `getBufferedRuntimeExecutor()` sites) and the macOS copy have the same unguarded pattern and are left for follow-up. Differential Revision: D122831570
1 parent 7c007b3 commit 42b1e31

1 file changed

Lines changed: 12 additions & 3 deletions

File tree

  • packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon

‎packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,19 @@ - (void)dealloc
171171

172172
- (void)callFunctionOnJSModule:(NSString *)moduleName method:(NSString *)method args:(NSArray *)args
173173
{
174-
if (_valid) {
175-
_reactInstance->callFunctionOnModule(
176-
[moduleName UTF8String], [method UTF8String], convertIdToFollyDynamic(args ? args : @[]));
174+
// Convert outside the lock: the payload can be large, and -invalidate blocks on
175+
// _invalidationMutex.
176+
auto dynamicArgs = convertIdToFollyDynamic(args ? args : @[]);
177+
std::string moduleNameString = [moduleName UTF8String] ?: "";
178+
std::string methodString = [method UTF8String] ?: "";
179+
180+
// This is called from arbitrary threads, while -invalidate destroys _reactInstance on
181+
// the JS thread, so checking _valid and dereferencing have to happen as one step.
182+
std::lock_guard<std::mutex> lock(_invalidationMutex);
183+
if (!_valid || !_reactInstance) {
184+
return;
177185
}
186+
_reactInstance->callFunctionOnModule(moduleNameString, methodString, std::move(dynamicArgs));
178187
}
179188

180189
- (void)invalidate

0 commit comments

Comments
 (0)