fix: Remove symbolication that causes deadlocks (#6562)

* fix: Remove symbolication that causes deadlocks

* PR feedback

* fix thread inspector tests

* Add comment

---------

Co-authored-by: Philipp Hofmann <[email protected]>

Author: noahsmartin

CHANGELOG.md

@@ -97,6 +97,7 @@
 - Properties on SentryOptions that had no effect on the WithoutUIKit variant are now removed from the API (#6644)
 - Removes the SentryOptions.inAppExclude property because it had no effect (#6646)
 - Removes segment property on SentryUser, SentryBaggage, and SentryTraceContext (#5638)
+- Removes local symbolication when `debug=True` which fixes various deadlocks (#6562)
 - Removes deprecated TraceContext initializers (#6348)
 - Removes deprecated user feedback API, this is replaced with the new feedback API (#5591)
 - Removes `enablePerformanceV2` option and makes this the default. The app start duration will now finish when the first frame is drawn instead of when the OS posts the UIWindowDidBecomeVisibleNotification. (#6008)

Sentry.xcodeproj/project.pbxproj

@@ -252,7 +252,6 @@
 		63FE711B20DA4C1000CDBAE8 /* SentryCrashString.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE701420DA4C1000CDBAE8 /* SentryCrashString.c */; };
 		63FE711D20DA4C1000CDBAE8 /* SentryCrashCPU_arm64.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE701520DA4C1000CDBAE8 /* SentryCrashCPU_arm64.c */; };
 		63FE711F20DA4C1000CDBAE8 /* SentryCrashObjC.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE701620DA4C1000CDBAE8 /* SentryCrashObjC.h */; };
-		63FE712120DA4C1000CDBAE8 /* SentryCrashSymbolicator.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE701720DA4C1000CDBAE8 /* SentryCrashSymbolicator.c */; };
 		63FE712320DA4C1000CDBAE8 /* SentryCrashID.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE701820DA4C1000CDBAE8 /* SentryCrashID.h */; };
 		63FE712520DA4C1000CDBAE8 /* SentryCrashSignalInfo.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE701920DA4C1000CDBAE8 /* SentryCrashSignalInfo.c */; };
 		63FE712720DA4C1000CDBAE8 /* SentryCrashThread.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE701A20DA4C1000CDBAE8 /* SentryCrashThread.c */; };
@@ -281,7 +280,6 @@
 		63FE715720DA4C1100CDBAE8 /* SentryCrashThread.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE703220DA4C1000CDBAE8 /* SentryCrashThread.h */; };
 		63FE715920DA4C1100CDBAE8 /* SentryCrashCPU_x86_32.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE703320DA4C1000CDBAE8 /* SentryCrashCPU_x86_32.c */; };
 		63FE715B20DA4C1100CDBAE8 /* SentryCrashSignalInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE703420DA4C1000CDBAE8 /* SentryCrashSignalInfo.h */; };
-		63FE715D20DA4C1100CDBAE8 /* SentryCrashSymbolicator.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE703520DA4C1000CDBAE8 /* SentryCrashSymbolicator.h */; };
 		63FE715F20DA4C1100CDBAE8 /* SentryCrashID.c in Sources */ = {isa = PBXBuildFile; fileRef = 63FE703620DA4C1000CDBAE8 /* SentryCrashID.c */; };
 		63FE716320DA4C1100CDBAE8 /* SentryCrashDynamicLinker.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE703820DA4C1000CDBAE8 /* SentryCrashDynamicLinker.h */; };
 		63FE716520DA4C1100CDBAE8 /* SentryCrashMemory.h in Headers */ = {isa = PBXBuildFile; fileRef = 63FE703920DA4C1000CDBAE8 /* SentryCrashMemory.h */; };
@@ -1542,7 +1540,6 @@
 		63FE701420DA4C1000CDBAE8 /* SentryCrashString.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashString.c; sourceTree = "<group>"; };
 		63FE701520DA4C1000CDBAE8 /* SentryCrashCPU_arm64.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashCPU_arm64.c; sourceTree = "<group>"; };
 		63FE701620DA4C1000CDBAE8 /* SentryCrashObjC.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SentryCrashObjC.h; sourceTree = "<group>"; };
-		63FE701720DA4C1000CDBAE8 /* SentryCrashSymbolicator.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashSymbolicator.c; sourceTree = "<group>"; };
 		63FE701820DA4C1000CDBAE8 /* SentryCrashID.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SentryCrashID.h; sourceTree = "<group>"; };
 		63FE701920DA4C1000CDBAE8 /* SentryCrashSignalInfo.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashSignalInfo.c; sourceTree = "<group>"; };
 		63FE701A20DA4C1000CDBAE8 /* SentryCrashThread.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashThread.c; sourceTree = "<group>"; };
@@ -1571,7 +1568,6 @@
 		63FE703220DA4C1000CDBAE8 /* SentryCrashThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryCrashThread.h; path = ../../../Sentry/include/SentryCrashThread.h; sourceTree = "<group>"; };
 		63FE703320DA4C1000CDBAE8 /* SentryCrashCPU_x86_32.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashCPU_x86_32.c; sourceTree = "<group>"; };
 		63FE703420DA4C1000CDBAE8 /* SentryCrashSignalInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SentryCrashSignalInfo.h; sourceTree = "<group>"; };
-		63FE703520DA4C1000CDBAE8 /* SentryCrashSymbolicator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SentryCrashSymbolicator.h; sourceTree = "<group>"; };
 		63FE703620DA4C1000CDBAE8 /* SentryCrashID.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = SentryCrashID.c; sourceTree = "<group>"; };
 		63FE703820DA4C1000CDBAE8 /* SentryCrashDynamicLinker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryCrashDynamicLinker.h; path = ../../../Sentry/include/SentryCrashDynamicLinker.h; sourceTree = "<group>"; };
 		63FE703920DA4C1000CDBAE8 /* SentryCrashMemory.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SentryCrashMemory.h; sourceTree = "<group>"; };
@@ -3290,8 +3286,6 @@
 				63FE701C20DA4C1000CDBAE8 /* SentryCrashStackCursor.h */,
 				63FE701420DA4C1000CDBAE8 /* SentryCrashString.c */,
 				63FE702C20DA4C1000CDBAE8 /* SentryCrashString.h */,
-				63FE701720DA4C1000CDBAE8 /* SentryCrashSymbolicator.c */,
-				63FE703520DA4C1000CDBAE8 /* SentryCrashSymbolicator.h */,
 				63FE701E20DA4C1000CDBAE8 /* SentryCrashSysCtl.c */,
 				63FE703D20DA4C1000CDBAE8 /* SentryCrashSysCtl.h */,
 				63FE701A20DA4C1000CDBAE8 /* SentryCrashThread.c */,
@@ -5147,7 +5141,6 @@
 				639FCF981EBC7B9700778193 /* SentryEvent.h in Headers */,
 				03F84D2527DD414C008FE43F /* SentryThreadState.hpp in Headers */,
 				8E4E7C6D25DAAAFE006AB9E2 /* SentryTransaction.h in Headers */,
-				63FE715D20DA4C1100CDBAE8 /* SentryCrashSymbolicator.h in Headers */,
 				FAE2DABA2E1F318900262307 /* SentryProfilingSwiftHelpers.h in Headers */,
 				D8ACE3CF2762187D00F5A213 /* SentryFileIOTrackingIntegration.h in Headers */,
 				FA27ECA12EBA325A00F2ECF7 /* SentryTraceContext+Private.h in Headers */,
@@ -5893,7 +5886,6 @@
 				63FE716720DA4C1100CDBAE8 /* SentryCrashCPU.c in Sources */,
 				63FE717320DA4C1100CDBAE8 /* SentryCrashC.c in Sources */,
 				6293F5752D422A95002BC3BD /* SentryStacktraceCodable.swift in Sources */,
-				63FE712120DA4C1000CDBAE8 /* SentryCrashSymbolicator.c in Sources */,
 				627C77892D50B6840055E966 /* SentryBreadcrumbCodable.swift in Sources */,
 				63FE70D720DA4C1000CDBAE8 /* SentryCrashMonitor_MachException.c in Sources */,
 				7B96572226830D2400C66E25 /* SentryScopeSyncC.c in Sources */,

Sources/Sentry/SentryCrashReportConverter.m

@@ -307,15 +307,11 @@ - (SentryFrame *)stackFrameAtIndex:(NSInteger)frameIndex inThreadIndex:(NSIntege
         = (uintptr_t)[frameDictionary[@"instruction_addr"] unsignedLongLongValue];
     NSDictionary *binaryImage = [self binaryImageForAddress:instructionAddress];
     SentryFrame *frame = [[SentryFrame alloc] init];
-    frame.symbolAddress = sentry_formatHexAddress(frameDictionary[@"symbol_addr"]);
     frame.instructionAddress = sentry_formatHexAddress(frameDictionary[@"instruction_addr"]);
     frame.imageAddress = sentry_formatHexAddress(binaryImage[@"image_addr"]);
     frame.package = binaryImage[@"name"];
     BOOL isInApp = [self.inAppLogic isInApp:binaryImage[@"name"]];
     frame.inApp = @(isInApp);
-    if (frameDictionary[@"symbol_name"]) {
-        frame.function = frameDictionary[@"symbol_name"];
-    }
     return frame;
 }
 

Sources/Sentry/SentryCrashStackEntryMapper.m

@@ -25,36 +25,15 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack
 {
     SentryFrame *frame = [[SentryFrame alloc] init];
 
-    if (stackEntry.symbolAddress != 0) {
-        frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress);
-    }
-
     frame.instructionAddress = sentry_formatHexAddressUInt64(stackEntry.address);
 
-    if (stackEntry.symbolName != NULL) {
-        frame.function = [NSString stringWithCString:stackEntry.symbolName
-                                            encoding:NSUTF8StringEncoding];
-    }
-
-    // If there is no symbolication, because debug was disabled
-    // we get image from the cache.
-    if (stackEntry.imageAddress == 0 && stackEntry.imageName == NULL) {
-        SentryBinaryImageInfo *info = [SentryDependencyContainer.sharedInstance.binaryImageCache
-            imageByAddress:(uint64_t)stackEntry.address];
-
-        frame.imageAddress = sentry_formatHexAddressUInt64(info.address);
-        frame.package = info.name;
-        frame.inApp = @([self.inAppLogic isInApp:info.name]);
-    } else {
-        frame.imageAddress = sentry_formatHexAddressUInt64(stackEntry.imageAddress);
+    // Get image from the cache.
+    SentryBinaryImageInfo *info = [SentryDependencyContainer.sharedInstance.binaryImageCache
+        imageByAddress:(uint64_t)stackEntry.address];
 
-        if (stackEntry.imageName != NULL) {
-            NSString *imageName = [NSString stringWithCString:stackEntry.imageName
-                                                     encoding:NSUTF8StringEncoding];
-            frame.package = imageName;
-            frame.inApp = @([self.inAppLogic isInApp:imageName]);
-        }
-    }
+    frame.imageAddress = sentry_formatHexAddressUInt64(info.address);
+    frame.package = info.name;
+    frame.inApp = @([self.inAppLogic isInApp:info.name]);
 
     return frame;
 }

Sources/Sentry/SentryDefaultThreadInspector.m

@@ -3,7 +3,6 @@
 #import "SentryCrashStackCursor.h"
 #include "SentryCrashStackCursor_MachineContext.h"
 #import "SentryCrashStackEntryMapper.h"
-#include "SentryCrashSymbolicator.h"
 #import "SentryFrame.h"
 #import "SentryStacktrace.h"
 #import "SentryStacktraceBuilder.h"
@@ -31,7 +30,7 @@ @interface SentryDefaultThreadInspector ()
 // async-signal-safe.
 unsigned int
 getStackEntriesFromThread(SentryCrashThread thread, struct SentryCrashMachineContext *context,
-    SentryCrashStackEntry *buffer, unsigned int maxEntries, bool asyncUnsafeSymbolicate)
+    SentryCrashStackEntry *buffer, unsigned int maxEntries)
 {
     sentrycrashmc_getContextForThread(thread, context, NO);
     SentryCrashStackCursor stackCursor;
@@ -42,10 +41,8 @@ @interface SentryDefaultThreadInspector ()
     while (stackCursor.advanceCursor(&stackCursor)) {
         if (entries == maxEntries)
             break;
-        if (asyncUnsafeSymbolicate == false || stackCursor.symbolicate(&stackCursor)) {
-            buffer[entries] = stackCursor.stackEntry;
-            entries++;
-        }
+        buffer[entries] = stackCursor.stackEntry;
+        entries++;
     }
 
     return entries;
@@ -55,12 +52,10 @@ @implementation SentryDefaultThreadInspector
 
 - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
        andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper
-                    symbolicate:(BOOL)symbolicate
 {
     if (self = [super init]) {
         self.stacktraceBuilder = stacktraceBuilder;
         self.machineContextWrapper = machineContextWrapper;
-        self.symbolicate = symbolicate;
     }
     return self;
 }
@@ -73,13 +68,11 @@ - (instancetype)initWithOptions:(SentryOptions *_Nullable)options
         [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
     SentryStacktraceBuilder *stacktraceBuilder =
         [[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];
-    stacktraceBuilder.symbolicate = options.debug;
 
     id<SentryCrashMachineContextWrapper> machineContextWrapper =
         [[SentryCrashDefaultMachineContextWrapper alloc] init];
     return [self initWithStacktraceBuilder:stacktraceBuilder
-                  andMachineContextWrapper:machineContextWrapper
-                               symbolicate:options.debug];
+                  andMachineContextWrapper:machineContextWrapper];
 }
 
 - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
@@ -142,8 +135,6 @@ - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
         thread_act_array_t suspendedThreads = NULL;
         mach_msg_type_number_t numSuspendedThreads = 0;
 
-        bool symbolicate = self.symbolicate;
-
         // SentryThreadInspector is crashing when there is too many threads.
         // We add a limit of 70 threads because in test with up to 100 threads it seems fine.
         // We are giving it an extra safety margin.
@@ -163,7 +154,7 @@ - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
         for (int i = 0; i < numSuspendedThreads; i++) {
             if (suspendedThreads[i] != currentThread) {
                 int numberOfEntries = getStackEntriesFromThread(suspendedThreads[i], context,
-                    threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH, symbolicate);
+                    threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH);
                 threadsInfos[i].stackLength = numberOfEntries;
             } else {
                 // We can't use 'getStackEntriesFromThread' to retrieve stack frames from the

Sources/Sentry/SentryStacktrace.m

@@ -29,12 +29,9 @@ - (void)fixDuplicateFrames
         return;
     }
 
-    SentryFrame *lastFrame = self.frames.lastObject;
     SentryFrame *beforeLastFrame = [self.frames objectAtIndex:self.frames.count - 2];
 
-    if ([lastFrame.symbolAddress
-            isEqualToString:SENTRY_UNWRAP_NULLABLE(NSString, beforeLastFrame.symbolAddress)]
-        && [self.registers[@"lr"]
+    if ([self.registers[@"lr"]
             isEqualToString:SENTRY_UNWRAP_NULLABLE(NSString, beforeLastFrame.instructionAddress)]) {
         NSMutableArray *copyFrames = self.frames.mutableCopy;
         [copyFrames removeObjectAtIndex:self.frames.count - 2];

Sources/Sentry/SentryStacktraceBuilder.m

@@ -3,7 +3,6 @@
 #import "SentryCrashStackCursor_MachineContext.h"
 #import "SentryCrashStackCursor_SelfThread.h"
 #import "SentryCrashStackEntryMapper.h"
-#import "SentryCrashSymbolicator.h"
 #import "SentryFrame.h"
 #import "SentryLogC.h"
 #import "SentryStacktrace.h"
@@ -24,7 +23,6 @@ - (id)initWithCrashStackEntryMapper:(SentryCrashStackEntryMapper *)crashStackEnt
 {
     if (self = [super init]) {
         self.crashStackEntryMapper = crashStackEntryMapper;
-        self.symbolicate = NO;
     }
     return self;
 }
@@ -41,10 +39,8 @@ - (SentryStacktrace *)retrieveStacktraceFromCursor:(SentryCrashStackCursor)stack
             // skip the marker frame
             continue;
         }
-        if (self.symbolicate == NO || stackCursor.symbolicate(&stackCursor)) {
-            frame = [self.crashStackEntryMapper mapStackEntryWithCursor:stackCursor];
-            [frames addObject:frame];
-        }
+        frame = [self.crashStackEntryMapper mapStackEntryWithCursor:stackCursor];
+        [frames addObject:frame];
     }
 
     return [SentryStacktraceBuilder buildStacktraceFromFrames:frames];
@@ -96,7 +92,6 @@ - (nullable SentryStacktrace *)buildStacktraceForCurrentThreadAsyncUnsafe
     SENTRY_LOG_DEBUG(@"Building async-unsafe stack trace...");
     SentryCrashStackCursor stackCursor;
     sentrycrashsc_initSelfThread(&stackCursor, 0);
-    stackCursor.symbolicate = sentrycrashsymbolicator_symbolicate_async_unsafe;
     return [self retrieveStacktraceFromCursor:stackCursor];
 }
 

Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m

@@ -1,6 +1,5 @@
 #import "SentryUseNSExceptionCallstackWrapper.h"
 #import "SentryCrashStackEntryMapper.h"
-#import "SentryCrashSymbolicator.h"
 #import "SentrySDK+Private.h"
 #import "SentryStacktraceBuilder.h"
 #import "SentrySwift.h"
@@ -39,12 +38,15 @@ - (instancetype)initWithName:(NSExceptionName)aName
     SentryCrashStackEntryMapper *crashStackToEntryMapper = [self buildCrashStackToEntryMapper];
     NSMutableArray<SentryFrame *> *frames = [NSMutableArray array];
 
-    // Iterate over all the addresses, symbolicate and create a SentryFrame
+    // Iterate over all the addresses and create a SentryFrame
     [self.returnAddressesArray
         enumerateObjectsUsingBlock:^(NSNumber *_Nonnull obj, NSUInteger idx, BOOL *_Nonnull stop) {
             SentryCrashStackCursor stackCursor;
             stackCursor.stackEntry.address = [obj unsignedLongValue];
-            sentrycrashsymbolicator_symbolicate_async_unsafe_sentryDlAddr(&stackCursor);
+            stackCursor.stackEntry.imageName = nil;
+            stackCursor.stackEntry.imageAddress = 0;
+            stackCursor.stackEntry.symbolAddress = 0;
+            stackCursor.stackEntry.symbolName = nil;
 
             [frames addObject:[crashStackToEntryMapper
                                   sentryCrashStackEntryToSentryFrame:stackCursor.stackEntry]];

Sources/Sentry/include/SentryCrashDynamicLinker.h

@@ -97,26 +97,6 @@ uint32_t sentrycrashdl_imageNamed(const char *const imageName, bool exactMatch);
  */
 const uint8_t *sentrycrashdl_imageUUID(const char *const imageName, bool exactMatch);
 
-/**
- * ATTENTION: This method isn't async-safe as it accesses @c _dyld_get_image_header, which acquires
- * a lock. We plan on removing this method with
- * https://github.com/getsentry/sentry-cocoa/issues/2996.
- *
- *
- * This method searches the dynamic loader for information about any image
- * containing the specified address. It may not be entirely successful in
- * finding information, in which case any fields it could not find will be set
- * to NULL.
- *
- * Unlike dladdr(), this method does not make use of locks, and does not call
- * async-unsafe functions.
- *
- * @param address The address to search for.
- * @param info Gets filled out by this function.
- * @return true if at least some information was found.
- */
-bool sentrycrashdl_dladdr(const uintptr_t address, Dl_info *const info);
-
 void sentrycrashdl_getCrashInfo(uint64_t address, SentryCrashBinaryImage *buffer);
 
 void sentrycrashdl_initialize(void);

Sources/Sentry/include/SentryDefaultThreadInspector.h

@@ -15,8 +15,7 @@ NS_ASSUME_NONNULL_BEGIN
 SENTRY_NO_INIT
 
 - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
-       andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper
-                    symbolicate:(BOOL)symbolicate;
+       andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper;
 
 - (instancetype)initWithOptions:(SentryOptionsObjC *_Nullable)options;