From 0b44b1ef08e305b2a0c206e9acbb2424a3a768ba Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Thu, 13 Jun 2024 16:20:28 +0200 Subject: [PATCH 01/32] feat: Resume session replay when app enters foreground (#4053) The session replay pauses when the app enters background, now it resumes when back to the foreground --- CHANGELOG.md | 1 + Sources/Sentry/SentrySessionReplay.m | 19 +++++++++++++++++-- .../Sentry/SentrySessionReplayIntegration.m | 12 ++++++++++-- Sources/Sentry/include/SentrySessionReplay.h | 5 +++++ .../SentrySessionReplayIntegration+Private.h | 4 ++++ .../SentrySessionReplayIntegrationTests.swift | 15 ++++++++++++++- 6 files changed, 51 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bddfea5789..0abb6f9c283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add a touch tracker for replay (#4041) - Add enableMetricKitRawPayload (#4044) +- Resume session replay when app enters foreground (#4053) ### Fixes diff --git a/Sources/Sentry/SentrySessionReplay.m b/Sources/Sentry/SentrySessionReplay.m index e3219cd6521..bd6938453cb 100644 --- a/Sources/Sentry/SentrySessionReplay.m +++ b/Sources/Sentry/SentrySessionReplay.m @@ -117,12 +117,28 @@ - (void)stop @synchronized(self) { [_displayLink invalidate]; _isRunning = NO; + [self prepareSegmentUntil:_dateProvider.date]; + } +} + +- (void)resume +{ + if (_reachedMaximumDuration == YES) { + return; + } + @synchronized(self) { + if (_isRunning) { + return; + } + _videoSegmentStart = nil; + [_displayLink linkWithTarget:self selector:@selector(newFrame:)]; + _isRunning = YES; } } - (void)dealloc { - [self stop]; + [_displayLink invalidate]; } - (void)captureReplayForEvent:(SentryEvent *)event; @@ -203,7 +219,6 @@ - (void)newFrame:(CADisplayLink *)sender if (_isFullSession && [now timeIntervalSinceDate:_sessionStart] > _replayOptions.maximumDuration) { _reachedMaximumDuration = YES; - [self prepareSegmentUntil:now]; [self stop]; return; } diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index 7d73f6486cd..cfc3378c5cd 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -29,10 +29,8 @@ */ static SentryTouchTracker *_touchTracker; -API_AVAILABLE(ios(16.0), tvos(16.0)) @interface SentrySessionReplayIntegration () -@property (nonatomic, strong) SentrySessionReplay *sessionReplay; - (void)newSceneActivate; @end @@ -141,6 +139,11 @@ - (void)startWithOptions:(SentryReplayOptions *)replayOptions name:UIApplicationDidEnterBackgroundNotification object:nil]; + [NSNotificationCenter.defaultCenter addObserver:self + selector:@selector(resume) + name:UIApplicationWillEnterForegroundNotification + object:nil]; + [SentryGlobalEventProcessor.shared addEventProcessor:^SentryEvent *_Nullable(SentryEvent *_Nonnull event) { [self.sessionReplay captureReplayForEvent:event]; @@ -154,6 +157,11 @@ - (void)stop [self.sessionReplay stop]; } +- (void)resume +{ + [self.sessionReplay resume]; +} + - (void)captureReplay { [self.sessionReplay captureReplay]; diff --git a/Sources/Sentry/include/SentrySessionReplay.h b/Sources/Sentry/include/SentrySessionReplay.h index 8383b23bc00..369432826d7 100644 --- a/Sources/Sentry/include/SentrySessionReplay.h +++ b/Sources/Sentry/include/SentrySessionReplay.h @@ -47,6 +47,11 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)stop; +/** + * Continue recording a stopped session replay. + */ +- (void)resume; + /** * Captures a replay for given event. */ diff --git a/Sources/Sentry/include/SentrySessionReplayIntegration+Private.h b/Sources/Sentry/include/SentrySessionReplayIntegration+Private.h index c26ba23060d..277b9a6baff 100644 --- a/Sources/Sentry/include/SentrySessionReplayIntegration+Private.h +++ b/Sources/Sentry/include/SentrySessionReplayIntegration+Private.h @@ -4,9 +4,13 @@ #if SENTRY_HAS_UIKIT && !TARGET_OS_VISION +@class SentrySessionReplay; + @interface SentrySessionReplayIntegration () +@property (nonatomic, strong) SentrySessionReplay *sessionReplay; + @end #endif diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 4627ef6e750..9c17ff434b5 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -86,7 +86,6 @@ class SentrySessionReplayIntegrationTests: XCTestCase { } func testInstallFullSessionReplayBecauseOfRandom() { - SentryDependencyContainer.sharedInstance().random = TestRandom(value: 0.1) startSDK(sessionSampleRate: 0.2, errorSampleRate: 0) @@ -116,6 +115,20 @@ class SentrySessionReplayIntegrationTests: XCTestCase { NotificationCenter.default.post(name: UIScene.didActivateNotification, object: nil) expect(Dynamic(sut).sessionReplay.asObject) != nil } + + func testPauseAndResumeForApplicationStateChange() { + startSDK(sessionSampleRate: 1, errorSampleRate: 0) + + guard let sut = SentrySDK.currentHub().installedIntegrations().first as? SentrySessionReplayIntegration else { + XCTFail("Did not find Session Replay Integration") + return + } + + NotificationCenter.default.post(name: UIApplication.didEnterBackgroundNotification, object: nil) + XCTAssertFalse(Dynamic(sut.sessionReplay).isRunning.asBool ?? true) + NotificationCenter.default.post(name: UIApplication.willEnterForegroundNotification, object: nil) + XCTAssertTrue(Dynamic(sut.sessionReplay).isRunning.asBool ?? false) + } } #endif From 2093e059e13e3290bbef06bf8d2f2bed33018555 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 13 Jun 2024 16:43:46 +0200 Subject: [PATCH 02/32] ref: Remove unused imports (#4065) Remove two unused imports in SentryUIViewControllerPerformanceTracker and SentryDelayedFramesTracker. --- Sources/Sentry/SentryUIViewControllerPerformanceTracker.m | 1 - Sources/Sentry/include/SentryDelayedFramesTracker.h | 1 - 2 files changed, 2 deletions(-) diff --git a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m index e8e23a45eda..82844579906 100644 --- a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m +++ b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m @@ -8,7 +8,6 @@ # import "SentryOptions.h" # import "SentryPerformanceTracker.h" # import "SentrySDK+Private.h" -# import "SentryScope.h" # import "SentrySpanId.h" # import "SentrySwift.h" # import "SentryTimeToDisplayTracker.h" diff --git a/Sources/Sentry/include/SentryDelayedFramesTracker.h b/Sources/Sentry/include/SentryDelayedFramesTracker.h index f2c3527caeb..bb02b3bf7f6 100644 --- a/Sources/Sentry/include/SentryDelayedFramesTracker.h +++ b/Sources/Sentry/include/SentryDelayedFramesTracker.h @@ -2,7 +2,6 @@ #if SENTRY_HAS_UIKIT -@class SentryDisplayLinkWrapper; @class SentryCurrentDateProvider; NS_ASSUME_NONNULL_BEGIN From 16d73bf63217a0df13d6e8fece9258b69268f566 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 13 Jun 2024 09:30:27 -0800 Subject: [PATCH 03/32] fix(profiling): various fixes for chunk envelopes to validate (#4058) --- .../Profiling/SentryProfilerSerialization.mm | 25 ++++++++++--------- .../Sentry/Profiling/SentryProfilerState.mm | 4 +-- Sources/Sentry/SentryFramesTracker.m | 5 ++-- Sources/Sentry/SentryMetricProfiler.mm | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Sources/Sentry/Profiling/SentryProfilerSerialization.mm b/Sources/Sentry/Profiling/SentryProfilerSerialization.mm index 4d1c2e7779e..3ee3688c563 100644 --- a/Sources/Sentry/Profiling/SentryProfilerSerialization.mm +++ b/Sources/Sentry/Profiling/SentryProfilerSerialization.mm @@ -222,7 +222,7 @@ } NSMutableDictionary * -sentry_serializedContinuousProfileChunk(SentryId *profileID, +sentry_serializedContinuousProfileChunk(SentryId *profileID, SentryId *chunkID, NSDictionary *profileData, NSDictionary *serializedMetrics, NSArray *debugMeta, SentryHub *hub # if SENTRY_HAS_UIKIT @@ -254,7 +254,7 @@ payload[@"debug_meta"] = @ { @"images" : debugImages }; } - payload[@"chunk_id"] = [[[SentryId alloc] init] sentryIdString]; + payload[@"chunk_id"] = [chunkID sentryIdString]; payload[@"profiler_id"] = profileID.sentryIdString; payload[@"environment"] = hub.scope.environmentString ?: hub.getClient.options.environment; payload[@"release"] = hub.getClient.options.releaseName; @@ -268,14 +268,14 @@ [NSMutableDictionary dictionaryWithDictionary:metrics]; if (gpuData.slowFrameTimestamps.count > 0) { const auto values = [NSMutableDictionary dictionary]; - values[@"unit"] = @"millisecond"; + values[@"unit"] = @"nanosecond"; values[@"values"] = gpuData.slowFrameTimestamps; mutableMetrics[kSentryProfilerSerializationKeySlowFrameRenders] = values; } if (gpuData.frozenFrameTimestamps.count > 0) { const auto values = [NSMutableDictionary dictionary]; - values[@"unit"] = @"millisecond"; + values[@"unit"] = @"nanosecond"; values[@"values"] = gpuData.frozenFrameTimestamps; mutableMetrics[kSentryProfilerSerializationKeyFrozenFrameRenders] = values; } @@ -308,15 +308,16 @@ # endif // SENTRY_HAS_UIKIT ) { - const auto payload - = sentry_serializedContinuousProfileChunk(profileID, profileState, metricProfilerState, - [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesCrashed:NO], - SentrySDK.currentHub + const auto chunkID = [[SentryId alloc] init]; + const auto payload = sentry_serializedContinuousProfileChunk( + profileID, chunkID, profileState, metricProfilerState, + [SentryDependencyContainer.sharedInstance.debugImageProvider getDebugImagesCrashed:NO], + SentrySDK.currentHub # if SENTRY_HAS_UIKIT - , - gpuData + , + gpuData # endif // SENTRY_HAS_UIKIT - ); + ); if (payload == nil) { SENTRY_LOG_DEBUG(@"Payload was empty, will not create a profiling envelope item."); @@ -340,7 +341,7 @@ length:JSONData.length]; const auto envelopeItem = [[SentryEnvelopeItem alloc] initWithHeader:header data:JSONData]; - return [[SentryEnvelope alloc] initWithId:[[SentryId alloc] init] singleItem:envelopeItem]; + return [[SentryEnvelope alloc] initWithId:chunkID singleItem:envelopeItem]; } SentryEnvelopeItem *_Nullable sentry_traceProfileEnvelopeItem( diff --git a/Sources/Sentry/Profiling/SentryProfilerState.mm b/Sources/Sentry/Profiling/SentryProfilerState.mm index 9a246810eeb..ca34a466fb0 100644 --- a/Sources/Sentry/Profiling/SentryProfilerState.mm +++ b/Sources/Sentry/Profiling/SentryProfilerState.mm @@ -149,8 +149,8 @@ - (void)appendBacktrace:(const Backtrace &)backtrace const auto sample = [[SentrySample alloc] init]; sample.absoluteTimestamp = backtrace.absoluteTimestamp; - sample.absoluteNSDateInterval = SentryDependencyContainer.sharedInstance.dateProvider.date - .timeIntervalSinceReferenceDate; + sample.absoluteNSDateInterval + = SentryDependencyContainer.sharedInstance.dateProvider.date.timeIntervalSince1970; sample.threadID = backtrace.threadMetadata.threadID; const auto stackKey = [stack componentsJoinedByString:@"|"]; diff --git a/Sources/Sentry/SentryFramesTracker.m b/Sources/Sentry/SentryFramesTracker.m index bda5b4e28e6..e7133516d66 100644 --- a/Sources/Sentry/SentryFramesTracker.m +++ b/Sources/Sentry/SentryFramesTracker.m @@ -202,9 +202,8 @@ - (void)displayLinkCallback # if SENTRY_TARGET_PROFILING_SUPPORTED NSDate *thisFrameNSDate = self.dateProvider.date; BOOL isContinuousProfiling = [SentryContinuousProfiler isCurrentlyProfiling]; - NSNumber *profilingTimestamp = isContinuousProfiling - ? @(thisFrameNSDate.timeIntervalSinceReferenceDate) - : @(thisFrameSystemTimestamp); + NSNumber *profilingTimestamp = isContinuousProfiling ? @(thisFrameNSDate.timeIntervalSince1970) + : @(thisFrameSystemTimestamp); if ([SentryTraceProfiler isCurrentlyProfiling] || isContinuousProfiling) { BOOL hasNoFrameRatesYet = self.frameRateTimestamps.count == 0; uint64_t previousFrameRate diff --git a/Sources/Sentry/SentryMetricProfiler.mm b/Sources/Sentry/SentryMetricProfiler.mm index cce3f0754d1..2ac24c1c5cb 100644 --- a/Sources/Sentry/SentryMetricProfiler.mm +++ b/Sources/Sentry/SentryMetricProfiler.mm @@ -297,7 +297,7 @@ - (SentryMetricReading *)metricReadingForValue:(NSNumber *)value reading.value = value; const auto dateProvider = SentryDependencyContainer.sharedInstance.dateProvider; reading.absoluteSystemTimestamp = dateProvider.systemTime; - reading.absoluteNSDateInterval = dateProvider.date.timeIntervalSinceReferenceDate; + reading.absoluteNSDateInterval = dateProvider.date.timeIntervalSince1970; return reading; } From 35b9d5431ec93e07231fc5ab91163ee6a6991cf2 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 13 Jun 2024 09:31:28 -0800 Subject: [PATCH 04/32] fix: remove dead code duplicated in SentryCrash (#4057) --- .../Installations/SentryCrashInstallation.h | 8 -------- .../Installations/SentryCrashInstallation.m | 14 -------------- 2 files changed, 22 deletions(-) diff --git a/Sources/SentryCrash/Installations/SentryCrashInstallation.h b/Sources/SentryCrash/Installations/SentryCrashInstallation.h index 16b1fb96dca..67ba8e40ed1 100644 --- a/Sources/SentryCrash/Installations/SentryCrashInstallation.h +++ b/Sources/SentryCrash/Installations/SentryCrashInstallation.h @@ -39,14 +39,6 @@ NS_ASSUME_NONNULL_BEGIN */ @interface SentryCrashInstallation : NSObject -/** C Function to call during a crash report to give the callee an opportunity - * to add to the report. NULL = ignore. - * - * WARNING: Only call async-safe functions from this function! DO NOT call - * Objective-C methods!!! - */ -@property (atomic, readwrite, assign) SentryCrashReportWriteCallback onCrash; - /** Install this installation. Call this instead of -[SentryCrash install] to * install with everything needed for your particular backend. */ diff --git a/Sources/SentryCrash/Installations/SentryCrashInstallation.m b/Sources/SentryCrash/Installations/SentryCrashInstallation.m index 5c1c220d4e5..209dfcc2118 100644 --- a/Sources/SentryCrash/Installations/SentryCrashInstallation.m +++ b/Sources/SentryCrash/Installations/SentryCrashInstallation.m @@ -165,20 +165,6 @@ - (NSArray *)makeKeyPaths:(NSArray *)keyPaths return result; } -- (SentryCrashReportWriteCallback)onCrash -{ - @synchronized(self) { - return self.crashHandlerData->userCrashCallback; - } -} - -- (void)setOnCrash:(SentryCrashReportWriteCallback)onCrash -{ - @synchronized(self) { - self.crashHandlerData->userCrashCallback = onCrash; - } -} - - (void)install:(NSString *)customCacheDirectory { SentryCrash *handler = SentryDependencyContainer.sharedInstance.crashReporter; From 14a6d4f4ff98274f34a59cd5575c0bf5bfa1778e Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 13 Jun 2024 09:32:24 -0800 Subject: [PATCH 05/32] test(iOS-Swift): more selective enabling/disabling of SDK features (#4060) --- .../iOS-Swift-UITests/BaseUITest.swift | 4 +-- .../xcshareddata/xcschemes/iOS-Swift.xcscheme | 30 +++++++++++++++++++ Samples/iOS-Swift/iOS-Swift/AppDelegate.swift | 30 +++++++++---------- .../iOS-Swift/ExtraViewController.swift | 2 +- .../Profiling/SentryProfilerSerialization.mm | 3 +- Sources/Sentry/SentryProfiler.mm | 2 +- 6 files changed, 51 insertions(+), 20 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift-UITests/BaseUITest.swift b/Samples/iOS-Swift/iOS-Swift-UITests/BaseUITest.swift index dfdedee18d1..ac7a449d348 100644 --- a/Samples/iOS-Swift/iOS-Swift-UITests/BaseUITest.swift +++ b/Samples/iOS-Swift/iOS-Swift-UITests/BaseUITest.swift @@ -11,7 +11,7 @@ class BaseUITest: XCTestCase { super.setUp() continueAfterFailure = false XCUIDevice.shared.orientation = .portrait - app.launchEnvironment["io.sentry.sdk-environment"] = "ui-tests" + app.launchEnvironment["--io.sentry.sdk-environment"] = "ui-tests" if automaticallyLaunchAndTerminateApp { launchApp() } @@ -28,7 +28,7 @@ class BaseUITest: XCTestCase { extension BaseUITest { func newAppSession() -> XCUIApplication { let app = XCUIApplication() - app.launchEnvironment["io.sentry.ui-test.test-name"] = name + app.launchEnvironment["--io.sentry.ui-test.test-name"] = name return app } diff --git a/Samples/iOS-Swift/iOS-Swift.xcodeproj/xcshareddata/xcschemes/iOS-Swift.xcscheme b/Samples/iOS-Swift/iOS-Swift.xcodeproj/xcshareddata/xcschemes/iOS-Swift.xcscheme index 0d21ce7f36e..3f091f996b3 100644 --- a/Samples/iOS-Swift/iOS-Swift.xcodeproj/xcshareddata/xcschemes/iOS-Swift.xcscheme +++ b/Samples/iOS-Swift/iOS-Swift.xcodeproj/xcshareddata/xcschemes/iOS-Swift.xcscheme @@ -73,6 +73,26 @@ argument = "--disable-file-io-tracing" isEnabled = "NO"> + + + + + + + + + + @@ -159,6 +179,16 @@ value = "" isEnabled = "NO"> + + + + Date: Thu, 13 Jun 2024 09:32:57 -0800 Subject: [PATCH 06/32] fix: remove unused DSN property in SentryNSURLRequest (#4059) --- Sources/Sentry/SentryNSURLRequest.m | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Sources/Sentry/SentryNSURLRequest.m b/Sources/Sentry/SentryNSURLRequest.m index d1e26c41cc2..22b840c5700 100644 --- a/Sources/Sentry/SentryNSURLRequest.m +++ b/Sources/Sentry/SentryNSURLRequest.m @@ -17,13 +17,6 @@ NSString *const SentryServerVersionString = @"7"; NSTimeInterval const SentryRequestTimeout = 15; -@interface -SentryNSURLRequest () - -@property (nonatomic, strong) SentryDsn *dsn; - -@end - @implementation SentryNSURLRequest - (_Nullable instancetype)initStoreRequestWithDsn:(SentryDsn *)dsn From e145ca1d83c5845c1d82e26e7b566e1431ab236e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Fri, 14 Jun 2024 08:34:56 +0200 Subject: [PATCH 07/32] fix: Potential deadlock in app hang detection (#4063) The app hang detection retrieves the stacktraces of all threads when it detects an app hang. The logic for retrieving the stacktrace uses sentrycrashdl_dladdr, which isn't async-safe, although it claims to be because it accesses _dyld_get_image_header, which acquires a lock. Therefore, in rare cases, retrieving the stacktrace for all threads can lead to a deadlock. We fix this by skipping symbolication, which calls sentrycrashdl_dladdr when debug is false, which we we thought we did in Fixes GH-4056 --- CHANGELOG.md | 6 +++ Sources/Sentry/SentryCrashStackEntryMapper.m | 4 +- Sources/Sentry/SentryStacktraceBuilder.m | 2 +- Sources/Sentry/SentryThreadInspector.m | 14 +++-- .../Sentry/include/SentryThreadInspector.h | 3 +- .../Tools/SentryCrashDynamicLinker.h | 6 ++- .../Recording/Tools/SentryCrashSymbolicator.c | 3 ++ .../SentryCrashStackEntryMapperTests.swift | 9 ++++ .../SentryThreadInspectorTests.swift | 53 ++++++++++++++++--- .../SentryCrash/TestThreadInspector.swift | 2 +- 10 files changed, 86 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e33bd5474a3..935b616abdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Fix potential deadlock in app hang detection (#4063) + ## 8.29.0 ### Features diff --git a/Sources/Sentry/SentryCrashStackEntryMapper.m b/Sources/Sentry/SentryCrashStackEntryMapper.m index 433ee796995..d9ac3f94f46 100644 --- a/Sources/Sentry/SentryCrashStackEntryMapper.m +++ b/Sources/Sentry/SentryCrashStackEntryMapper.m @@ -29,7 +29,9 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack { SentryFrame *frame = [[SentryFrame alloc] init]; - frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress); + if (stackEntry.symbolAddress != 0) { + frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress); + } frame.instructionAddress = sentry_formatHexAddressUInt64(stackEntry.address); diff --git a/Sources/Sentry/SentryStacktraceBuilder.m b/Sources/Sentry/SentryStacktraceBuilder.m index 7a841f7777e..1ff89403848 100644 --- a/Sources/Sentry/SentryStacktraceBuilder.m +++ b/Sources/Sentry/SentryStacktraceBuilder.m @@ -42,7 +42,7 @@ - (SentryStacktrace *)retrieveStacktraceFromCursor:(SentryCrashStackCursor)stack // skip the marker frame continue; } - if (self.symbolicate == false || stackCursor.symbolicate(&stackCursor)) { + if (self.symbolicate == NO || stackCursor.symbolicate(&stackCursor)) { frame = [self.crashStackEntryMapper mapStackEntryWithCursor:stackCursor]; [frames addObject:frame]; } diff --git a/Sources/Sentry/SentryThreadInspector.m b/Sources/Sentry/SentryThreadInspector.m index e9b828c317c..6d303aca895 100644 --- a/Sources/Sentry/SentryThreadInspector.m +++ b/Sources/Sentry/SentryThreadInspector.m @@ -18,6 +18,7 @@ @property (nonatomic, strong) SentryStacktraceBuilder *stacktraceBuilder; @property (nonatomic, strong) id machineContextWrapper; +@property (nonatomic, assign) BOOL symbolicate; @end @@ -31,7 +32,7 @@ // calling into not async-signal-safe code while there are suspended threads. unsigned int getStackEntriesFromThread(SentryCrashThread thread, struct SentryCrashMachineContext *context, - SentryCrashStackEntry *buffer, unsigned int maxEntries) + SentryCrashStackEntry *buffer, unsigned int maxEntries, bool symbolicate) { sentrycrashmc_getContextForThread(thread, context, NO); SentryCrashStackCursor stackCursor; @@ -42,7 +43,7 @@ while (stackCursor.advanceCursor(&stackCursor)) { if (entries == maxEntries) break; - if (stackCursor.symbolicate(&stackCursor)) { + if (symbolicate == false || stackCursor.symbolicate(&stackCursor)) { buffer[entries] = stackCursor.stackEntry; entries++; } @@ -55,10 +56,12 @@ @implementation SentryThreadInspector - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder andMachineContextWrapper:(id)machineContextWrapper + symbolicate:(BOOL)symbolicate { if (self = [super init]) { self.stacktraceBuilder = stacktraceBuilder; self.machineContextWrapper = machineContextWrapper; + self.symbolicate = symbolicate; } return self; } @@ -77,7 +80,8 @@ - (instancetype)initWithOptions:(SentryOptions *)options id machineContextWrapper = [[SentryCrashDefaultMachineContextWrapper alloc] init]; return [self initWithStacktraceBuilder:stacktraceBuilder - andMachineContextWrapper:machineContextWrapper]; + andMachineContextWrapper:machineContextWrapper + symbolicate:options.debug]; } - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe @@ -140,6 +144,8 @@ - (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. @@ -159,7 +165,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); + threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH, symbolicate); threadsInfos[i].stackLength = numberOfEntries; } else { // We can't use 'getStackEntriesFromThread' to retrieve stack frames from the diff --git a/Sources/Sentry/include/SentryThreadInspector.h b/Sources/Sentry/include/SentryThreadInspector.h index b493b4f2948..6611b39c37e 100644 --- a/Sources/Sentry/include/SentryThreadInspector.h +++ b/Sources/Sentry/include/SentryThreadInspector.h @@ -10,7 +10,8 @@ NS_ASSUME_NONNULL_BEGIN SENTRY_NO_INIT - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder - andMachineContextWrapper:(id)machineContextWrapper; + andMachineContextWrapper:(id)machineContextWrapper + symbolicate:(BOOL)symbolicate; - (instancetype)initWithOptions:(SentryOptions *)options; diff --git a/Sources/SentryCrash/Recording/Tools/SentryCrashDynamicLinker.h b/Sources/SentryCrash/Recording/Tools/SentryCrashDynamicLinker.h index 4e66e1fb6d4..dc2ab1cfcb9 100644 --- a/Sources/SentryCrash/Recording/Tools/SentryCrashDynamicLinker.h +++ b/Sources/SentryCrash/Recording/Tools/SentryCrashDynamicLinker.h @@ -99,7 +99,11 @@ uint32_t sentrycrashdl_imageNamed(const char *const imageName, bool exactMatch); */ const uint8_t *sentrycrashdl_imageUUID(const char *const imageName, bool exactMatch); -/** async-safe version of dladdr. +/** + * 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 diff --git a/Sources/SentryCrash/Recording/Tools/SentryCrashSymbolicator.c b/Sources/SentryCrash/Recording/Tools/SentryCrashSymbolicator.c index 0f151a13158..b3132694d95 100644 --- a/Sources/SentryCrash/Recording/Tools/SentryCrashSymbolicator.c +++ b/Sources/SentryCrash/Recording/Tools/SentryCrashSymbolicator.c @@ -68,6 +68,9 @@ symbolicate_internal(SentryCrashStackCursor *cursor, bool asyncUnsafe) if (asyncUnsafe) { symbols_succeed = dladdr((void *)cursor->stackEntry.address, &symbolsBuffer) != 0; } else { + // sentrycrashdl_dladdr isn't async safe, but we've been using it for + // crashes since the beginning. We plan on removing it with + // https://github.com/getsentry/sentry-cocoa/issues/2996. symbols_succeed = sentrycrashdl_dladdr( CALL_INSTRUCTION_FROM_RETURN_ADDRESS(cursor->stackEntry.address), &symbolsBuffer); } diff --git a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift index 0245b2582e5..448df612a99 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift @@ -28,6 +28,15 @@ class SentryCrashStackEntryMapperTests: XCTestCase { XCTAssertEqual("0x000000008e902bf0", frame.symbolAddress ?? "") } + func testSymbolAddress_IsZero() { + var cursor = SentryCrashStackCursor() + cursor.stackEntry.symbolAddress = 0 + + let frame = sut.mapStackEntry(with: cursor) + + XCTAssertNil(frame.symbolAddress) + } + func testInstructionAddress() { var cursor = SentryCrashStackCursor() cursor.stackEntry.address = 2_412_813_376 diff --git a/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift b/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift index 767316040e6..a0930392380 100644 --- a/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryThreadInspectorTests.swift @@ -17,7 +17,7 @@ class SentryThreadInspectorTests: XCTestCase { return SentryThreadInspector( stacktraceBuilder: stacktraceBuilder, - andMachineContextWrapper: machineContextWrapper + andMachineContextWrapper: machineContextWrapper, symbolicate: symbolicate ) } } @@ -47,18 +47,51 @@ class SentryThreadInspectorTests: XCTestCase { XCTAssertTrue(30 < stacktrace?.frames.count ?? 0, "Not enough stacktrace frames.") } - func testStacktraceHasFrames_forEveryThread() { - assertStackForEveryThread() + func testGetCurrentThreadsWithStacktrace_WithSymbolication() { + let queue = DispatchQueue(label: "test-queue", attributes: [.concurrent, .initiallyInactive]) + + let expect = expectation(description: "Read every thread") + expect.expectedFulfillmentCount = 10 + + let sut = self.fixture.getSut(testWithRealMachineContextWrapper: true) + for _ in 0..<10 { + + queue.async { + let actual = sut.getCurrentThreadsWithStackTrace() + + // Sometimes during tests its possible to have one thread without frames + // We just need to make sure we retrieve frame information for at least one other thread than the main thread + var threadsWithFrames = 0 + + for thr in actual { + if (thr.stacktrace?.frames.count ?? 0) >= 1 { + threadsWithFrames += 1 + } + + for frame in thr.stacktrace?.frames ?? [] { + XCTAssertNotNil(frame.instructionAddress) + XCTAssertNotNil(frame.imageAddress) + } + } + + XCTAssertTrue(threadsWithFrames > 1, "Not enough threads with frames") + + expect.fulfill() + } + } + + queue.activate() + wait(for: [expect], timeout: 10) } - func assertStackForEveryThread() { - - let queue = DispatchQueue(label: "defaultphil", attributes: [.concurrent, .initiallyInactive]) + func testGetCurrentThreadsWithStacktrace_WithoutSymbolication() { + let queue = DispatchQueue(label: "test-queue", attributes: [.concurrent, .initiallyInactive]) let expect = expectation(description: "Read every thread") expect.expectedFulfillmentCount = 10 - let sut = self.fixture.getSut(testWithRealMachineContextWrapper: true) + let sut = self.fixture.getSut(testWithRealMachineContextWrapper: true, symbolicate: false) + for _ in 0..<10 { queue.async { @@ -72,6 +105,12 @@ class SentryThreadInspectorTests: XCTestCase { if (thr.stacktrace?.frames.count ?? 0) >= 1 { threadsWithFrames += 1 } + + for frame in thr.stacktrace?.frames ?? [] { + XCTAssertNotNil(frame.instructionAddress) + XCTAssertNotNil(frame.imageAddress) + XCTAssertNil(frame.symbolAddress) + } } XCTAssertTrue(threadsWithFrames > 1, "Not enough threads with frames") diff --git a/Tests/SentryTests/SentryCrash/TestThreadInspector.swift b/Tests/SentryTests/SentryCrash/TestThreadInspector.swift index 984546ed298..f177d7feef0 100644 --- a/Tests/SentryTests/SentryCrash/TestThreadInspector.swift +++ b/Tests/SentryTests/SentryCrash/TestThreadInspector.swift @@ -9,7 +9,7 @@ class TestThreadInspector: SentryThreadInspector { let inAppLogic = SentryInAppLogic(inAppIncludes: [], inAppExcludes: []) let crashStackEntryMapper = SentryCrashStackEntryMapper(inAppLogic: inAppLogic) let stacktraceBuilder = SentryStacktraceBuilder(crashStackEntryMapper: crashStackEntryMapper) - return TestThreadInspector(stacktraceBuilder: stacktraceBuilder, andMachineContextWrapper: SentryCrashDefaultMachineContextWrapper()) + return TestThreadInspector(stacktraceBuilder: stacktraceBuilder, andMachineContextWrapper: SentryCrashDefaultMachineContextWrapper(), symbolicate: false) } override func stacktraceForCurrentThreadAsyncUnsafe() -> SentryStacktrace? { From 4090908328ccb35d7764c6f3228b17444aac5651 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Fri, 14 Jun 2024 10:38:21 +0200 Subject: [PATCH 08/32] test: Remove Nimble from iOS-Swift sample app (#4072) We don't use Nimble for the iOS-Swift UI tests, so we can remove the package reference. --- .../iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj b/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj index 22a1c6cf49d..ae21dc487e1 100644 --- a/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj +++ b/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj @@ -786,7 +786,6 @@ ); mainGroup = 637AFD9D243B02760034958B; packageReferences = ( - 6268F2C02B0DF9920019DA38 /* XCRemoteSwiftPackageReference "Nimble" */, ); productRefGroup = 637AFDA7243B02760034958B /* Products */; projectDirPath = ""; @@ -2097,17 +2096,6 @@ }; /* End XCConfigurationList section */ -/* Begin XCRemoteSwiftPackageReference section */ - 6268F2C02B0DF9920019DA38 /* XCRemoteSwiftPackageReference "Nimble" */ = { - isa = XCRemoteSwiftPackageReference; - repositoryURL = "https://github.com/Quick/Nimble"; - requirement = { - kind = exactVersion; - version = 10.0.0; - }; - }; -/* End XCRemoteSwiftPackageReference section */ - /* Begin XCVersionGroup section */ D845F35927BAD4CC00A4D7A2 /* SentryData.xcdatamodeld */ = { isa = XCVersionGroup; From e19cca315a89dc377bdeeb5c9ed33d0038720eaa Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Fri, 14 Jun 2024 10:59:40 +0200 Subject: [PATCH 09/32] Swizzling of view controllers `loadView` that don`t implement `loadView` (#4071) Stop swizzling loadView function of UIViewControllers that doesn't implement it --- CHANGELOG.md | 1 + Samples/iOS-Swift/iOS-Swift/Tools/UIAssert.swift | 2 +- Sources/Sentry/SentryUIViewControllerSwizzling.m | 12 +++++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 935b616abdd..15cb85bbd4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Fix potential deadlock in app hang detection (#4063) +- Swizzling of view controllers `loadView` that don`t implement `loadView` (#4071) ## 8.29.0 diff --git a/Samples/iOS-Swift/iOS-Swift/Tools/UIAssert.swift b/Samples/iOS-Swift/iOS-Swift/Tools/UIAssert.swift index 7b591a3d1ed..2769e9909c3 100644 --- a/Samples/iOS-Swift/iOS-Swift/Tools/UIAssert.swift +++ b/Samples/iOS-Swift/iOS-Swift/Tools/UIAssert.swift @@ -84,7 +84,7 @@ class UIAssert { return } - let steps = stepsToCheck ?? ["loadView", "viewDidLoad", "viewWillAppear", "viewDidAppear"] + let steps = stepsToCheck ?? ["viewDidLoad", "viewWillAppear", "viewDidAppear"] var missing = [String]() steps.forEach { spanDescription in diff --git a/Sources/Sentry/SentryUIViewControllerSwizzling.m b/Sources/Sentry/SentryUIViewControllerSwizzling.m index 88564b33035..45c25b75fdf 100644 --- a/Sources/Sentry/SentryUIViewControllerSwizzling.m +++ b/Sources/Sentry/SentryUIViewControllerSwizzling.m @@ -348,13 +348,15 @@ - (BOOL)shouldSwizzleViewController:(Class)class - (void)swizzleLoadView:(Class)class { - // The UIViewController only searches for a nib file if you do not override the loadView method. + // Loading a Nib file is done automatically during `loadView` in the UIViewController + // or other native view controllers. // When swizzling the loadView of a custom UIViewController, the UIViewController doesn't search - // for a nib file and doesn't load a view. This would lead to crashes as no view is loaded. As a - // workaround, we skip swizzling the loadView and accept that the SKD doesn't create a span for - // loadView if the UIViewController doesn't implement it. + // for a nib file and doesn't load a view. This would lead to crashes as no view is loaded. + // By checking the implementation pointer of `loadView` from the current class with + // the implementation pointer of its parent class, we can determine if current class + // has a custom implementation of it, therefore it's safe to swizzle it. SEL selector = NSSelectorFromString(@"loadView"); - IMP viewControllerImp = class_getMethodImplementation([UIViewController class], selector); + IMP viewControllerImp = class_getMethodImplementation([class superclass], selector); IMP classLoadViewImp = class_getMethodImplementation(class, selector); if (viewControllerImp == classLoadViewImp) { return; From a8f7a1bd76d9d75ac58aff7fddd44ccd836a5239 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 17 Jun 2024 13:16:41 +0200 Subject: [PATCH 10/32] feat: Pause and resume AppHangTracking API (#4077) Add two methods pauseAppHangTracking and resumeAppHangTracking to ignore reported AppHangs. Fixes GH-3472 --- CHANGELOG.md | 4 ++ .../iOS-Swift/Base.lproj/Main.storyboard | 21 +++++++--- .../iOS-Swift/ExtraViewController.swift | 15 +++++++ Sources/Sentry/Public/SentrySDK.h | 13 ++++++ Sources/Sentry/SentryANRTrackingIntegration.m | 17 ++++++++ Sources/Sentry/SentryHub.m | 12 ++++++ Sources/Sentry/SentrySDK.m | 19 +++++++++ .../include/SentryANRTrackingIntegration.h | 3 ++ Sources/Sentry/include/SentryHub+Private.h | 2 + .../SentryANRTrackingIntegrationTests.swift | 41 +++++++++++++++++++ Tests/SentryTests/SentryHubTests.swift | 18 ++++++++ Tests/SentryTests/SentrySDKTests.swift | 41 +++++++++++++++++++ 12 files changed, 200 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15cb85bbd4e..ff5b89fd841 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add pause and resume AppHangTracking API (#4077). You can now pause and resume app hang tracking with `SentrySDK.pauseAppHangTracking()` and `SentrySDK.resumeAppHangTracking()`. + ### Fixes - Fix potential deadlock in app hang detection (#4063) diff --git a/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard b/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard index 09e22eaf5c7..016a19ef88c 100644 --- a/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard +++ b/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard @@ -3,7 +3,7 @@ - + @@ -943,7 +943,7 @@ +