[RTDB] Fix concurrency crash in FView (#15548)

Author: paulb777

FirebaseDatabase/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Concurrency crash in FView. (#15514)
+
 # 11.9.0
 - [fixed] Fix connection failure issue introduced in 10.27.0 by restoring the
   Socket Rocket implementation instead of `NSURLSessionWebSocket`. Note that

FirebaseDatabase/Sources/Core/View/FView.m

@@ -70,6 +70,8 @@ @interface FView ()
 @property(nonatomic, strong, readwrite) FQuerySpec *query;
 @property(nonatomic, strong) FViewProcessor *processor;
 @property(nonatomic, strong) FViewCache *viewCache;
+
+// All accesses must be guarded by @synchronized(self.eventRegistrations)
 @property(nonatomic, strong) NSMutableArray *eventRegistrations;
 @property(nonatomic, strong) FEventGenerator *eventGenerator;
 
@@ -165,11 +167,15 @@ - (id)initWithQuery:(FQuerySpec *)query
 }
 
 - (BOOL)isEmpty {
-    return self.eventRegistrations.count == 0;
+    @synchronized(self.eventRegistrations) {
+        return self.eventRegistrations.count == 0;
+    }
 }
 
 - (void)addEventRegistration:(id<FEventRegistration>)eventRegistration {
-    [self.eventRegistrations addObject:eventRegistration];
+    @synchronized(self.eventRegistrations) {
+        [self.eventRegistrations addObject:eventRegistration];
+    }
 }
 
 /**
@@ -181,31 +187,33 @@ - (void)addEventRegistration:(id<FEventRegistration>)eventRegistration {
 - (NSArray *)removeEventRegistration:(id<FEventRegistration>)eventRegistration
                          cancelError:(NSError *)cancelError {
     NSMutableArray *cancelEvents = [[NSMutableArray alloc] init];
-    if (cancelError != nil) {
-        NSAssert(eventRegistration == nil,
-                 @"A cancel should cancel all event registrations.");
-        FPath *path = self.query.path;
-        for (id<FEventRegistration> registration in self.eventRegistrations) {
-            FCancelEvent *maybeEvent =
-                [registration createCancelEventFromError:cancelError path:path];
-            if (maybeEvent) {
-                [cancelEvents addObject:maybeEvent];
+    @synchronized(self.eventRegistrations) {
+        if (cancelError != nil) {
+            NSAssert(eventRegistration == nil,
+                     @"A cancel should cancel all event registrations.");
+            FPath *path = self.query.path;
+            for (id<FEventRegistration> registration in self
+                     .eventRegistrations) {
+                FCancelEvent *maybeEvent =
+                    [registration createCancelEventFromError:cancelError
+                                                        path:path];
+                if (maybeEvent) {
+                    [cancelEvents addObject:maybeEvent];
+                }
             }
         }
-    }
 
-    if (eventRegistration) {
-        NSUInteger i = 0;
-        while (i < self.eventRegistrations.count) {
-            id<FEventRegistration> existing = self.eventRegistrations[i];
-            if ([existing matches:eventRegistration]) {
-                [self.eventRegistrations removeObjectAtIndex:i];
-            } else {
-                i++;
-            }
+        if (eventRegistration) {
+            NSIndexSet *indexesToRemove =
+                [self.eventRegistrations indexesOfObjectsPassingTest:^BOOL(
+                                             id<FEventRegistration> existing,
+                                             NSUInteger idx, BOOL *stop) {
+                  return [existing matches:eventRegistration];
+                }];
+            [self.eventRegistrations removeObjectsAtIndexes:indexesToRemove];
+        } else {
+            [self.eventRegistrations removeAllObjects];
         }
-    } else {
-        [self.eventRegistrations removeAllObjects];
     }
     return cancelEvents;
 }
@@ -270,7 +278,10 @@ - (NSArray *)generateEventsForChanges:(NSArray *)changes
                          registration:(id<FEventRegistration>)registration {
     NSArray *registrations;
     if (registration == nil) {
-        registrations = [[NSArray alloc] initWithArray:self.eventRegistrations];
+        @synchronized(self.eventRegistrations) {
+            registrations =
+                [[NSArray alloc] initWithArray:self.eventRegistrations];
+        }
     } else {
         registrations = [[NSArray alloc] initWithObjects:registration, nil];
     }

FirebaseDatabase/Tests/Integration/FIRDatabaseQueryTests.m

@@ -3563,7 +3563,6 @@ - (void)testListenForChildAddedWithLimitEnsureEventsFireProperly {
   WAIT_FOR(count == 3);
 }
 
-#ifdef FLAKY_TEST
 - (void)testListenForChildChangedWithLimitEnsureEventsFireProperly {
   FTupleFirebase* refs = [FTestHelpers getRandomNodePair];
   FIRDatabaseReference* writer = refs.one;
@@ -3603,7 +3602,6 @@ - (void)testListenForChildChangedWithLimitEnsureEventsFireProperly {
 
   WAIT_FOR(count == 3);
 }
-#endif
 
 - (void)testListenForChildRemovedWithLimitEnsureEventsFireProperly {
   FTupleFirebase* refs = [FTestHelpers getRandomNodePair];