diff --git a/Crashlytics/CHANGELOG.md b/Crashlytics/CHANGELOG.md index f65bacfc228..907e2cfe92c 100644 --- a/Crashlytics/CHANGELOG.md +++ b/Crashlytics/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased - [added] Added support for catching the SIGTERM signal (#12881). +- [fixed] Fixed a hang when persisting Remote Config Rollouts to disk (#12913). # 10.25.0 - [changed] Removed usages of user defaults API from internal Firebase Sessions diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m b/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m index defd9d68a52..57993a965f3 100644 --- a/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m +++ b/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m @@ -57,12 +57,13 @@ - (void)updateRolloutsStateToPersistenceWithRollouts:(NSData *_Nonnull)rollouts NSFileHandle *rolloutsFile = [NSFileHandle fileHandleForUpdatingAtPath:rolloutsPath]; - dispatch_sync(FIRCLSGetLoggingQueue(), ^{ + dispatch_async(FIRCLSGetLoggingQueue(), ^{ @try { [rolloutsFile seekToEndOfFile]; NSMutableData *rolloutsWithNewLineData = [rollouts mutableCopy]; [rolloutsWithNewLineData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]]; [rolloutsFile writeData:rolloutsWithNewLineData]; + [rolloutsFile closeFile]; } @catch (NSException *exception) { FIRCLSDebugLog(@"Failed to write new rollouts. Exception name: %s - message: %s", exception.name, exception.reason); diff --git a/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m b/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m index aec030d7538..4caceb242b1 100644 --- a/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m +++ b/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m @@ -16,6 +16,7 @@ #import #import "Crashlytics/Crashlytics/Components/FIRCLSContext.h" +#include "Crashlytics/Crashlytics/Components/FIRCLSGlobals.h" #import "Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.h" #import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h" #import "Crashlytics/UnitTests/Mocks/FIRCLSTempMockFileManager.h" @@ -51,6 +52,9 @@ - (void)tearDown { } - (void)testUpdateRolloutsStateToPersistenceWithRollouts { + XCTestExpectation *expectation = [[XCTestExpectation alloc] + initWithDescription:@"Expect updating rollouts to finish writing."]; + NSString *encodedStateString = @"{rollouts:[{\"parameter_key\":\"6d795f66656174757265\",\"parameter_value\":" @"\"e8bf99e698af7468656d6973e79a84e6b58be8af95e695b0e68daeefbc8ce8be93e585a5e4b8ade69687\"," @@ -64,7 +68,43 @@ - (void)testUpdateRolloutsStateToPersistenceWithRollouts { [self.rolloutsPersistenceManager updateRolloutsStateToPersistenceWithRollouts:data reportID:reportId]; + + // Wait for the logging queue to finish. + dispatch_async(FIRCLSGetLoggingQueue(), ^{ + [expectation fulfill]; + }); + + [self waitForExpectations:@[ expectation ] timeout:3]; + XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:rolloutsFilePath]); } +- (void)testUpdateRolloutsStateToPersistenceEnsureNoHang { + dispatch_queue_t testQueue = dispatch_queue_create("TestQueue", DISPATCH_QUEUE_SERIAL); + XCTestExpectation *expectation = + [[XCTestExpectation alloc] initWithDescription:@"Expect updating rollouts to return."]; + NSString *encodedStateString = + @"{rollouts:[{\"parameter_key\":\"6d795f66656174757265\",\"parameter_value\":" + @"\"e8bf99e698af7468656d6973e79a84e6b58be8af95e695b0e68daeefbc8ce8be93e585a5e4b8ade69687\"," + @"\"rollout_id\":\"726f6c6c6f75745f31\",\"template_version\":1,\"variant_id\":" + @"\"636f6e74726f6c\"}]}"; + + NSData *data = [encodedStateString dataUsingEncoding:NSUTF8StringEncoding]; + + // Clog up the queue with a long running operation. This sleep time + // must be longer than the expectation timeout. + dispatch_async(FIRCLSGetLoggingQueue(), ^{ + sleep(10); + }); + + dispatch_async(testQueue, ^{ + // Ensure that calling this returns quickly so we don't hang + [self.rolloutsPersistenceManager updateRolloutsStateToPersistenceWithRollouts:data + reportID:reportId]; + [expectation fulfill]; + }); + + [self waitForExpectations:@[ expectation ] timeout:3]; +} + @end