From 28605fc5359c860d8e429cd5212c2dc3f7671f04 Mon Sep 17 00:00:00 2001 From: Themis wang Date: Tue, 22 Apr 2025 18:06:42 -0400 Subject: [PATCH] make Crashlytics context Init unlocking main --- Crashlytics/CHANGELOG.md | 3 + .../Crashlytics/Components/FIRCLSContext.h | 4 +- .../Crashlytics/Components/FIRCLSContext.m | 15 +-- .../Controllers/FIRCLSContextManager.h | 6 +- .../Controllers/FIRCLSContextManager.m | 6 +- .../Controllers/FIRCLSReportManager.m | 106 ++++++++++-------- .../UnitTests/FIRCLSContextManagerTests.m | 21 ++-- .../UnitTests/FIRCLSOnDemandModelTests.m | 7 +- .../UnitTests/FIRRecordExceptionModelTests.m | 7 +- .../UnitTests/Mocks/FIRCLSMockReportManager.m | 10 +- 10 files changed, 104 insertions(+), 81 deletions(-) diff --git a/Crashlytics/CHANGELOG.md b/Crashlytics/CHANGELOG.md index 14e65e3dae1..bc0df7e184f 100644 --- a/Crashlytics/CHANGELOG.md +++ b/Crashlytics/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Improved startup time by putting some initialization steps on a background. (#13675, #13232) + # 11.9.0 - [fixed] Made on-demand fatal recording thread suspension configurable through setting to improve performance and avoid audio glitch on Unity. Change is for framework only. diff --git a/Crashlytics/Crashlytics/Components/FIRCLSContext.h b/Crashlytics/Crashlytics/Components/FIRCLSContext.h index b477ae1bcdb..edd421cc3f0 100644 --- a/Crashlytics/Crashlytics/Components/FIRCLSContext.h +++ b/Crashlytics/Crashlytics/Components/FIRCLSContext.h @@ -39,6 +39,7 @@ __BEGIN_DECLS @class FIRCLSInstallIdentifierModel; @class FIRCLSFileManager; @class FIRCLSContextInitData; +@class FBLPromise; #endif typedef struct { @@ -82,7 +83,8 @@ typedef struct { FIRCLSAllocatorRef allocator; } FIRCLSContext; #ifdef __OBJC__ -bool FIRCLSContextInitialize(FIRCLSContextInitData* initData, FIRCLSFileManager* fileManager); +FBLPromise* FIRCLSContextInitialize(FIRCLSContextInitData* initData, + FIRCLSFileManager* fileManager); FIRCLSContextInitData* FIRCLSContextBuildInitData(FIRCLSInternalReport* report, FIRCLSSettings* settings, FIRCLSFileManager* fileManager, diff --git a/Crashlytics/Crashlytics/Components/FIRCLSContext.m b/Crashlytics/Crashlytics/Components/FIRCLSContext.m index a09ab43db48..3242a08e884 100644 --- a/Crashlytics/Crashlytics/Components/FIRCLSContext.m +++ b/Crashlytics/Crashlytics/Components/FIRCLSContext.m @@ -46,8 +46,6 @@ // We need enough space here for the context, plus storage for strings. #define CLS_MINIMUM_READABLE_SIZE (sizeof(FIRCLSReadOnlyContext) + 4096 * 4) -static const int64_t FIRCLSContextInitWaitTime = 5LL * NSEC_PER_SEC; - static const char* FIRCLSContextAppendToRoot(NSString* root, NSString* component); static void FIRCLSContextAllocate(FIRCLSContext* context); @@ -76,7 +74,8 @@ return initData; } -bool FIRCLSContextInitialize(FIRCLSContextInitData* initData, FIRCLSFileManager* fileManager) { +FBLPromise* FIRCLSContextInitialize(FIRCLSContextInitData* initData, + FIRCLSFileManager* fileManager) { if (!initData) { return false; } @@ -102,6 +101,8 @@ bool FIRCLSContextInitialize(FIRCLSContextInitData* initData, FIRCLSFileManager* // some values that aren't tied to particular subsystem _firclsContext.readonly->debuggerAttached = FIRCLSProcessDebuggerAttached(); + __block FBLPromise* initPromise = [FBLPromise pendingPromise]; + dispatch_group_async(group, queue, ^{ FIRCLSHostInitialize(&_firclsContext.readonly->host); }); @@ -220,14 +221,10 @@ bool FIRCLSContextInitialize(FIRCLSContextInitData* initData, FIRCLSFileManager* if (!FIRCLSAllocatorProtect(_firclsContext.allocator)) { FIRCLSSDKLog("Error: Memory protection failed\n"); } + [initPromise fulfill:nil]; }); - if (dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, FIRCLSContextInitWaitTime)) != - 0) { - FIRCLSSDKLog("Error: Delayed initialization\n"); - } - - return true; + return initPromise; } void FIRCLSContextBaseInit(void) { diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.h b/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.h index 1c107dc9c19..125fc1c49be 100644 --- a/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.h +++ b/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.h @@ -34,9 +34,9 @@ NS_ASSUME_NONNULL_BEGIN /// a new Session ID. @property(nonatomic, copy) NSString *appQualitySessionId; -- (BOOL)setupContextWithReport:(FIRCLSInternalReport *)report - settings:(FIRCLSSettings *)settings - fileManager:(FIRCLSFileManager *)fileManager; +- (FBLPromise *)setupContextWithReport:(FIRCLSInternalReport *)report + settings:(FIRCLSSettings *)settings + fileManager:(FIRCLSFileManager *)fileManager; @end diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.m b/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.m index 260b05fd1b9..68429b3f1be 100644 --- a/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.m +++ b/Crashlytics/Crashlytics/Controllers/FIRCLSContextManager.m @@ -40,9 +40,9 @@ - (instancetype)init { return self; } -- (BOOL)setupContextWithReport:(FIRCLSInternalReport *)report - settings:(FIRCLSSettings *)settings - fileManager:(FIRCLSFileManager *)fileManager { +- (FBLPromise *)setupContextWithReport:(FIRCLSInternalReport *)report + settings:(FIRCLSSettings *)settings + fileManager:(FIRCLSFileManager *)fileManager { _report = report; _settings = settings; _fileManager = fileManager; diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.m b/Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.m index 8d5b4095cd5..941dfa5d4c4 100644 --- a/Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.m +++ b/Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.m @@ -289,15 +289,26 @@ - (FBLPromise *)deleteUnsentReports { BOOL launchFailure = [self.launchMarker checkForAndCreateLaunchMarker]; - FIRCLSInternalReport *report = [self setupCurrentReport:executionIdentifier]; + __block FIRCLSInternalReport *report = [self setupCurrentReport:executionIdentifier]; if (!report) { FIRCLSErrorLog(@"Unable to setup a new report"); } - if (![self startCrashReporterWithProfilingReport:report]) { - FIRCLSErrorLog(@"Unable to start crash reporter"); - report = nil; - } + FBLPromise *reportProfilingPromise; + reportProfilingPromise = + [[self startCrashReporterWithProfilingReport:report] then:^id _Nullable(id _Nullable value) { + if ([value isEqual:@NO]) { + FIRCLSErrorLog(@"Unable to start crash reporter"); + report = nil; + return [FBLPromise resolvedWith:@NO]; + } + + // empty for disabled start-up time + dispatch_async(FIRCLSGetLoggingQueue(), ^{ + FIRCLSUserLoggingWriteInternalKeyValue(FIRCLSStartTimeKey, @""); + }); + return [FBLPromise resolvedWith:@YES]; + }]; #if CLS_METRICKIT_SUPPORTED if (@available(iOS 15, *)) { @@ -317,9 +328,12 @@ - (FBLPromise *)deleteUnsentReports { [self beginSettingsWithToken:dataCollectionToken]; // Wait for MetricKit data to be available, then continue to send reports and resolve promise. - promise = [[self waitForMetricKitData] + promise = [[reportProfilingPromise onQueue:_dispatchQueue + then:^id _Nullable(id _Nullable value) { + return [self waitForMetricKitData]; + }] onQueue:_dispatchQueue - then:^id _Nullable(id _Nullable metricKitValue) { + then:^id _Nullable(id _Nullable value) { [self beginReportUploadsWithToken:dataCollectionToken blockingSend:launchFailure]; // If data collection is enabled, the SDK will not notify the user @@ -335,36 +349,33 @@ - (FBLPromise *)deleteUnsentReports { // Wait for an action to get sent, either from processReports: or automatic data collection, // and for MetricKit data to be available. - promise = [[FBLPromise all:@[ [self waitForReportAction], [self waitForMetricKitData] ]] + promise = [[reportProfilingPromise onQueue:_dispatchQueue - then:^id _Nullable(NSArray *_Nullable wrappedActionAndData) { - // Process the actions for the reports on disk. - FIRCLSReportAction action = [[wrappedActionAndData firstObject] reportActionValue]; - - if (action == FIRCLSReportActionSend) { - FIRCLSDebugLog(@"Sending unsent reports."); - FIRCLSDataCollectionToken *dataCollectionToken = - [FIRCLSDataCollectionToken validToken]; - - [self beginSettingsWithToken:dataCollectionToken]; - - [self beginReportUploadsWithToken:dataCollectionToken blockingSend:NO]; - - } else if (action == FIRCLSReportActionDelete) { - FIRCLSDebugLog(@"Deleting unsent reports."); - [self.existingReportManager deleteUnsentReports]; - } else { - FIRCLSErrorLog(@"Unknown report action: %d", action); - } - return @(report != nil); - }]; - } - - if (report != nil) { - // empty for disabled start-up time - dispatch_async(FIRCLSGetLoggingQueue(), ^{ - FIRCLSUserLoggingWriteInternalKeyValue(FIRCLSStartTimeKey, @""); - }); + then:^id _Nullable(id _Nullable value) { + return [FBLPromise all:@[ [self waitForReportAction], [self waitForMetricKitData] ]]; + }] onQueue:_dispatchQueue + then:^id _Nullable(NSArray *_Nullable wrappedActionAndData) { + // Process the actions for the reports on disk. + FIRCLSReportAction action = + [[wrappedActionAndData firstObject] reportActionValue]; + + if (action == FIRCLSReportActionSend) { + FIRCLSDebugLog(@"Sending unsent reports."); + FIRCLSDataCollectionToken *dataCollectionToken = + [FIRCLSDataCollectionToken validToken]; + + [self beginSettingsWithToken:dataCollectionToken]; + + [self beginReportUploadsWithToken:dataCollectionToken blockingSend:NO]; + + } else if (action == FIRCLSReportActionDelete) { + FIRCLSDebugLog(@"Deleting unsent reports."); + [self.existingReportManager deleteUnsentReports]; + } else { + FIRCLSErrorLog(@"Unknown report action: %d", action); + } + return @(report != nil); + }]; } // To make the code more predictable and therefore testable, don't resolve the startup promise @@ -412,24 +423,23 @@ - (void)beginReportUploadsWithToken:(FIRCLSDataCollectionToken *)token } } -- (BOOL)startCrashReporterWithProfilingReport:(FIRCLSInternalReport *)report { +- (FBLPromise *)startCrashReporterWithProfilingReport:(FIRCLSInternalReport *)report { if (!report) { - return NO; - } - - if (![self.contextManager setupContextWithReport:report - settings:self.settings - fileManager:_fileManager]) { - return NO; + return [FBLPromise resolvedWith:@NO]; } - [self.notificationManager registerNotificationListener]; + return [[self.contextManager setupContextWithReport:report + settings:self.settings + fileManager:_fileManager] + then:^id _Nullable(id _Nullable value) { + [self.notificationManager registerNotificationListener]; - [self.analyticsManager registerAnalyticsListener]; + [self.analyticsManager registerAnalyticsListener]; - [self crashReportingSetupCompleted]; + [self crashReportingSetupCompleted]; - return YES; + return [FBLPromise resolvedWith:@YES]; + }]; } - (void)crashReportingSetupCompleted { diff --git a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m index 887eb442bf0..af2bde64223 100644 --- a/Crashlytics/UnitTests/FIRCLSContextManagerTests.m +++ b/Crashlytics/UnitTests/FIRCLSContextManagerTests.m @@ -68,9 +68,10 @@ - (void)tearDown { } - (void)test_notSettingSessionID_protoHasNilSessionID { - [self.contextManager setupContextWithReport:self.report - settings:self.mockSettings - fileManager:self.fileManager]; + FBLPromiseAwait([self.contextManager setupContextWithReport:self.report + settings:self.mockSettings + fileManager:self.fileManager], + nil); FIRCLSReportAdapter *adapter = [[FIRCLSReportAdapter alloc] initWithPath:self.report.path googleAppId:@"TestGoogleAppID" @@ -84,9 +85,10 @@ - (void)test_notSettingSessionID_protoHasNilSessionID { - (void)test_settingSessionIDMultipleTimes_protoHasLastSessionID { [self.contextManager setAppQualitySessionId:TestContextSessionID]; - [self.contextManager setupContextWithReport:self.report - settings:self.mockSettings - fileManager:self.fileManager]; + FBLPromiseAwait([self.contextManager setupContextWithReport:self.report + settings:self.mockSettings + fileManager:self.fileManager], + nil); [self.contextManager setAppQualitySessionId:TestContextSessionID2]; @@ -101,9 +103,10 @@ - (void)test_settingSessionIDMultipleTimes_protoHasLastSessionID { } - (void)test_settingSessionIDOutOfOrder_protoHasLastSessionID { - [self.contextManager setupContextWithReport:self.report - settings:self.mockSettings - fileManager:self.fileManager]; + FBLPromiseAwait([self.contextManager setupContextWithReport:self.report + settings:self.mockSettings + fileManager:self.fileManager], + nil); [self.contextManager setAppQualitySessionId:TestContextSessionID]; diff --git a/Crashlytics/UnitTests/FIRCLSOnDemandModelTests.m b/Crashlytics/UnitTests/FIRCLSOnDemandModelTests.m index a87b9cf122a..fb5bd894611 100644 --- a/Crashlytics/UnitTests/FIRCLSOnDemandModelTests.m +++ b/Crashlytics/UnitTests/FIRCLSOnDemandModelTests.m @@ -98,9 +98,10 @@ - (void)setUp { executionIdentifier:@"TEST_EXECUTION_IDENTIFIER"]; FIRCLSContextManager *contextManager = [[FIRCLSContextManager alloc] init]; - [contextManager setupContextWithReport:report - settings:self.mockSettings - fileManager:self.fileManager]; + FBLPromiseAwait([contextManager setupContextWithReport:report + settings:self.mockSettings + fileManager:self.fileManager], + nil); } - (void)tearDown { diff --git a/Crashlytics/UnitTests/FIRRecordExceptionModelTests.m b/Crashlytics/UnitTests/FIRRecordExceptionModelTests.m index e564614f8ae..b0733f5a910 100644 --- a/Crashlytics/UnitTests/FIRRecordExceptionModelTests.m +++ b/Crashlytics/UnitTests/FIRRecordExceptionModelTests.m @@ -54,9 +54,10 @@ - (void)setUp { executionIdentifier:@"TEST_EXECUTION_IDENTIFIER"]; FIRCLSContextManager *contextManager = [[FIRCLSContextManager alloc] init]; - [contextManager setupContextWithReport:report - settings:self.mockSettings - fileManager:self.fileManager]; + FBLPromiseAwait([contextManager setupContextWithReport:report + settings:self.mockSettings + fileManager:self.fileManager], + nil); } - (void)tearDown { diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockReportManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockReportManager.m index ff0dae7fe2a..e2c3fcc0d18 100644 --- a/Crashlytics/UnitTests/Mocks/FIRCLSMockReportManager.m +++ b/Crashlytics/UnitTests/Mocks/FIRCLSMockReportManager.m @@ -12,6 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +#if __has_include() +#import +#else +#import "FBLPromises.h" +#endif + #import "Crashlytics/UnitTests/Mocks/FIRCLSMockReportManager.h" #import "Crashlytics/Crashlytics/Components/FIRCLSContext.h" @@ -22,10 +28,10 @@ @implementation FIRCLSMockReportManager -- (BOOL)startCrashReporterWithProfilingReport:(FIRCLSInternalReport *)report { +- (FBLPromise *)startCrashReporterWithProfilingReport:(FIRCLSInternalReport *)report { NSLog(@"Crash Reporting system disabled for testing"); - return YES; + return [FBLPromise resolvedWith:@YES]; } - (BOOL)installCrashReportingHandlers:(FIRCLSContextInitData *)initData {