From 88814d52ddd4af15be1bf986e2b49ce6d2043a69 Mon Sep 17 00:00:00 2001
From: Paige Sun <paigesun@fb.com>
Date: Wed, 6 Apr 2022 20:29:30 -0700
Subject: [PATCH] Fix: Make RCTSurfacePresenter weakly retain its observers

Summary:
Changelog: [Fabric][iOS] Fix: Make RCTSurfacePresenter weakly retain its observers

There is retain cycle because RCTSurfacePresenter is keeping an array of  RCTSurfacePresenterObserver, which is strongly retaining the class that owns this RCTSurfacePresenter.

This diff makes RCTSurfacePresenter weakly retain observers instead.

Reviewed By: RSNara

Differential Revision: D35439589

fbshipit-source-id: ddc7813976b543de12af6173b2f1b31c69b043a8
---
 React/Fabric/RCTSurfacePresenter.mm | 42 ++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm
index dd7a1bd0fb5..59937990722 100644
--- a/React/Fabric/RCTSurfacePresenter.mm
+++ b/React/Fabric/RCTSurfacePresenter.mm
@@ -80,7 +80,7 @@ static BackgroundExecutor RCTGetBackgroundExecutor()
   RuntimeExecutor _runtimeExecutor; // Protected by `_schedulerLifeCycleMutex`.
 
   butter::shared_mutex _observerListMutex;
-  NSMutableArray<id<RCTSurfacePresenterObserver>> *_observers;
+  std::vector<__weak id<RCTSurfacePresenterObserver>> _observers; // Protected by `_observerListMutex`.
 }
 
 - (instancetype)initWithContextContainer:(ContextContainer::Shared)contextContainer
@@ -96,8 +96,6 @@ static BackgroundExecutor RCTGetBackgroundExecutor()
     _mountingManager.contextContainer = contextContainer;
     _mountingManager.delegate = self;
 
-    _observers = [NSMutableArray array];
-
     _scheduler = [self _createScheduler];
 
     auto reactNativeConfig = _contextContainer->at<std::shared_ptr<ReactNativeConfig const>>("ReactNativeConfig");
@@ -386,13 +384,17 @@ static BackgroundExecutor RCTGetBackgroundExecutor()
 - (void)addObserver:(id<RCTSurfacePresenterObserver>)observer
 {
   std::unique_lock<butter::shared_mutex> lock(_observerListMutex);
-  [self->_observers addObject:observer];
+  _observers.push_back(observer);
 }
 
 - (void)removeObserver:(id<RCTSurfacePresenterObserver>)observer
 {
   std::unique_lock<butter::shared_mutex> lock(_observerListMutex);
-  [self->_observers removeObject:observer];
+  std::vector<__weak id<RCTSurfacePresenterObserver>>::const_iterator it =
+      std::find(_observers.begin(), _observers.end(), observer);
+  if (it != _observers.end()) {
+    _observers.erase(it);
+  }
 }
 
 #pragma mark - RCTMountingManagerDelegate
@@ -401,8 +403,13 @@ static BackgroundExecutor RCTGetBackgroundExecutor()
 {
   RCTAssertMainQueue();
 
-  std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
-  for (id<RCTSurfacePresenterObserver> observer in _observers) {
+  NSArray<id<RCTSurfacePresenterObserver>> *observersCopy;
+  {
+    std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
+    observersCopy = [self _getObservers];
+  }
+
+  for (id<RCTSurfacePresenterObserver> observer in observersCopy) {
     if ([observer respondsToSelector:@selector(willMountComponentsWithRootTag:)]) {
       [observer willMountComponentsWithRootTag:rootTag];
     }
@@ -413,12 +420,29 @@ static BackgroundExecutor RCTGetBackgroundExecutor()
 {
   RCTAssertMainQueue();
 
-  std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
-  for (id<RCTSurfacePresenterObserver> observer in _observers) {
+  NSArray<id<RCTSurfacePresenterObserver>> *observersCopy;
+  {
+    std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
+    observersCopy = [self _getObservers];
+  }
+
+  for (id<RCTSurfacePresenterObserver> observer in observersCopy) {
     if ([observer respondsToSelector:@selector(didMountComponentsWithRootTag:)]) {
       [observer didMountComponentsWithRootTag:rootTag];
     }
   }
 }
 
+- (NSArray<id<RCTSurfacePresenterObserver>> *)_getObservers
+{
+  NSMutableArray<id<RCTSurfacePresenterObserver>> *observersCopy = [NSMutableArray new];
+  for (id<RCTSurfacePresenterObserver> observer : _observers) {
+    if (observer) {
+      [observersCopy addObject:observer];
+    }
+  }
+
+  return observersCopy;
+}
+
 @end