From 8822c483646f2ce8445af63a9451dcc2aab12e17 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 11:41:56 -0800 Subject: [PATCH 1/8] Fix GDTCCTUploader-upload Crash --- CHANGELOG.md | 4 ++ .../GDTCCTLibrary/GDTCCTUploadOperation.m | 5 +- .../GDTCCTTests/Unit/GDTCCTUploaderTest.m | 48 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b25a4b..75fa08d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v10.1.1 +- Fix GDTCCTUploader-upload Crash. + (#156, [firebase-ios-sdk/#15129](https://github.com/firebase/firebase-ios-sdk/issues/15129)) + # v10.1.0 - Fix `[FBLPromise HTTPBody]` SwiftUI Previews crash when using binary distribution. ([firebase-ios-sdk/#13318](https://github.com/firebase/firebase-ios-sdk/issues/13318), diff --git a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m index 354d14c..40c9ed7 100644 --- a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m +++ b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m @@ -135,9 +135,8 @@ - (void)uploadTarget:(GDTCORTarget)target withConditions:(GDTCORUploadConditions beginBackgroundTaskWithName:@"GDTCCTUploader-upload" expirationHandler:^{ if (backgroundTaskID != GDTCORBackgroundIdentifierInvalid) { - // Cancel the upload and complete delivery. - [self.currentTask cancel]; - + // Finish the operation. + [self finishOperation]; // End the background task. backgroundTaskCompletion(); } else { diff --git a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m index e4d2a58..21552dc 100644 --- a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m +++ b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m @@ -15,6 +15,7 @@ */ #import +#import #import "FBLPromise+Testing.h" @@ -775,6 +776,53 @@ - (void)testUploadTarget_WhenBatchWithMetricsAreUploaded { [self waitForUploadOperationsToFinish:self.uploader]; } +- (void)testBackgroundTaskExpirationFinishesOperation { + // 1. Swizzle the background task method. + Method originalMethod = class_getInstanceMethod( + [GDTCORApplication class], @selector(beginBackgroundTaskWithName:expirationHandler:)); + Method swizzledMethod = class_getInstanceMethod( + [self class], @selector(gdt_test_beginBackgroundTaskWithName:expirationHandler:)); + method_exchangeImplementations(originalMethod, swizzledMethod); + + // 2. Generate a test event. + [self.generator generateEvent:GDTCOREventQoSFast]; + + // 3. Set up expectations. + [self setUpStorageExpectations]; + self.testStorage.removeBatchAndDeleteEventsExpectation.inverted = YES; + + // 4. Expect hasEventsForTarget:onComplete: to be called. + XCTestExpectation *hasEventsExpectation = + [self expectStorageHasEventsForTarget:self.generator.target result:YES]; + + // 5. Start the upload. + [self.uploader uploadTarget:self.generator.target withConditions:GDTCORUploadConditionWifiData]; + + // 6. Wait for the upload to finish. + [self waitForUploadOperationsToFinish:self.uploader]; + + // 7. Wait for all expectations to be fulfilled. + [self waitForExpectations:@[ + hasEventsExpectation, + self.testStorage.batchWithEventSelectorExpectation, + self.testStorage.removeBatchWithoutDeletingEventsExpectation, + ] + timeout:1]; + + // 8. Unswizzle the background task method. + method_exchangeImplementations(swizzledMethod, originalMethod); +} + +#pragma mark - Swizzled Methods + +- (GDTCORBackgroundIdentifier)gdt_test_beginBackgroundTaskWithName:(NSString *)name + expirationHandler:(void (^)(void))handler { + if (handler) { + handler(); + } + return 1234; +} + #pragma mark - Storage interaction tests - (void)testStorageSelectorWhenConditionsHighPriority { From 532aa532d34fdaae732503f4e92c13588076a2a1 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 12:08:25 -0800 Subject: [PATCH 2/8] fix --- .../GDTCCTLibrary/GDTCCTUploadOperation.m | 6 +- .../GDTCCTTests/Unit/GDTCCTUploaderTest.m | 59 ++++++++++++++++--- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m index 40c9ed7..da0595a 100644 --- a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m +++ b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m @@ -135,10 +135,8 @@ - (void)uploadTarget:(GDTCORTarget)target withConditions:(GDTCORUploadConditions beginBackgroundTaskWithName:@"GDTCCTUploader-upload" expirationHandler:^{ if (backgroundTaskID != GDTCORBackgroundIdentifierInvalid) { - // Finish the operation. - [self finishOperation]; - // End the background task. - backgroundTaskCompletion(); + // Cancel the upload and complete delivery. + [self cancel]; } else { GDTCORLog(GDTCORMCDDebugLog, GDTCORLoggingLevelWarnings, @"Attempted to cancel invalid background task in " diff --git a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m index 21552dc..1718c1d 100644 --- a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m +++ b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m @@ -777,40 +777,81 @@ - (void)testUploadTarget_WhenBatchWithMetricsAreUploaded { } - (void)testBackgroundTaskExpirationFinishesOperation { + // 1. Swizzle the background task method. + Method originalMethod = class_getInstanceMethod( + [GDTCORApplication class], @selector(beginBackgroundTaskWithName:expirationHandler:)); - Method swizzledMethod = class_getInstanceMethod( - [self class], @selector(gdt_test_beginBackgroundTaskWithName:expirationHandler:)); + + Method swizzledMethod = + + class_getInstanceMethod([self class], @selector(gdt_test_beginBackgroundTaskWithName:expirationHandler:)); + method_exchangeImplementations(originalMethod, swizzledMethod); + + // 2. Generate a test event. + [self.generator generateEvent:GDTCOREventQoSFast]; - // 3. Set up expectations. + + + // 3. Set up inverted expectations. + [self setUpStorageExpectations]; + + self.testStorage.removeBatchWithoutDeletingEventsExpectation.inverted = YES; + + self.testStorage.batchWithEventSelectorExpectation.inverted = YES; + self.testStorage.removeBatchAndDeleteEventsExpectation.inverted = YES; - // 4. Expect hasEventsForTarget:onComplete: to be called. + + XCTestExpectation *hasEventsExpectation = + [self expectStorageHasEventsForTarget:self.generator.target result:YES]; - // 5. Start the upload. + hasEventsExpectation.inverted = YES; + + + + // 4. Start the upload. + [self.uploader uploadTarget:self.generator.target withConditions:GDTCORUploadConditionWifiData]; - // 6. Wait for the upload to finish. + + + // 5. Wait for the upload to finish. + [self waitForUploadOperationsToFinish:self.uploader]; - // 7. Wait for all expectations to be fulfilled. + + + // 6. Wait for all expectations to be fulfilled. + [self waitForExpectations:@[ + hasEventsExpectation, + self.testStorage.batchWithEventSelectorExpectation, + self.testStorage.removeBatchWithoutDeletingEventsExpectation, + + self.testStorage.removeBatchAndDeleteEventsExpectation, + ] - timeout:1]; - // 8. Unswizzle the background task method. + timeout:0.5]; + + + + // 7. Unswizzle the background task method. + method_exchangeImplementations(swizzledMethod, originalMethod); + } #pragma mark - Swizzled Methods From 793ebe6a63a820a045afc3148585d3a226f57423 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 12:13:53 -0800 Subject: [PATCH 3/8] style --- .../GDTCCTTests/Unit/GDTCCTUploaderTest.m | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m index 1718c1d..98204d1 100644 --- a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m +++ b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m @@ -777,7 +777,6 @@ - (void)testUploadTarget_WhenBatchWithMetricsAreUploaded { } - (void)testBackgroundTaskExpirationFinishesOperation { - // 1. Swizzle the background task method. Method originalMethod = class_getInstanceMethod( @@ -786,18 +785,15 @@ - (void)testBackgroundTaskExpirationFinishesOperation { Method swizzledMethod = - class_getInstanceMethod([self class], @selector(gdt_test_beginBackgroundTaskWithName:expirationHandler:)); + class_getInstanceMethod([self class], @selector(gdt_test_beginBackgroundTaskWithName: + expirationHandler:)); method_exchangeImplementations(originalMethod, swizzledMethod); - - // 2. Generate a test event. [self.generator generateEvent:GDTCOREventQoSFast]; - - // 3. Set up inverted expectations. [self setUpStorageExpectations]; @@ -808,28 +804,20 @@ - (void)testBackgroundTaskExpirationFinishesOperation { self.testStorage.removeBatchAndDeleteEventsExpectation.inverted = YES; - - XCTestExpectation *hasEventsExpectation = [self expectStorageHasEventsForTarget:self.generator.target result:YES]; hasEventsExpectation.inverted = YES; - - // 4. Start the upload. [self.uploader uploadTarget:self.generator.target withConditions:GDTCORUploadConditionWifiData]; - - // 5. Wait for the upload to finish. [self waitForUploadOperationsToFinish:self.uploader]; - - // 6. Wait for all expectations to be fulfilled. [self waitForExpectations:@[ @@ -846,12 +834,9 @@ - (void)testBackgroundTaskExpirationFinishesOperation { timeout:0.5]; - - // 7. Unswizzle the background task method. method_exchangeImplementations(swizzledMethod, originalMethod); - } #pragma mark - Swizzled Methods From 26af9abcbfbdd0047e7dd361469f7c5c72beb62f Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 17:31:15 -0800 Subject: [PATCH 4/8] Gemini failed to create a working test --- .../GDTCCTTests/Unit/GDTCCTUploaderTest.m | 74 ------------------- 1 file changed, 74 deletions(-) diff --git a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m index 98204d1..e4d2a58 100644 --- a/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m +++ b/GoogleDataTransport/GDTCCTTests/Unit/GDTCCTUploaderTest.m @@ -15,7 +15,6 @@ */ #import -#import #import "FBLPromise+Testing.h" @@ -776,79 +775,6 @@ - (void)testUploadTarget_WhenBatchWithMetricsAreUploaded { [self waitForUploadOperationsToFinish:self.uploader]; } -- (void)testBackgroundTaskExpirationFinishesOperation { - // 1. Swizzle the background task method. - - Method originalMethod = class_getInstanceMethod( - - [GDTCORApplication class], @selector(beginBackgroundTaskWithName:expirationHandler:)); - - Method swizzledMethod = - - class_getInstanceMethod([self class], @selector(gdt_test_beginBackgroundTaskWithName: - expirationHandler:)); - - method_exchangeImplementations(originalMethod, swizzledMethod); - - // 2. Generate a test event. - - [self.generator generateEvent:GDTCOREventQoSFast]; - - // 3. Set up inverted expectations. - - [self setUpStorageExpectations]; - - self.testStorage.removeBatchWithoutDeletingEventsExpectation.inverted = YES; - - self.testStorage.batchWithEventSelectorExpectation.inverted = YES; - - self.testStorage.removeBatchAndDeleteEventsExpectation.inverted = YES; - - XCTestExpectation *hasEventsExpectation = - - [self expectStorageHasEventsForTarget:self.generator.target result:YES]; - - hasEventsExpectation.inverted = YES; - - // 4. Start the upload. - - [self.uploader uploadTarget:self.generator.target withConditions:GDTCORUploadConditionWifiData]; - - // 5. Wait for the upload to finish. - - [self waitForUploadOperationsToFinish:self.uploader]; - - // 6. Wait for all expectations to be fulfilled. - - [self waitForExpectations:@[ - - hasEventsExpectation, - - self.testStorage.batchWithEventSelectorExpectation, - - self.testStorage.removeBatchWithoutDeletingEventsExpectation, - - self.testStorage.removeBatchAndDeleteEventsExpectation, - - ] - - timeout:0.5]; - - // 7. Unswizzle the background task method. - - method_exchangeImplementations(swizzledMethod, originalMethod); -} - -#pragma mark - Swizzled Methods - -- (GDTCORBackgroundIdentifier)gdt_test_beginBackgroundTaskWithName:(NSString *)name - expirationHandler:(void (^)(void))handler { - if (handler) { - handler(); - } - return 1234; -} - #pragma mark - Storage interaction tests - (void)testStorageSelectorWhenConditionsHighPriority { From 98f22dc436de4f7789133db4e50461f868171ab9 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Fri, 28 Nov 2025 17:38:45 -0800 Subject: [PATCH 5/8] Delete unused variable --- .../GDTCCTLibrary/Private/GDTCCTUploadOperation.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/GoogleDataTransport/GDTCCTLibrary/Private/GDTCCTUploadOperation.h b/GoogleDataTransport/GDTCCTLibrary/Private/GDTCCTUploadOperation.h index cfc2c48..de234d5 100644 --- a/GoogleDataTransport/GDTCCTLibrary/Private/GDTCCTUploadOperation.h +++ b/GoogleDataTransport/GDTCCTLibrary/Private/GDTCCTUploadOperation.h @@ -70,9 +70,6 @@ NS_ASSUME_NONNULL_BEGIN /** The queue on which all CCT uploading will occur. */ @property(nonatomic, readonly) dispatch_queue_t uploaderQueue; -/** The current upload task. */ -@property(nullable, nonatomic, readonly) NSURLSessionUploadTask *currentTask; - @end NS_ASSUME_NONNULL_END From 6d59e57ee7942be7469a540ec93e58e296198f72 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Mon, 1 Dec 2025 09:12:58 -0800 Subject: [PATCH 6/8] review --- GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m | 3 +++ 1 file changed, 3 insertions(+) diff --git a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m index da0595a..4f688f8 100644 --- a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m +++ b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m @@ -137,6 +137,9 @@ - (void)uploadTarget:(GDTCORTarget)target withConditions:(GDTCORUploadConditions if (backgroundTaskID != GDTCORBackgroundIdentifierInvalid) { // Cancel the upload and complete delivery. [self cancel]; + + // End the background task. + backgroundTaskCompletion(); } else { GDTCORLog(GDTCORMCDDebugLog, GDTCORLoggingLevelWarnings, @"Attempted to cancel invalid background task in " From d61d7b8fbaa7f5ac58e29489b282692ed75e457e Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Mon, 8 Dec 2025 14:29:19 -0800 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com> --- CHANGELOG.md | 3 ++- GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75fa08d..d7bbfc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # v10.1.1 - Fix GDTCCTUploader-upload Crash. - (#156, [firebase-ios-sdk/#15129](https://github.com/firebase/firebase-ios-sdk/issues/15129)) +# Unreleased +- Cancel upload operation when background task expires. # v10.1.0 - Fix `[FBLPromise HTTPBody]` SwiftUI Previews crash when using binary diff --git a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m index 4f688f8..0b85840 100644 --- a/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m +++ b/GoogleDataTransport/GDTCCTLibrary/GDTCCTUploadOperation.m @@ -135,7 +135,8 @@ - (void)uploadTarget:(GDTCORTarget)target withConditions:(GDTCORUploadConditions beginBackgroundTaskWithName:@"GDTCCTUploader-upload" expirationHandler:^{ if (backgroundTaskID != GDTCORBackgroundIdentifierInvalid) { - // Cancel the upload and complete delivery. + // Cancel the upload. The outgoing upload network request + // is not cancelled and may or may not complete delivery. [self cancel]; // End the background task. From f6dbd636cc3849101b84bf868b1370b8fc58b500 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Mon, 8 Dec 2025 14:32:48 -0800 Subject: [PATCH 8/8] Fix Changelog merge --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7bbfc6..186ba25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,3 @@ -# v10.1.1 -- Fix GDTCCTUploader-upload Crash. # Unreleased - Cancel upload operation when background task expires.