Fabric: Using shared mutex to protect scheduler object in RCTSurfacePresenter

Summary:
This fixes a deadlock caused by recursive calling of `-[RCTSurfacePresenter setMinimumSize:minimumSize:]` (because of an attempt to lock the mutex which was already locked upper the call stack).
Seems there is no reason why this operation should not be allowed. This diff replaces a regular mutex with a shared one which allows multiple shared locks and this prevents the deadlock. A Scheduler is already a thread-safe object and the mutex only protected the mutation of a pointer to it.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D19249490

fbshipit-source-id: 00e8934c6502328f34b78ea6ec004b7216665db1

React/Fabric/RCTSurfacePresenter.mm

@@ -40,13 +40,14 @@ using namespace facebook::react;
 @end
 
 @implementation RCTSurfacePresenter {
-  std::mutex _schedulerMutex;
-  RCTScheduler
-      *_Nullable _scheduler; // Thread-safe. Mutation of the instance variable is protected by `_schedulerMutex`.
-  ContextContainer::Shared _contextContainer;
-  RuntimeExecutor _runtimeExecutor;
   RCTMountingManager *_mountingManager; // Thread-safe.
   RCTSurfaceRegistry *_surfaceRegistry; // Thread-safe.
+
+  better::shared_mutex _schedulerMutex;
+  RCTScheduler *_Nullable _scheduler; // Thread-safe. Pointer is protected by `_schedulerMutex`.
+  ContextContainer::Shared _contextContainer; // Protected by `_schedulerMutex`.
+  RuntimeExecutor _runtimeExecutor; // Protected by `_schedulerMutex`.
+
   better::shared_mutex _observerListMutex;
   NSMutableArray<id<RCTSurfacePresenterObserver>> *_observers;
 }
@@ -74,33 +75,33 @@ using namespace facebook::react;
 
 - (ContextContainer::Shared)contextContainer
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   return _contextContainer;
 }
 
 - (void)setContextContainer:(ContextContainer::Shared)contextContainer
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::unique_lock<better::shared_mutex> lock(_schedulerMutex);
   _contextContainer = contextContainer;
 }
 
-- (void)setRuntimeExecutor:(RuntimeExecutor)runtimeExecutor
-{
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
-  _runtimeExecutor = runtimeExecutor;
-}
-
 - (RuntimeExecutor)runtimeExecutor
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   return _runtimeExecutor;
 }
 
+- (void)setRuntimeExecutor:(RuntimeExecutor)runtimeExecutor
+{
+  std::unique_lock<better::shared_mutex> lock(_schedulerMutex);
+  _runtimeExecutor = runtimeExecutor;
+}
+
 #pragma mark - Internal Surface-dedicated Interface
 
 - (void)registerSurface:(RCTFabricSurface *)surface
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   [_surfaceRegistry registerSurface:surface];
   if (_scheduler) {
     [self _startSurface:surface];
@@ -109,7 +110,7 @@ using namespace facebook::react;
 
 - (void)unregisterSurface:(RCTFabricSurface *)surface
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   if (_scheduler) {
     [self _stopSurface:surface];
   }
@@ -118,7 +119,7 @@ using namespace facebook::react;
 
 - (void)setProps:(NSDictionary *)props surface:(RCTFabricSurface *)surface
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   // This implementation is suboptimal indeed but still better than nothing for now.
   [self _stopSurface:surface];
   [self _startSurface:surface];
@@ -133,7 +134,7 @@ using namespace facebook::react;
                       maximumSize:(CGSize)maximumSize
                           surface:(RCTFabricSurface *)surface
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   LayoutContext layoutContext = {.pointScaleFactor = RCTScreenScale()};
   LayoutConstraints layoutConstraints = {.minimumSize = RCTSizeFromCGSize(minimumSize),
                                          .maximumSize = RCTSizeFromCGSize(maximumSize)};
@@ -144,7 +145,7 @@ using namespace facebook::react;
 
 - (void)setMinimumSize:(CGSize)minimumSize maximumSize:(CGSize)maximumSize surface:(RCTFabricSurface *)surface
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   LayoutContext layoutContext = {.pointScaleFactor = RCTScreenScale()};
   LayoutConstraints layoutConstraints = {.minimumSize = RCTSizeFromCGSize(minimumSize),
                                          .maximumSize = RCTSizeFromCGSize(maximumSize)};
@@ -155,7 +156,7 @@ using namespace facebook::react;
 
 - (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::shared_lock<better::shared_mutex> lock(_schedulerMutex);
   ReactTag tag = [reactTag integerValue];
   UIView<RCTComponentViewProtocol> *componentView =
       [_mountingManager.componentViewRegistry findComponentViewWithTag:tag];
@@ -175,7 +176,7 @@ using namespace facebook::react;
 
 - (BOOL)suspend
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::unique_lock<better::shared_mutex> lock(_schedulerMutex);
 
   if (!_scheduler) {
     return NO;
@@ -189,7 +190,7 @@ using namespace facebook::react;
 
 - (BOOL)resume
 {
-  std::lock_guard<std::mutex> lock(_schedulerMutex);
+  std::unique_lock<better::shared_mutex> lock(_schedulerMutex);
 
   if (_scheduler) {
     return NO;