From c6f8576f25f2c2b032db4c2810684b58f181b67f Mon Sep 17 00:00:00 2001 From: Themis wang Date: Tue, 28 Jan 2025 17:07:19 -0500 Subject: [PATCH 1/4] work in progress --- Crashlytics/Crashlytics/Handlers/FIRCLSException.mm | 4 ++-- Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h | 2 +- Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m | 10 +++++++--- Crashlytics/Crashlytics/Handlers/FIRCLSMachException.c | 2 +- Crashlytics/Crashlytics/Handlers/FIRCLSSignal.c | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm b/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm index e894f5fff71..0e33929b7ed 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm @@ -235,7 +235,7 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type, FIRCLSExceptionWrite(&file, type, name, reason, frames, nil); // We only want to do this work if we have the expectation that we'll actually crash - FIRCLSHandler(&file, mach_thread_self(), NULL); + FIRCLSHandler(&file, mach_thread_self(), NULL, YES); FIRCLSFileClose(&file); }); @@ -353,7 +353,7 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type, return nil; } FIRCLSExceptionWrite(&file, type, name, reason, frames, nil); - FIRCLSHandler(&file, mach_thread_self(), NULL); + FIRCLSHandler(&file, mach_thread_self(), NULL, NO); FIRCLSFileClose(&file); // Return the path to the new report. diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h index 04bbab76fe8..3b8d736a3ab 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h @@ -20,6 +20,6 @@ __BEGIN_DECLS -void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid); +void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid, bool isNativeFatal); __END_DECLS diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m index b9d027f52c9..1648eb93964 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m @@ -22,12 +22,14 @@ #import "Crashlytics/Crashlytics/Controllers/FIRCLSReportManager_Private.h" -void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid) { +void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid, bool isNativeFatal) { FIRCLSProcess process; FIRCLSProcessInit(&process, crashedThread, uapVoid); - FIRCLSProcessSuspendAllOtherThreads(&process); + if (isNativeFatal) { + FIRCLSProcessSuspendAllOtherThreads(&process); + } FIRCLSProcessRecordAllThreads(&process, file); @@ -45,5 +47,7 @@ void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid) { // Store a crash file marker to indicate that a crash has occurred FIRCLSCreateCrashedMarkerFile(); - FIRCLSProcessResumeAllOtherThreads(&process); + if (isNativeFatal) { + FIRCLSProcessResumeAllOtherThreads(&process); + } } diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSMachException.c b/Crashlytics/Crashlytics/Handlers/FIRCLSMachException.c index 35fe5cb45d3..048a975180b 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSMachException.c +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSMachException.c @@ -520,7 +520,7 @@ static bool FIRCLSMachExceptionRecord(FIRCLSMachExceptionReadContext* context, FIRCLSFileWriteSectionEnd(&file); - FIRCLSHandler(&file, message->thread.name, NULL); + FIRCLSHandler(&file, message->thread.name, NULL, true); FIRCLSFileClose(&file); diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSSignal.c b/Crashlytics/Crashlytics/Handlers/FIRCLSSignal.c index 850f58e165d..155490c1c3c 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSSignal.c +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSSignal.c @@ -286,7 +286,7 @@ static void FIRCLSSignalRecordSignal(int savedErrno, siginfo_t *info, void *uapV FIRCLSFileWriteSectionEnd(&file); - FIRCLSHandler(&file, mach_thread_self(), uapVoid); + FIRCLSHandler(&file, mach_thread_self(), uapVoid, true); FIRCLSFileClose(&file); } From a7bf51d3fafeb9991cae61a8f6a5d3bdbdad88b1 Mon Sep 17 00:00:00 2001 From: Themis wang Date: Tue, 11 Feb 2025 18:06:08 -0500 Subject: [PATCH 2/4] add setting flag for on demand thread suspension --- Crashlytics/Crashlytics/Handlers/FIRCLSException.h | 6 ++++-- Crashlytics/Crashlytics/Handlers/FIRCLSException.mm | 11 +++++++---- Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h | 5 ++++- Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m | 9 ++++++--- Crashlytics/Crashlytics/Models/FIRCLSOnDemandModel.m | 4 +++- Crashlytics/Crashlytics/Models/FIRCLSSettings.h | 5 +++++ Crashlytics/Crashlytics/Models/FIRCLSSettings.m | 9 +++++++++ 7 files changed, 38 insertions(+), 11 deletions(-) diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSException.h b/Crashlytics/Crashlytics/Handlers/FIRCLSException.h index 65aae9bfd32..d8c229f9c42 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSException.h +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSException.h @@ -63,7 +63,8 @@ void FIRCLSExceptionRaiseTestCppException(void) __attribute((noreturn)); void FIRCLSExceptionRecordModel(FIRExceptionModel* exceptionModel, NSString* rolloutsInfoJSON); NSString* FIRCLSExceptionRecordOnDemandModel(FIRExceptionModel* exceptionModel, int previousRecordedOnDemandExceptions, - int previousDroppedOnDemandExceptions); + int previousDroppedOnDemandExceptions, + BOOL shouldSuspendThread); void FIRCLSExceptionRecordNSException(NSException* exception); void FIRCLSExceptionRecord(FIRCLSExceptionType type, const char* name, @@ -76,7 +77,8 @@ NSString* FIRCLSExceptionRecordOnDemand(FIRCLSExceptionType type, NSArray* frames, BOOL fatal, int previousRecordedOnDemandExceptions, - int previousDroppedOnDemandExceptions); + int previousDroppedOnDemandExceptions, + BOOL shouldSuspendThread); #endif __END_DECLS diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm b/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm index 0e33929b7ed..2ee85fb4346 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSException.mm @@ -91,14 +91,15 @@ void FIRCLSExceptionRecordModel(FIRExceptionModel *exceptionModel, NSString *rol NSString *FIRCLSExceptionRecordOnDemandModel(FIRExceptionModel *exceptionModel, int previousRecordedOnDemandExceptions, - int previousDroppedOnDemandExceptions) { + int previousDroppedOnDemandExceptions, + BOOL shouldSuspendThread) { const char *name = [[exceptionModel.name copy] UTF8String]; const char *reason = [[exceptionModel.reason copy] UTF8String] ?: ""; return FIRCLSExceptionRecordOnDemand(FIRCLSExceptionTypeCustom, name, reason, [exceptionModel.stackTrace copy], exceptionModel.isFatal, previousRecordedOnDemandExceptions, - previousDroppedOnDemandExceptions); + previousDroppedOnDemandExceptions, shouldSuspendThread); } void FIRCLSExceptionRecordNSException(NSException *exception) { @@ -258,7 +259,8 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type, NSArray *frames, BOOL fatal, int previousRecordedOnDemandExceptions, - int previousDroppedOnDemandExceptions) { + int previousDroppedOnDemandExceptions, + BOOL shouldSuspendThread) { if (!FIRCLSContextIsInitialized()) { return nil; } @@ -353,7 +355,8 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type, return nil; } FIRCLSExceptionWrite(&file, type, name, reason, frames, nil); - FIRCLSHandler(&file, mach_thread_self(), NULL, NO); + + FIRCLSHandler(&file, mach_thread_self(), NULL, shouldSuspendThread); FIRCLSFileClose(&file); // Return the path to the new report. diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h index 3b8d736a3ab..c1734356520 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h @@ -20,6 +20,9 @@ __BEGIN_DECLS -void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid, bool isNativeFatal); +void FIRCLSHandler(FIRCLSFile* file, + thread_t crashedThread, + void* uapVoid, + bool shouldSuspendThread); __END_DECLS diff --git a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m index 1648eb93964..b9348f0e804 100644 --- a/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m +++ b/Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m @@ -22,12 +22,15 @@ #import "Crashlytics/Crashlytics/Controllers/FIRCLSReportManager_Private.h" -void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid, bool isNativeFatal) { +void FIRCLSHandler(FIRCLSFile* file, + thread_t crashedThread, + void* uapVoid, + bool shouldSuspendThread) { FIRCLSProcess process; FIRCLSProcessInit(&process, crashedThread, uapVoid); - if (isNativeFatal) { + if (shouldSuspendThread) { FIRCLSProcessSuspendAllOtherThreads(&process); } @@ -47,7 +50,7 @@ void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid, bool // Store a crash file marker to indicate that a crash has occurred FIRCLSCreateCrashedMarkerFile(); - if (isNativeFatal) { + if (shouldSuspendThread) { FIRCLSProcessResumeAllOtherThreads(&process); } } diff --git a/Crashlytics/Crashlytics/Models/FIRCLSOnDemandModel.m b/Crashlytics/Crashlytics/Models/FIRCLSOnDemandModel.m index 6283d9e200c..649940f0bd9 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSOnDemandModel.m +++ b/Crashlytics/Crashlytics/Models/FIRCLSOnDemandModel.m @@ -178,8 +178,10 @@ - (void)implementOnDemandUploadDelay:(int)delay { } - (NSString *)recordOnDemandExceptionWithModel:(FIRExceptionModel *)exceptionModel { + BOOL shouldSuspendThread = self.settings.onDemandThreadSuspensionEnabled; return FIRCLSExceptionRecordOnDemandModel(exceptionModel, self.recordedOnDemandExceptionCount, - self.droppedOnDemandExceptionCount); + self.droppedOnDemandExceptionCount, + shouldSuspendThread); } - (int)droppedOnDemandExceptionCount { diff --git a/Crashlytics/Crashlytics/Models/FIRCLSSettings.h b/Crashlytics/Crashlytics/Models/FIRCLSSettings.h index 59e3f5f9ba8..f122fffd93b 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSSettings.h +++ b/Crashlytics/Crashlytics/Models/FIRCLSSettings.h @@ -121,6 +121,11 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic, readonly) uint32_t onDemandBackoffStepDuration; +/** + * When this is true, Crashlytics will suspend all threads to do on-demand fatal recording. + */ +@property(nonatomic, readonly) BOOL onDemandThreadSuspensionEnabled; + @end NS_ASSUME_NONNULL_END diff --git a/Crashlytics/Crashlytics/Models/FIRCLSSettings.m b/Crashlytics/Crashlytics/Models/FIRCLSSettings.m index c0703eccdab..3e3112ec35a 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSSettings.m +++ b/Crashlytics/Crashlytics/Models/FIRCLSSettings.m @@ -357,4 +357,13 @@ - (uint32_t)onDemandBackoffStepDuration { return 6; // step duration for exponential backoff } +- (BOOL)onDemandThreadSuspensionEnabled { + NSNumber *value = self.settingsDictionary[@"on_demand_thread_suspension_enabled"]; + + if (value != nil) { + return value.boolValue; + } + + return NO; +} @end From 231c7a48a292dd5478bed29efb09a05bcc58763c Mon Sep 17 00:00:00 2001 From: Themis wang Date: Thu, 13 Feb 2025 13:47:25 -0500 Subject: [PATCH 3/4] change default value to current behavior --- Crashlytics/Crashlytics/Models/FIRCLSSettings.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Crashlytics/Crashlytics/Models/FIRCLSSettings.m b/Crashlytics/Crashlytics/Models/FIRCLSSettings.m index 3e3112ec35a..758772e5b8d 100644 --- a/Crashlytics/Crashlytics/Models/FIRCLSSettings.m +++ b/Crashlytics/Crashlytics/Models/FIRCLSSettings.m @@ -358,12 +358,12 @@ - (uint32_t)onDemandBackoffStepDuration { } - (BOOL)onDemandThreadSuspensionEnabled { - NSNumber *value = self.settingsDictionary[@"on_demand_thread_suspension_enabled"]; + NSNumber *value = self.settingsDictionary[@"on_demand_thread_recording_suspension_enabled"]; if (value != nil) { return value.boolValue; } - return NO; + return YES; } @end From 47ccc2307778b1971a7e3093771cd79ba196145c Mon Sep 17 00:00:00 2001 From: Themis wang Date: Thu, 13 Feb 2025 14:02:05 -0500 Subject: [PATCH 4/4] add changelog --- Crashlytics/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Crashlytics/CHANGELOG.md b/Crashlytics/CHANGELOG.md index baf09d78f57..fd5a3625887 100644 --- a/Crashlytics/CHANGELOG.md +++ b/Crashlytics/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Made on-demand fatal recording thread suspension configurable through setting to imrpove performance and avoid audio glitch on Unity. Change is for framework only. + # 11.7.0 - [fixed] Updated `upload-symbols` to version 3.20, wait for `debug.dylib` DWARF content getting generated when build with `--build-phase` option. Added `debug.dylib` DWARF content to run script input file list for user who enabled user script sandboxing (#14054). - [fixed] Updated all memory allocation from `malloc()` to `calloc()` (#14209).