From e63fc2a36e07d7bff01a22796cc34869764cf7ea Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 26 Nov 2025 16:14:45 -0800 Subject: [PATCH 1/5] Fix concurrency crash in FView --- FirebaseDatabase/CHANGELOG.md | 3 ++ FirebaseDatabase/Sources/Core/View/FView.m | 57 +++++++++++++--------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/FirebaseDatabase/CHANGELOG.md b/FirebaseDatabase/CHANGELOG.md index d2a83c45116..2a5e2987318 100644 --- a/FirebaseDatabase/CHANGELOG.md +++ b/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 diff --git a/FirebaseDatabase/Sources/Core/View/FView.m b/FirebaseDatabase/Sources/Core/View/FView.m index 8612def9e93..a15a5440f7b 100644 --- a/FirebaseDatabase/Sources/Core/View/FView.m +++ b/FirebaseDatabase/Sources/Core/View/FView.m @@ -165,11 +165,15 @@ - (id)initWithQuery:(FQuerySpec *)query } - (BOOL)isEmpty { - return self.eventRegistrations.count == 0; + @synchronized(self.eventRegistrations) { + return self.eventRegistrations.count == 0; + } } - (void)addEventRegistration:(id)eventRegistration { - [self.eventRegistrations addObject:eventRegistration]; + @synchronized(self.eventRegistrations) { + [self.eventRegistrations addObject:eventRegistration]; + } } /** @@ -181,31 +185,35 @@ - (void)addEventRegistration:(id)eventRegistration { - (NSArray *)removeEventRegistration:(id)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 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 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 existing = self.eventRegistrations[i]; - if ([existing matches:eventRegistration]) { - [self.eventRegistrations removeObjectAtIndex:i]; - } else { - i++; + if (eventRegistration) { + NSUInteger i = 0; + while (i < self.eventRegistrations.count) { + id existing = self.eventRegistrations[i]; + if ([existing matches:eventRegistration]) { + [self.eventRegistrations removeObjectAtIndex:i]; + } else { + i++; + } } + } else { + [self.eventRegistrations removeAllObjects]; } - } else { - [self.eventRegistrations removeAllObjects]; } return cancelEvents; } @@ -270,7 +278,10 @@ - (NSArray *)generateEventsForChanges:(NSArray *)changes registration:(id)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]; } From b07bbdf90b2a8ac3fb68d63118c29b0734c26b2a Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 26 Nov 2025 16:46:35 -0800 Subject: [PATCH 2/5] review --- FirebaseDatabase/Sources/Core/View/FView.m | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/FirebaseDatabase/Sources/Core/View/FView.m b/FirebaseDatabase/Sources/Core/View/FView.m index a15a5440f7b..55bdf431a95 100644 --- a/FirebaseDatabase/Sources/Core/View/FView.m +++ b/FirebaseDatabase/Sources/Core/View/FView.m @@ -202,15 +202,13 @@ - (NSArray *)removeEventRegistration:(id)eventRegistration } if (eventRegistration) { - NSUInteger i = 0; - while (i < self.eventRegistrations.count) { - id existing = self.eventRegistrations[i]; - if ([existing matches:eventRegistration]) { - [self.eventRegistrations removeObjectAtIndex:i]; - } else { - i++; - } - } + NSIndexSet *indexesToRemove = + [self.eventRegistrations indexesOfObjectsPassingTest:^BOOL( + id existing, + NSUInteger idx, BOOL *stop) { + return [existing matches:eventRegistration]; + }]; + [self.eventRegistrations removeObjectsAtIndexes:indexesToRemove]; } else { [self.eventRegistrations removeAllObjects]; } From 8f83f93096a459f95c77d0025ee20f427e026746 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 26 Nov 2025 17:28:53 -0800 Subject: [PATCH 3/5] enable skipped integration tests --- FirebaseDatabase/Tests/Integration/FData.m | 50 ++----------------- .../Tests/Integration/FIRDatabaseQueryTests.m | 2 - 2 files changed, 3 insertions(+), 49 deletions(-) diff --git a/FirebaseDatabase/Tests/Integration/FData.m b/FirebaseDatabase/Tests/Integration/FData.m index d21a552367b..0baa3646bf4 100644 --- a/FirebaseDatabase/Tests/Integration/FData.m +++ b/FirebaseDatabase/Tests/Integration/FData.m @@ -299,41 +299,6 @@ - (void)testWriteLeafNodeOverwriteAtParentMultipleTimesVerifyExpectedEvents { [et wait]; } -#ifdef FLAKY_TEST -This test flakes frequently on the emulator on travis and almost always on GHA with - - testWriteLeafNodeRemoveLeafVerifyExpectedEvents, - failed - : caught "NSInternalInconsistencyException", - "Unable to report test assertion failure '(([target isEqualTo:recvd]) is true) failed: throwing - "Unable to report test assertion failure '(([target isEqualTo:recvd]) is true) failed - Expected - http : // localhost:9000/-M8IJYWb68MuqQKKz2IY/a aa (0) to match - http : // localhost:9000/-M8IJYWb68MuqQKKz2IY/a (null) (4)' from - / - Users / runner / runners / 2.262.1 / work / firebase - - ios - sdk / firebase - ios - - sdk / Example / Database / Tests / Helpers / - FEventTester - .m - : 123 because it was raised inside test case -[FEventTester( - null)] which has no associated XCTestRun object.This may happen when test cases - are constructed and invoked independently of standard XCTest infrastructure, - or when the test has already finished - ." - Expected http://localhost:9000/-M8IJYWb68MuqQKKz2IY/a aa (0) to match " - "http://localhost:9000/-M8IJYWb68MuqQKKz2IY/a (null) (4)' from " - "/Users/runner/runners/2.262.1/work/firebase-ios-sdk/firebase-ios-sdk/" - "Example/Database/Tests/Helpers/FEventTester.m:123 because it was raised " - "inside test case -[FEventTester (null)] which has no associated XCTestRun " - "object. This may happen when test cases are constructed and invoked " - "independently of standard XCTest infrastructure, or when the test has " - "already finished." / - Users / runner / runners / 2.262.1 / work / firebase - - ios - sdk / firebase - ios - - sdk / Example / Database / Tests / Helpers / FEventTester.m: -123 -``` FTupleEventTypeString *recvd = [self.actualPathsAndEvents objectAtIndex:i]; -XCTAssertTrue([target isEqualTo:recvd], @"Expected %@ to match %@", target, recvd); - - (void)testWriteParentNodeOverwriteAtLeafVerifyExpectedEvents { FIRDatabaseReference *node = [FTestHelpers getRandomNode]; @@ -629,7 +594,6 @@ - (void)testWriteLeafNodeRemoveLeafVerifyExpectedEvents { fabs([writeVal doubleValue] - 3.1415) < 0.001; }]; } -#endif - (void)testWriteMultipleLeafNodesRemoveOnlyOneVerifyExpectedEvents { // XXX impl @@ -954,7 +918,6 @@ - (void)testNumChildrenWorksCorrectly { }]; } -#ifdef FLAKY_TEST - (void)testSettingANodeWithChildrenToAPrimitiveAndBack { // Can't tolerate stale data; so disable persistence. FTupleFirebase *tuple = [FTestHelpers getRandomNodePairWithoutPersistence]; @@ -1045,7 +1008,6 @@ - (void)testSettingANodeWithChildrenToAPrimitiveAndBack { XCTAssertTrue(done, @"Properly finished"); } -#endif - (void)testWriteLeafRemoveLeafAddChildToRemovedNode { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; @@ -2197,9 +2159,7 @@ - (void)testUpdateDoesntAffectPriorityRemotely { }]; } -// TODO: On arm hardware Macs, the following test hangs with the emulator, but passes with a real -// project. -- (void)SKIPtestUpdateReplacesChildrenAndIsNotRecursive { +- (void)testUpdateReplacesChildrenAndIsNotRecursive { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *reader = refs.one; FIRDatabaseReference *writer = refs.two; @@ -2240,8 +2200,7 @@ - (void)SKIPtestUpdateReplacesChildrenAndIsNotRecursive { }]; } -// TODO: https://github.com/firebase/firebase-ios-sdk/issues/15103 -- (void)SKIP_testDeepUpdatesWork { +- (void)testDeepUpdatesWork { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *reader = refs.one; FIRDatabaseReference *writer = refs.two; @@ -2545,7 +2504,6 @@ - (void)testLargeNumbers { XCTAssertTrue([result isEqualToNumber:toSet], @"Should get back same number"); } -#ifdef FLAKY_TEST - (void)testParentDeleteShadowsChildListeners { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *writer = refs.one; @@ -2573,7 +2531,6 @@ - (void)testParentDeleteShadowsChildListeners { WAIT_FOR(done); [deleter removeAllObservers]; } -#endif - (void)testParentDeleteShadowsChildListenersWithNonDefaultQuery { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; @@ -3009,8 +2966,7 @@ - (void)checkServerIncrementPriorityWhileOnline:(BOOL)online { } } -// TODO: https://github.com/firebase/firebase-ios-sdk/issues/15103 -- (void)SKIP_testServerIncrementOverflowAndTypeCoercion { +- (void)testServerIncrementOverflowAndTypeCoercion { FIRDatabaseReference *ref = [FTestHelpers getRandomNode]; __block NSMutableArray *found = [NSMutableArray new]; __block NSMutableArray *foundTypes = [NSMutableArray new]; diff --git a/FirebaseDatabase/Tests/Integration/FIRDatabaseQueryTests.m b/FirebaseDatabase/Tests/Integration/FIRDatabaseQueryTests.m index 4163cd15ae6..91ccdbc0a40 100644 --- a/FirebaseDatabase/Tests/Integration/FIRDatabaseQueryTests.m +++ b/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]; From 8b955275d08ad3d31a0e291262ed049df9bd35b0 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 26 Nov 2025 19:31:48 -0800 Subject: [PATCH 4/5] Re-disable 3 flaky tests --- FirebaseDatabase/Tests/Integration/FData.m | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/FirebaseDatabase/Tests/Integration/FData.m b/FirebaseDatabase/Tests/Integration/FData.m index 0baa3646bf4..28d7c51ff66 100644 --- a/FirebaseDatabase/Tests/Integration/FData.m +++ b/FirebaseDatabase/Tests/Integration/FData.m @@ -464,6 +464,7 @@ - (void)testWriteLeafNodeRemoveParentNodeVerifyExpectedEvents { }]; } +#ifdef FLAKY_TEST - (void)testWriteLeafNodeRemoveLeafVerifyExpectedEvents { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *writer = refs.one; @@ -594,6 +595,7 @@ - (void)testWriteLeafNodeRemoveLeafVerifyExpectedEvents { fabs([writeVal doubleValue] - 3.1415) < 0.001; }]; } +#endif - (void)testWriteMultipleLeafNodesRemoveOnlyOneVerifyExpectedEvents { // XXX impl @@ -918,6 +920,7 @@ - (void)testNumChildrenWorksCorrectly { }]; } +#ifdef FLAKY_TEST - (void)testSettingANodeWithChildrenToAPrimitiveAndBack { // Can't tolerate stale data; so disable persistence. FTupleFirebase *tuple = [FTestHelpers getRandomNodePairWithoutPersistence]; @@ -1008,6 +1011,7 @@ - (void)testSettingANodeWithChildrenToAPrimitiveAndBack { XCTAssertTrue(done, @"Properly finished"); } +#endif - (void)testWriteLeafRemoveLeafAddChildToRemovedNode { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; @@ -2966,7 +2970,8 @@ - (void)checkServerIncrementPriorityWhileOnline:(BOOL)online { } } -- (void)testServerIncrementOverflowAndTypeCoercion { +// TODO: https://github.com/firebase/firebase-ios-sdk/issues/15103 +- (void)SKIPtestServerIncrementOverflowAndTypeCoercion { FIRDatabaseReference *ref = [FTestHelpers getRandomNode]; __block NSMutableArray *found = [NSMutableArray new]; __block NSMutableArray *foundTypes = [NSMutableArray new]; From 3e5c2e15985801fb0fefa203fd986103c48177b1 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 27 Nov 2025 06:14:52 -0800 Subject: [PATCH 5/5] Restore FData.m --- FirebaseDatabase/Tests/Integration/FData.m | 47 ++++++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/FirebaseDatabase/Tests/Integration/FData.m b/FirebaseDatabase/Tests/Integration/FData.m index 28d7c51ff66..d21a552367b 100644 --- a/FirebaseDatabase/Tests/Integration/FData.m +++ b/FirebaseDatabase/Tests/Integration/FData.m @@ -299,6 +299,41 @@ - (void)testWriteLeafNodeOverwriteAtParentMultipleTimesVerifyExpectedEvents { [et wait]; } +#ifdef FLAKY_TEST +This test flakes frequently on the emulator on travis and almost always on GHA with + + testWriteLeafNodeRemoveLeafVerifyExpectedEvents, + failed + : caught "NSInternalInconsistencyException", + "Unable to report test assertion failure '(([target isEqualTo:recvd]) is true) failed: throwing + "Unable to report test assertion failure '(([target isEqualTo:recvd]) is true) failed - Expected + http : // localhost:9000/-M8IJYWb68MuqQKKz2IY/a aa (0) to match + http : // localhost:9000/-M8IJYWb68MuqQKKz2IY/a (null) (4)' from + / + Users / runner / runners / 2.262.1 / work / firebase - + ios - sdk / firebase - ios - + sdk / Example / Database / Tests / Helpers / + FEventTester + .m + : 123 because it was raised inside test case -[FEventTester( + null)] which has no associated XCTestRun object.This may happen when test cases + are constructed and invoked independently of standard XCTest infrastructure, + or when the test has already finished + ." - Expected http://localhost:9000/-M8IJYWb68MuqQKKz2IY/a aa (0) to match " + "http://localhost:9000/-M8IJYWb68MuqQKKz2IY/a (null) (4)' from " + "/Users/runner/runners/2.262.1/work/firebase-ios-sdk/firebase-ios-sdk/" + "Example/Database/Tests/Helpers/FEventTester.m:123 because it was raised " + "inside test case -[FEventTester (null)] which has no associated XCTestRun " + "object. This may happen when test cases are constructed and invoked " + "independently of standard XCTest infrastructure, or when the test has " + "already finished." / + Users / runner / runners / 2.262.1 / work / firebase - + ios - sdk / firebase - ios - + sdk / Example / Database / Tests / Helpers / FEventTester.m: +123 +``` FTupleEventTypeString *recvd = [self.actualPathsAndEvents objectAtIndex:i]; +XCTAssertTrue([target isEqualTo:recvd], @"Expected %@ to match %@", target, recvd); + - (void)testWriteParentNodeOverwriteAtLeafVerifyExpectedEvents { FIRDatabaseReference *node = [FTestHelpers getRandomNode]; @@ -464,7 +499,6 @@ - (void)testWriteLeafNodeRemoveParentNodeVerifyExpectedEvents { }]; } -#ifdef FLAKY_TEST - (void)testWriteLeafNodeRemoveLeafVerifyExpectedEvents { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *writer = refs.one; @@ -2163,7 +2197,9 @@ - (void)testUpdateDoesntAffectPriorityRemotely { }]; } -- (void)testUpdateReplacesChildrenAndIsNotRecursive { +// TODO: On arm hardware Macs, the following test hangs with the emulator, but passes with a real +// project. +- (void)SKIPtestUpdateReplacesChildrenAndIsNotRecursive { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *reader = refs.one; FIRDatabaseReference *writer = refs.two; @@ -2204,7 +2240,8 @@ - (void)testUpdateReplacesChildrenAndIsNotRecursive { }]; } -- (void)testDeepUpdatesWork { +// TODO: https://github.com/firebase/firebase-ios-sdk/issues/15103 +- (void)SKIP_testDeepUpdatesWork { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *reader = refs.one; FIRDatabaseReference *writer = refs.two; @@ -2508,6 +2545,7 @@ - (void)testLargeNumbers { XCTAssertTrue([result isEqualToNumber:toSet], @"Should get back same number"); } +#ifdef FLAKY_TEST - (void)testParentDeleteShadowsChildListeners { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; FIRDatabaseReference *writer = refs.one; @@ -2535,6 +2573,7 @@ - (void)testParentDeleteShadowsChildListeners { WAIT_FOR(done); [deleter removeAllObservers]; } +#endif - (void)testParentDeleteShadowsChildListenersWithNonDefaultQuery { FTupleFirebase *refs = [FTestHelpers getRandomNodePair]; @@ -2971,7 +3010,7 @@ - (void)checkServerIncrementPriorityWhileOnline:(BOOL)online { } // TODO: https://github.com/firebase/firebase-ios-sdk/issues/15103 -- (void)SKIPtestServerIncrementOverflowAndTypeCoercion { +- (void)SKIP_testServerIncrementOverflowAndTypeCoercion { FIRDatabaseReference *ref = [FTestHelpers getRandomNode]; __block NSMutableArray *found = [NSMutableArray new]; __block NSMutableArray *foundTypes = [NSMutableArray new];