Merge c07668d into 9143c4b

Crashlytics/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fixed a threading-related hang during initialization in urgent mode (#11216)
+
 # 10.10.0
 - [changed] Removed references to deprecated CTCarrier API in FirebaseSessions. (#11144)
 - [fixed] Fix Xcode 14.3 Analyzer issue. (#11228)

Crashlytics/Crashlytics/Controllers/FIRCLSReportUploader.m

@@ -89,9 +89,21 @@ - (void)prepareAndSubmitReport:(FIRCLSInternalReport *)report
       FIRCLSApplicationActivityDefault, @"Crashlytics Crash Report Processing", ^{
         // Check to see if the FID has rotated before we construct the payload
         // so that the payload has an updated value.
-        [self.installIDModel regenerateInstallIDIfNeededWithBlock:^(NSString *_Nonnull newFIID) {
-          self.fiid = [newFIID copy];
-        }];
+        //
+        // If we're in urgent mode, this will be running on the main thread. Since
+        // the FIID callback is run on the main thread, this call can deadlock in
+        // urgent mode. Since urgent mode happens when the app is in a crash loop,
+        // we can safely assume users aren't rotating their FIID, so this can be skipped.
+        if (!urgent) {
+          [self.installIDModel regenerateInstallIDIfNeededWithBlock:^(NSString *_Nonnull newFIID) {
+            self.fiid = [newFIID copy];
+          }];
+        } else {
+          FIRCLSWarningLog(
+              @"Crashlytics skipped rotating the Install ID during urgent mode because it is run "
+              @"on the main thread, which can't succeed. This can happen if the app crashed the "
+              @"last run and Crashlytics is uploading urgently.");
+        }
 
         // Run on-device symbolication before packaging if we should process
         if (shouldProcess) {

Crashlytics/UnitTests/Data/GoogleService-Info.plist

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>API_KEY</key>
+	<string>ATestTestTestTestTestTestTestTestTest00</string>
+	<key>PROJECT_ID</key>
+	<string>test-1aaaa</string>
+	<key>GOOGLE_APP_ID</key>
+	<string>1:632950151350:ios:d5b0d08d4f00f4b1</string>
+</dict>
+</plist>

Crashlytics/UnitTests/FIRCLSReportUploaderTests.m

@@ -74,11 +74,12 @@ - (void)setUp {
 
 - (void)tearDown {
   self.uploader = nil;
+  [FIRApp resetApps];
 
   [super tearDown];
 }
 
-- (void)setupUploaderWithInstallations:(FIRMockInstallations *)installations {
+- (void)setupUploaderWithInstallations:(FIRInstallations *)installations {
   // Allow nil values only in tests
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wnonnull"
@@ -95,17 +96,33 @@ - (void)setupUploaderWithInstallations:(FIRMockInstallations *)installations {
   self.uploader = [[FIRCLSReportUploader alloc] initWithManagerData:self.managerData];
 }
 
+- (NSString *)resourcePath {
+#if SWIFT_PACKAGE
+  NSBundle *bundle = SWIFTPM_MODULE_BUNDLE;
+  return [bundle.resourcePath stringByAppendingPathComponent:@"Data"];
+#else
+  NSBundle *bundle = [NSBundle bundleForClass:[self class]];
+  return bundle.resourcePath;
+#endif
+}
+
+- (FIRCLSInternalReport *)createReport {
+  NSString *path = [self.fileManager.activePath stringByAppendingPathComponent:@"pkg_uuid"];
+  FIRCLSInternalReport *report = [[FIRCLSInternalReport alloc] initWithPath:path];
+  self.fileManager.moveItemAtPathResult = [NSNumber numberWithInt:1];
+  return report;
+}
+
 #pragma mark - Tests
 
 - (void)testPrepareReport {
-  FIRCLSInternalReport *report = [[FIRCLSInternalReport alloc] initWithPath:[self packagePath]];
-  self.fileManager.moveItemAtPathResult = [NSNumber numberWithInt:1];
+  FIRCLSInternalReport *report = [self createReport];
 
   XCTAssertNil(self.uploader.fiid);
 
   [self.uploader prepareAndSubmitReport:report
                     dataCollectionToken:FIRCLSDataCollectionToken.validToken
-                               asUrgent:YES
+                               asUrgent:NO
                          withProcessing:YES];
 
   XCTAssertEqual(self.uploader.fiid, TestFIID);
@@ -115,9 +132,64 @@ - (void)testPrepareReport {
       [self.fileManager.moveItemAtPath_destDir containsString:self.fileManager.preparedPath]);
 }
 
+- (void)testPrepareReportOnMainThread {
+  NSString *pathToPlist =
+      [[self resourcePath] stringByAppendingPathComponent:@"GoogleService-Info.plist"];
+  FIROptions *options = [[FIROptions alloc] initWithContentsOfFile:pathToPlist];
+
+  [FIRApp configureWithName:@"__FIRAPP_DEFAULT" options:options];
+  XCTAssertNotNil([FIRApp defaultApp], @"configureWithName must have been initialized");
+
+  FIRInstallations *installations = [FIRInstallations installationsWithApp:[FIRApp defaultApp]];
+  FIRCLSInternalReport *report = [self createReport];
+  [self setupUploaderWithInstallations:installations];
+
+  /*
+   if a report is urgent report will be processed on the Main Thread
+   otherwise, it will be dispatched to a NSOperationQueue (see `FIRCLSExistingReportManager.m:230`)
+
+   This test checks if `prepareAndSubmitReport` finishes in a reasonable time.
+   */
+
+  NSOperationQueue *queue = [NSOperationQueue new];
+
+  // target call will block the main thread, so we need a background thread
+  // that will wait on a semaphore for a timeout
+  dispatch_semaphore_t backgroundWaiter = dispatch_semaphore_create(0);
+  [queue addOperationWithBlock:^{
+    intptr_t result = dispatch_semaphore_wait(backgroundWaiter,
+                                              dispatch_time(DISPATCH_TIME_NOW, 1 * NSEC_PER_SEC));
+    BOOL exitBecauseOfTimeout = result != 0;
+    XCTAssertFalse(exitBecauseOfTimeout, @"Main Thread was blocked for more than 1 second");
+  }];
+
+  // Urgent (on the Main thread)
+  [self.uploader prepareAndSubmitReport:report
+                    dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                               asUrgent:YES
+                         withProcessing:YES];
+  dispatch_semaphore_signal(backgroundWaiter);
+
+  // Not urgent (on a background thread)
+  XCTestExpectation *expectation =
+      [self expectationWithDescription:@"wait for a preparation to complete"];
+
+  [queue addOperationWithBlock:^{
+    [self.uploader prepareAndSubmitReport:report
+                      dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                                 asUrgent:YES
+                           withProcessing:YES];
+    [expectation fulfill];
+  }];
+
+  [self waitForExpectationsWithTimeout:1
+                               handler:^(NSError *_Nullable error) {
+                                 XCTAssertNil(error, @"expectations failed: %@", error);
+                               }];
+}
+
 - (void)test_NilFIID_DoesNotCrash {
-  FIRCLSInternalReport *report = [[FIRCLSInternalReport alloc] initWithPath:[self packagePath]];
-  self.fileManager.moveItemAtPathResult = [NSNumber numberWithInt:1];
+  FIRCLSInternalReport *report = [self createReport];
 
   self.mockInstallations = [[FIRMockInstallations alloc]
       initWithError:[NSError errorWithDomain:@"TestDomain" code:-1 userInfo:nil]];