Skip to content

Commit af9a25a

Browse files
zeyapfacebook-github-bot
authored andcommitted
Fix RCTInstance self-deadlock during invalidate
Summary: `-[RCTInstance invalidate]` held `_invalidationMutex` while it synchronously called `[_surfacePresenter suspend]`. Stopping surfaces notifies mount observers on the calling thread, and any observer that reaches `callFunctionOnJSModule:` (e.g. `RCTLog` forwarding to JS in debug builds) blocks forever on the same non-recursive mutex. Release the mutex after setting `_valid = false` and unregistering from the inspector, before `suspend`. Re-entrant calls now see `!_valid` and return. Changelog: [iOS][Fixed] - Fix deadlock in `RCTInstance` invalidation when a surface observer calls into JS Reviewed By: javache Differential Revision: D125706316
1 parent 96dddb0 commit af9a25a

2 files changed

Lines changed: 35 additions & 4 deletions

File tree

‎packages/react-native/ReactCommon/react/runtime/iostests/RCTInstanceTests.mm‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#import <React/RCTBridgeModule.h>
1212
#import <React/RCTBundleManager.h>
1313
#import <React/RCTJavaScriptLoader.h>
14+
#import <React/RCTSurfacePresenter.h>
1415
#import <ReactCommon/RCTHermesInstance.h>
1516
#import <ReactCommon/RCTInstance.h>
1617
#import <ReactCommon/RCTTurboModule.h>
@@ -241,4 +242,30 @@ - (void)testBundleLoadAwaitsMainQueueModuleSetup
241242
@"FakeEagerModule -init must have fully returned before the JS bundle finishes loading");
242243
}
243244

245+
- (void)testInvalidateDoesNotDeadlockWhenSuspendCallsIntoJS
246+
{
247+
OCMStub([_mockDelegate unstableModulesRequiringMainQueueSetup]).andReturn(@[]);
248+
249+
RCTInstance *instance = [self makeInstance];
250+
__weak RCTInstance *weakInstance = instance;
251+
252+
// Stands in for a surface observer that calls into JS while suspend stops surfaces.
253+
id presenterMock = OCMPartialMock(instance.surfacePresenter);
254+
OCMStub([(RCTSurfacePresenter *)presenterMock suspend]).andDo(^(NSInvocation *invocation) {
255+
[weakInstance callFunctionOnJSModule:@"RCTLog" method:@"logIfNoNativeHook" args:@[]];
256+
BOOL result = YES;
257+
[invocation setReturnValue:&result];
258+
});
259+
260+
// Invalidate off-main so a deadlock fails the wait instead of hanging the test runner.
261+
XCTestExpectation *invalidated = [self expectationWithDescription:@"invalidate returned"];
262+
dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{
263+
[instance invalidate];
264+
[invalidated fulfill];
265+
});
266+
[self waitForExpectations:@[ invalidated ] timeout:5.0];
267+
268+
[presenterMock stopMocking];
269+
}
270+
244271
@end

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,15 @@ - (void)callFunctionOnJSModule:(NSString *)moduleName method:(NSString *)method
183183

184184
- (void)invalidate
185185
{
186-
std::lock_guard<std::mutex> lock(_invalidationMutex);
187-
_valid = false;
188-
if (_reactInstance) {
189-
_reactInstance->unregisterFromInspector();
186+
{
187+
std::lock_guard<std::mutex> lock(_invalidationMutex);
188+
_valid = false;
189+
if (_reactInstance) {
190+
_reactInstance->unregisterFromInspector();
191+
}
190192
}
193+
// suspend shouldn't be called under _invalidationMutex: it notifies surface observers,
194+
// which can call callFunctionOnJSModule and deadlock on the same mutex.
191195
[_surfacePresenter suspend];
192196
[_jsThreadManager dispatchToJSThread:^{
193197
/**

0 commit comments

Comments
 (0)