diff --git a/FirebaseDatabase/CHANGELOG.md b/FirebaseDatabase/CHANGELOG.md index 0d4d26a525c..94db9290a6b 100644 --- a/FirebaseDatabase/CHANGELOG.md +++ b/FirebaseDatabase/CHANGELOG.md @@ -1,4 +1,6 @@ # 13.1.0 +- [fixed] Fixed a race where calling childByAutoId() concurrently from multiple + threads could generate duplicate push IDs. (#16768) - [fixed] Fixed an issue where transactions could fail with a maxretry error when every child of a node had a priority. (#16769) - [fixed] Fixed an issue where observers could miss events, and keep showing stale diff --git a/FirebaseDatabase/Sources/Utilities/FNextPushId.m b/FirebaseDatabase/Sources/Utilities/FNextPushId.m index bfe890fa369..643476527ea 100644 --- a/FirebaseDatabase/Sources/Utilities/FNextPushId.m +++ b/FirebaseDatabase/Sources/Utilities/FNextPushId.m @@ -37,38 +37,42 @@ @implementation FNextPushId + (NSString *)get:(NSTimeInterval)currentTime { static long long lastPushTime = 0; static int lastRandChars[12]; + @synchronized([FNextPushId class]) { - long long now = (long long)(currentTime * 1000); + long long now = (long long)(currentTime * 1000); - BOOL duplicateTime = now == lastPushTime; - lastPushTime = now; + BOOL duplicateTime = now == lastPushTime; + lastPushTime = now; - unichar timeStampChars[8]; - for (int i = 7; i >= 0; i--) { - timeStampChars[i] = [PUSH_CHARS characterAtIndex:(now % 64)]; - now = (long long)floor(now / 64); - } + unichar timeStampChars[8]; + for (int i = 7; i >= 0; i--) { + timeStampChars[i] = [PUSH_CHARS characterAtIndex:(now % 64)]; + now = (long long)floor(now / 64); + } - NSMutableString *id = [[NSMutableString alloc] init]; - [id appendString:[NSString stringWithCharacters:timeStampChars length:8]]; + NSMutableString *id = [[NSMutableString alloc] init]; + [id appendString:[NSString stringWithCharacters:timeStampChars + length:8]]; - if (!duplicateTime) { - for (int i = 0; i < 12; i++) { - lastRandChars[i] = (int)floor(arc4random() % 64); + if (!duplicateTime) { + for (int i = 0; i < 12; i++) { + lastRandChars[i] = (int)floor(arc4random() % 64); + } + } else { + int i = 0; + for (i = 11; i >= 0 && lastRandChars[i] == 63; i--) { + lastRandChars[i] = 0; + } + lastRandChars[i]++; } - } else { - int i = 0; - for (i = 11; i >= 0 && lastRandChars[i] == 63; i--) { - lastRandChars[i] = 0; + + for (int i = 0; i < 12; i++) { + [id appendFormat:@"%C", + [PUSH_CHARS characterAtIndex:lastRandChars[i]]]; } - lastRandChars[i]++; - } - for (int i = 0; i < 12; i++) { - [id appendFormat:@"%C", [PUSH_CHARS characterAtIndex:lastRandChars[i]]]; + return [NSString stringWithString:id]; } - - return [NSString stringWithString:id]; } + (NSString *)from:(NSString *)fn successor:(NSString *_Nonnull)key { diff --git a/FirebaseDatabase/Tests/Unit/FNextPushIdTest.m b/FirebaseDatabase/Tests/Unit/FNextPushIdTest.m index 70267651fe3..62cff3f109f 100644 --- a/FirebaseDatabase/Tests/Unit/FNextPushIdTest.m +++ b/FirebaseDatabase/Tests/Unit/FNextPushIdTest.m @@ -49,6 +49,33 @@ - (void)testSuccessorSpecialValues { @"successor(MAX_PUSH_CHAR repeated MAX_KEY_LEN times) == MAX_NAME"); } +- (void)testConcurrentPushIdsAreUniqueForTheSameTimestamp { + const size_t pushIdCount = 1000; + NSMutableArray *pushIds = [NSMutableArray arrayWithCapacity:pushIdCount]; + NSLock *pushIdsLock = [[NSLock alloc] init]; + + dispatch_apply(pushIdCount, dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^(size_t index) { + (void)index; + NSString *pushId = [FNextPushId get:12345.0]; + [pushIdsLock lock]; + [pushIds addObject:pushId]; + [pushIdsLock unlock]; + }); + + XCTAssertEqual([NSSet setWithArray:pushIds].count, pushIdCount); +} + +- (void)testGenerationExceptionDoesNotBlockLaterCalls { + XCTAssertThrowsSpecific([FNextPushId get:-0.001], NSException); + XCTestExpectation *generated = [self expectationWithDescription:@"Generate after exception"]; + dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ + NSString *pushId = [FNextPushId get:12345.0]; + XCTAssertEqual(pushId.length, 20U); + [generated fulfill]; + }); + [self waitForExpectationsWithTimeout:2.0 handler:nil]; +} + - (void)testSuccessorBasic { NSString *actual = [FNextPushId from:@"test" successor:@"abc"]; NSString *expected = [NSString stringWithFormat:@"abc%@", MIN_PUSH_CHAR];