Make sure rollouts logging queue is not nil (#13058)

firebase · Jun 4, 2024 · 309707d · 309707d
1 parent 02d963c
commit 309707d
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 10 deletions.
diff --git a/Crashlytics/CHANGELOG.md b/Crashlytics/CHANGELOG.md
@@ -1,3 +1,6 @@
+# # Unreleased
+- [fixed] Created a new queue for rollouts persistence writes and made sure rollouts logging queue is not nil while dispatching (#12913).
+
 # 10.27.0
 - [added] Added support for catching the SIGTERM signal (#12881).
 - [fixed] Fixed a hang when persisting Remote Config Rollouts to disk (#12913).

diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.h b/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.h
@@ -25,7 +25,8 @@
 
 @interface FIRCLSRolloutsPersistenceManager : NSObject <FIRCLSPersistenceLog>
 
-- (instancetype _Nullable)initWithFileManager:(FIRCLSFileManager *_Nonnull)fileManager;
+- (instancetype _Nullable)initWithFileManager:(FIRCLSFileManager *_Nonnull)fileManager
+                                     andQueue:(dispatch_queue_t _Nonnull)queue;
 - (instancetype _Nonnull)init NS_UNAVAILABLE;
 + (instancetype _Nonnull)new NS_UNAVAILABLE;
 

diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m b/Crashlytics/Crashlytics/Controllers/FIRCLSRolloutsPersistenceManager.m
@@ -13,7 +13,6 @@
 // limitations under the License.
 
 #import <Foundation/Foundation.h>
-#include "Crashlytics/Crashlytics/Components/FIRCLSGlobals.h"
 #include "Crashlytics/Crashlytics/Components/FIRCLSUserLogging.h"
 #import "Crashlytics/Crashlytics/Helpers/FIRCLSLogger.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSFileManager.h"
@@ -32,15 +31,23 @@
 
 @interface FIRCLSRolloutsPersistenceManager : NSObject <FIRCLSPersistenceLog>
 @property(nonatomic, readonly) FIRCLSFileManager *fileManager;
+@property(nonnull, nonatomic, readonly) dispatch_queue_t rolloutsLoggingQueue;
 @end
 
 @implementation FIRCLSRolloutsPersistenceManager
-- (instancetype)initWithFileManager:(FIRCLSFileManager *)fileManager {
+- (instancetype)initWithFileManager:(FIRCLSFileManager *)fileManager
+                           andQueue:(dispatch_queue_t)queue {
   self = [super init];
   if (!self) {
     return nil;
   }
   _fileManager = fileManager;
+
+  if (!queue) {
+    FIRCLSDebugLog(@"Failed to intialize FIRCLSRolloutsPersistenceManager, logging queue is nil");
+    return nil;
+  }
+  _rolloutsLoggingQueue = queue;
   return self;
 }
 
@@ -57,7 +64,11 @@ - (void)updateRolloutsStateToPersistenceWithRollouts:(NSData *_Nonnull)rollouts
 
   NSFileHandle *rolloutsFile = [NSFileHandle fileHandleForUpdatingAtPath:rolloutsPath];
 
-  dispatch_async(FIRCLSGetLoggingQueue(), ^{
+  if (!_rolloutsLoggingQueue) {
+    FIRCLSDebugLog(@"Rollouts logging queue is dealloccated");
+  }
+
+  dispatch_async(_rolloutsLoggingQueue, ^{
     @try {
       [rolloutsFile seekToEndOfFile];
       NSMutableData *rolloutsWithNewLineData = [rollouts mutableCopy];

diff --git a/Crashlytics/Crashlytics/FIRCrashlytics.m b/Crashlytics/Crashlytics/FIRCrashlytics.m
@@ -211,7 +211,11 @@ - (instancetype)initWithApp:(FIRApp *)app
       FIRCLSDebugLog(@"Registering RemoteConfig SDK subscription for rollouts data");
 
       FIRCLSRolloutsPersistenceManager *persistenceManager =
-          [[FIRCLSRolloutsPersistenceManager alloc] initWithFileManager:_fileManager];
+          [[FIRCLSRolloutsPersistenceManager alloc]
+              initWithFileManager:_fileManager
+                         andQueue:dispatch_queue_create(
+                                      "com.google.firebase.FIRCLSRolloutsPersistence",
+                                      DISPATCH_QUEUE_SERIAL)];
       _remoteConfigManager =
           [[FIRCLSRemoteConfigManager alloc] initWithRemoteConfig:remoteConfig
                                               persistenceDelegate:persistenceManager];

diff --git a/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m b/Crashlytics/UnitTests/FIRCLSRolloutsPersistenceManagerTests.m
@@ -30,6 +30,7 @@
 
 @interface FIRCLSRolloutsPersistenceManagerTests : XCTestCase
 @property(nonatomic, strong) FIRCLSTempMockFileManager *fileManager;
+@property(nonatomic, strong) dispatch_queue_t loggingQueue;
 @property(nonatomic, strong) FIRCLSRolloutsPersistenceManager *rolloutsPersistenceManager;
 @end
 
@@ -41,8 +42,11 @@ - (void)setUp {
   [self.fileManager createReportDirectories];
   [self.fileManager setupNewPathForExecutionIdentifier:reportId];
 
+  self.loggingQueue =
+      dispatch_queue_create("com.google.firebase.FIRCLSRolloutsPersistence", DISPATCH_QUEUE_SERIAL);
   self.rolloutsPersistenceManager =
-      [[FIRCLSRolloutsPersistenceManager alloc] initWithFileManager:self.fileManager];
+      [[FIRCLSRolloutsPersistenceManager alloc] initWithFileManager:self.fileManager
+                                                           andQueue:self.loggingQueue];
 }
 
 - (void)tearDown {
@@ -65,18 +69,22 @@ - (void)testUpdateRolloutsStateToPersistenceWithRollouts {
   NSString *rolloutsFilePath =
       [[[self.fileManager activePath] stringByAppendingPathComponent:reportId]
           stringByAppendingPathComponent:FIRCLSReportRolloutsFile];
-
   [self.rolloutsPersistenceManager updateRolloutsStateToPersistenceWithRollouts:data
                                                                        reportID:reportId];
-
+  XCTAssertNotNil(self.loggingQueue);
   // Wait for the logging queue to finish.
-  dispatch_async(FIRCLSGetLoggingQueue(), ^{
+  dispatch_async(self.loggingQueue, ^{
     [expectation fulfill];
   });
 
   [self waitForExpectations:@[ expectation ] timeout:3];
 
   XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:rolloutsFilePath]);
+
+  NSFileHandle *rolloutsFile = [NSFileHandle fileHandleForUpdatingAtPath:rolloutsFilePath];
+  NSData *fileData = [rolloutsFile readDataToEndOfFile];
+  NSString *fileString = [[NSString alloc] initWithData:fileData encoding:NSUTF8StringEncoding];
+  XCTAssertTrue([fileString isEqualToString:[encodedStateString stringByAppendingString:@"\n"]]);
 }
 
 - (void)testUpdateRolloutsStateToPersistenceEnsureNoHang {
@@ -93,7 +101,7 @@ - (void)testUpdateRolloutsStateToPersistenceEnsureNoHang {
 
   // Clog up the queue with a long running operation. This sleep time
   // must be longer than the expectation timeout.
-  dispatch_async(FIRCLSGetLoggingQueue(), ^{
+  dispatch_async(self.loggingQueue, ^{
     sleep(10);
   });