From 814e1ebff6ec4bceebc69df1aec69def1ad35311 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 21 Feb 2025 16:41:30 -0800 Subject: [PATCH] avoid sync dispatch with locked mutex in RCTKeyWindowValuesProxy (#49605) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/49605 changelog: [internal] Calling `RCTUnsafeExecuteOnMainQueueSync` while holding a lock can lead to a deadlock. In this diff, we remove it from RCTKeyWindowValuesProxy. Reviewed By: javache Differential Revision: D69997888 fbshipit-source-id: a09fc641c9fb2aec59aef34e4047e1ef11cdaf02 --- .../UIKitProxies/RCTKeyWindowValuesProxy.mm | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/react-native/React/Base/UIKitProxies/RCTKeyWindowValuesProxy.mm b/packages/react-native/React/Base/UIKitProxies/RCTKeyWindowValuesProxy.mm index a53c0f9de06..bbf118c8a1a 100644 --- a/packages/react-native/React/Base/UIKitProxies/RCTKeyWindowValuesProxy.mm +++ b/packages/react-native/React/Base/UIKitProxies/RCTKeyWindowValuesProxy.mm @@ -44,21 +44,28 @@ static NSString *const kFrameKeyPath = @"frame"; - (void)startObservingWindowSizeIfNecessary { - std::lock_guard lock(_mutex); - if (!_isObserving) { + // Accesing _isObserving must be done under the lock to avoid a race condition. + // We can't hold the lock while calling RCTUnsafeExecuteOnMainQueueSync. + // Therefore, reading/writing _isObserving is kept separate from calling RCTUnsafeExecuteOnMainQueueSync. + { + std::lock_guard lock(_mutex); + if (_isObserving) { + return; + } _isObserving = YES; - // For backwards compatibility, we register for notifications from the main thread only. - // On the new architecture, we are already on the main thread and RCTUnsafeExecuteOnMainQueueSync will simply call - // the block. - RCTUnsafeExecuteOnMainQueueSync(^{ - [RCTKeyWindow() addObserver:self forKeyPath:kFrameKeyPath options:NSKeyValueObservingOptionNew context:nil]; - }); - - [[NSNotificationCenter defaultCenter] addObserver:self - selector:@selector(_interfaceOrientationDidChange) - name:UIApplicationDidBecomeActiveNotification - object:nil]; } + + // For backwards compatibility, we register for notifications from the main thread only. + // On the new architecture, we are already on the main thread and RCTUnsafeExecuteOnMainQueueSync will simply call + // the block. + RCTUnsafeExecuteOnMainQueueSync(^{ + [RCTKeyWindow() addObserver:self forKeyPath:kFrameKeyPath options:NSKeyValueObservingOptionNew context:nil]; + }); + + [[NSNotificationCenter defaultCenter] addObserver:self + selector:@selector(_interfaceOrientationDidChange) + name:UIApplicationDidBecomeActiveNotification + object:nil]; } - (void)observeValueForKeyPath:(NSString *)keyPath