Not suspend threads for on-demand fatal thread recording (#14391)

Author: themiswang

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).

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<FIRStackFrame*>* frames,
                                         BOOL fatal,
                                         int previousRecordedOnDemandExceptions,
-                                        int previousDroppedOnDemandExceptions);
+                                        int previousDroppedOnDemandExceptions,
+                                        BOOL shouldSuspendThread);
 #endif
 
 __END_DECLS

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) {
@@ -235,7 +236,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);
     });
@@ -258,7 +259,8 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type,
                                         NSArray<FIRStackFrame *> *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);
+
+  FIRCLSHandler(&file, mach_thread_self(), NULL, shouldSuspendThread);
   FIRCLSFileClose(&file);
 
   // Return the path to the new report.

Crashlytics/Crashlytics/Handlers/FIRCLSHandler.h

@@ -20,6 +20,9 @@
 
 __BEGIN_DECLS
 
-void FIRCLSHandler(FIRCLSFile* file, thread_t crashedThread, void* uapVoid);
+void FIRCLSHandler(FIRCLSFile* file,
+                   thread_t crashedThread,
+                   void* uapVoid,
+                   bool shouldSuspendThread);
 
 __END_DECLS

Crashlytics/Crashlytics/Handlers/FIRCLSHandler.m

@@ -22,12 +22,17 @@
 
 #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 shouldSuspendThread) {
   FIRCLSProcess process;
 
   FIRCLSProcessInit(&process, crashedThread, uapVoid);
 
-  FIRCLSProcessSuspendAllOtherThreads(&process);
+  if (shouldSuspendThread) {
+    FIRCLSProcessSuspendAllOtherThreads(&process);
+  }
 
   FIRCLSProcessRecordAllThreads(&process, file);
 
@@ -45,5 +50,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 (shouldSuspendThread) {
+    FIRCLSProcessResumeAllOtherThreads(&process);
+  }
 }

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);
 

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);
 }

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 {

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

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_recording_suspension_enabled"];
+
+  if (value != nil) {
+    return value.boolValue;
+  }
+
+  return YES;
+}
 @end