Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Reporting crashes when restarting the SDK #2440

Merged
merged 42 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
a32dffd
fix: Reporting crashes when restarting the SDK
philipphofmann Nov 23, 2022
a3cddd7
More fixes and test buttons
philipphofmann Nov 24, 2022
82ac290
fixes and tests
philipphofmann Nov 24, 2022
27683ea
Fix iOS 13 sample
philipphofmann Nov 24, 2022
737463a
Undo signing changes
philipphofmann Nov 24, 2022
1c64a36
Fix tests
philipphofmann Nov 24, 2022
809e967
Comments
philipphofmann Nov 24, 2022
1f8b2a5
Clear reserved threads
philipphofmann Nov 25, 2022
1230444
Check if threads are cleared
philipphofmann Nov 25, 2022
d5263e5
Unsubscribe from NSNotificationCenter
philipphofmann Nov 25, 2022
c1a43b3
Replace with TestNotificationCenter
philipphofmann Nov 25, 2022
6658e27
comment out notifications
philipphofmann Nov 25, 2022
53ba5a9
Add notifications again
philipphofmann Nov 25, 2022
20d2f3b
Assert removeObserver
philipphofmann Nov 25, 2022
a20c27a
Disable set monitoring
philipphofmann Nov 25, 2022
1fb6f10
Fix notification assertions
philipphofmann Nov 25, 2022
f70824a
Fix
philipphofmann Nov 25, 2022
6a7e86b
Disable CPPException
philipphofmann Nov 28, 2022
3ab8372
Disable AppState
philipphofmann Nov 28, 2022
60b7ef4
Disable Mach
philipphofmann Nov 28, 2022
4f81525
Disable NSException
philipphofmann Nov 28, 2022
eb4ebd6
Disable Signal
philipphofmann Nov 28, 2022
7405e8d
Disable zombie
philipphofmann Nov 28, 2022
bc91046
Fix setting enabled flag
philipphofmann Nov 28, 2022
1016fc6
Set reserved threads to 2
philipphofmann Nov 28, 2022
295e1cb
Don't disable options
philipphofmann Nov 28, 2022
b372194
Don't disable options
philipphofmann Nov 28, 2022
6210dbf
Refactor
philipphofmann Nov 28, 2022
aa93f19
Undo fix in SentrySDK
philipphofmann Nov 28, 2022
91b01cb
Undo changes
philipphofmann Nov 28, 2022
876ada9
Fix SentryCrashInstallationTests
philipphofmann Nov 28, 2022
541f127
Undo changes
philipphofmann Nov 28, 2022
23a00db
Remove crash logger file
philipphofmann Nov 28, 2022
04944ca
Fix clear
philipphofmann Nov 28, 2022
aadecc5
Move reserved threads to mach
philipphofmann Nov 29, 2022
cf58530
Fix tests
philipphofmann Nov 29, 2022
798179f
One more fix
philipphofmann Nov 29, 2022
a603d73
Undo mach setenabled
philipphofmann Nov 29, 2022
d86e7a0
Undo mach while enabled
philipphofmann Nov 29, 2022
e123106
Don't uninstall mach
philipphofmann Nov 29, 2022
5a9358b
Merge branch 'master' into fix/no-crash-reports-restarting-sdk
philipphofmann Nov 29, 2022
61948d6
comment
philipphofmann Nov 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Reporting crashes when restarting the SDK (#2440)

## 7.31.2

### Fixes
Expand Down
10 changes: 7 additions & 3 deletions Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
var window: UIWindow?

static let defaultDSN = "https://[email protected]/5428557"

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {


static func startSentry() {
// For testing purposes, we want to be able to change the DSN and store it to disk. In a real app, you shouldn't need this behavior.
let dsn = DSNStorage.shared.getDSN() ?? AppDelegate.defaultDSN
DSNStorage.shared.saveDSN(dsn: dsn)
Expand Down Expand Up @@ -45,6 +44,11 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
let httpStatusCodeRange = HttpStatusCodeRange(min: 400, max: 599)
options.failedRequestStatusCodes = [ httpStatusCodeRange ]
}
}

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {

AppDelegate.startSentry()

if #available(iOS 14.0, *) {
metricKit.receiveReports()
Expand Down
62 changes: 38 additions & 24 deletions Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions Samples/iOS-Swift/iOS-Swift/ViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,11 @@ class ViewController: UIViewController {
SentrySDK.flush(timeout: 5)
}

@IBAction func close(_ sender: Any) {
SentrySDK.close()
}

@IBAction func startSDK(_ sender: Any) {
AppDelegate.startSentry()
}
}
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@
7B6D98EB24C6E84F005502FA /* SentryCrashInstallationReporterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B6D98EA24C6E84F005502FA /* SentryCrashInstallationReporterTests.swift */; };
7B6D98ED24C703F8005502FA /* Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B6D98EC24C703F8005502FA /* Async.swift */; };
7B72D23A28D074BC0014798A /* TestExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B72D23928D074BC0014798A /* TestExtensions.swift */; };
7B7725D8292F5DC20015BBF9 /* SentryCrashInstallationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 7B7725D7292F5DC20015BBF9 /* SentryCrashInstallationTests.m */; };
7B77BE3527EC8445003C9020 /* SentryDiscardReasonMapper.h in Headers */ = {isa = PBXBuildFile; fileRef = 7B77BE3427EC8445003C9020 /* SentryDiscardReasonMapper.h */; };
7B77BE3727EC8460003C9020 /* SentryDiscardReasonMapper.m in Sources */ = {isa = PBXBuildFile; fileRef = 7B77BE3627EC8460003C9020 /* SentryDiscardReasonMapper.m */; };
7B7A30C624B48321005A4C6E /* SentryCrashWrapper.h in Headers */ = {isa = PBXBuildFile; fileRef = 7B7A30C524B48321005A4C6E /* SentryCrashWrapper.h */; };
Expand Down Expand Up @@ -1137,6 +1138,7 @@
7B6D98EA24C6E84F005502FA /* SentryCrashInstallationReporterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashInstallationReporterTests.swift; sourceTree = "<group>"; };
7B6D98EC24C703F8005502FA /* Async.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Async.swift; sourceTree = "<group>"; };
7B72D23928D074BC0014798A /* TestExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestExtensions.swift; sourceTree = "<group>"; };
7B7725D7292F5DC20015BBF9 /* SentryCrashInstallationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryCrashInstallationTests.m; sourceTree = "<group>"; };
7B77BE3427EC8445003C9020 /* SentryDiscardReasonMapper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDiscardReasonMapper.h; path = include/SentryDiscardReasonMapper.h; sourceTree = "<group>"; };
7B77BE3627EC8460003C9020 /* SentryDiscardReasonMapper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDiscardReasonMapper.m; sourceTree = "<group>"; };
7B7A30C524B48321005A4C6E /* SentryCrashWrapper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryCrashWrapper.h; path = include/SentryCrashWrapper.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2208,6 +2210,7 @@
63FE71F120DA66EA00CDBAE8 /* Container+DeepSearch_Tests.m */,
63FE71F620DA66EB00CDBAE8 /* FileBasedTestCase.h */,
63FE71D920DA66E700CDBAE8 /* FileBasedTestCase.m */,
7B7725D7292F5DC20015BBF9 /* SentryCrashInstallationTests.m */,
D855AD61286ED6A4002573E1 /* SentryCrashTests.m */,
63FE71E220DA66E800CDBAE8 /* NSError+SimpleConstructor_Tests.m */,
63FE71D520DA66E600CDBAE8 /* RFC3339UTFString_Tests.m */,
Expand Down Expand Up @@ -3678,6 +3681,7 @@
7BED3576266F7BFF00EAA70D /* TestSentryCrashWrapper.m in Sources */,
7BC6EC18255C44540059822A /* SentryDebugMetaTests.swift in Sources */,
A811D867248E2770008A41EA /* SentrySystemEventBreadcrumbsTest.swift in Sources */,
7B7725D8292F5DC20015BBF9 /* SentryCrashInstallationTests.m in Sources */,
7B82D54924E2A2D400EE670F /* SentryIdTests.swift in Sources */,
7BD47B4E268F0B470076A663 /* ClearTestState.swift in Sources */,
7B6D98ED24C703F8005502FA /* Async.swift in Sources */,
Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/SentryCrashIntegration.m
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,12 @@ + (void)sendAllSentryCrashReports
- (void)uninstall
{
if (nil != installation) {
[self.crashAdapter close];
[installation uninstall];
installationToken = 0;
}

[self.crashAdapter uninstallAsyncHooks];

[NSNotificationCenter.defaultCenter removeObserver:self
name:NSCurrentLocaleDidChangeNotification
object:nil];
Expand Down
9 changes: 1 addition & 8 deletions Sources/Sentry/SentryCrashWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,9 @@ - (void)installAsyncHooks
sentrycrash_install_async_hooks();
}

- (void)close
- (void)uninstallAsyncHooks
{
SentryCrash *handler = [SentryCrash sharedInstance];
@synchronized(handler) {
[handler setMonitoring:SentryCrashMonitorTypeNone];
handler.onCrash = NULL;
}

sentrycrash_deactivate_async_hooks();
sentrycrashccd_close();
}

- (NSDictionary *)systemInfo
Expand Down
6 changes: 1 addition & 5 deletions Sources/Sentry/include/SentryCrashWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ SENTRY_NO_INIT

- (void)installAsyncHooks;

/**
* It's not really possible to close SentryCrash. Best we can do is to deactivate all the monitors,
* clear the `onCrash` callback installed on the global handler, and a few more minor things.
*/
- (void)close;
- (void)uninstallAsyncHooks;

- (NSDictionary *)systemInfo;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@
IMPLEMENT_REPORT_VALUE_PROPERTY(NAME, NAMEUPPER, TYPE) \
IMPLEMENT_REPORT_KEY_PROPERTY(NAME, NAMEUPPER)

typedef struct {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken from SentryCrashInstallation.m for testing

const char *key;
const char *value;
} ReportField;

typedef struct {
SentryCrashReportWriteCallback userCrashCallback;
int reportFieldsCount;
ReportField *reportFields[0];
} CrashHandlerData;

@interface
SentryCrashInstallation ()

Expand Down Expand Up @@ -84,4 +95,7 @@ SentryCrashInstallation ()
/** Make an absolute key paths from the specified paths. */
- (NSArray *)makeKeyPaths:(NSArray *)keyPaths;

/** Only needed for testing */
- (CrashHandlerData *)g_crashHandlerData;

@end
2 changes: 2 additions & 0 deletions Sources/SentryCrash/Installations/SentryCrashInstallation.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
*/
- (void)install;

- (void)uninstall;

/** Convenience method to call -[SentryCrash sendAllReportsWithCompletion:].
* This method will set the SentryCrash sink and then send all outstanding
* reports.
Expand Down
28 changes: 17 additions & 11 deletions Sources/SentryCrash/Installations/SentryCrashInstallation.m
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,6 @@
/** Max number of properties that can be defined for writing to the report */
#define kMaxProperties 500

typedef struct {
const char *key;
const char *value;
} ReportField;

typedef struct {
SentryCrashReportWriteCallback userCrashCallback;
int reportFieldsCount;
ReportField *reportFields[0];
} CrashHandlerData;

static CrashHandlerData *g_crashHandlerData;

static void
Expand Down Expand Up @@ -199,6 +188,11 @@ - (CrashHandlerData *)crashHandlerData
return (CrashHandlerData *)self.crashHandlerDataBacking.mutableBytes;
}

- (CrashHandlerData *)g_crashHandlerData
{
return g_crashHandlerData;
}

- (SentryCrashInstReportField *)reportFieldForProperty:(NSString *)propertyName
{
SentryCrashInstReportField *field = [self.fields objectForKey:propertyName];
Expand Down Expand Up @@ -298,6 +292,18 @@ - (void)install
}
}

- (void)uninstall
{
SentryCrash *handler = [SentryCrash sharedInstance];
@synchronized(handler) {
if (g_crashHandlerData == self.crashHandlerData) {
g_crashHandlerData = NULL;
handler.onCrash = NULL;
}
[handler uninstall];
}
}

- (void)sendAllReportsWithCompletion:(SentryCrashReportFilterCompletion)onCompletion
{
NSError *error = [self validateProperties];
Expand Down
8 changes: 7 additions & 1 deletion Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,17 @@ addContextualInfoToEvent(Monitor *monitor, struct SentryCrash_MonitorContext *ev
}

void
sentrycrashcm_setEventCallback(void (*onEvent)(struct SentryCrash_MonitorContext *monitorContext))
sentrycrashcm_setEventCallback(SentryCrashMonitorEventCallback onEvent)
{
g_onExceptionEvent = onEvent;
}

SentryCrashMonitorEventCallback
sentrycrashcm_getEventCallback(void)
{
return g_onExceptionEvent;
}

void
sentrycrashcm_setActiveMonitors(SentryCrashMonitorType monitorTypes)
{
Expand Down
10 changes: 8 additions & 2 deletions Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,18 @@ void sentrycrashcm_setActiveMonitors(SentryCrashMonitorType monitorTypes);
*/
SentryCrashMonitorType sentrycrashcm_getActiveMonitors(void);

typedef void (*SentryCrashMonitorEventCallback)(struct SentryCrash_MonitorContext *);

/** Set the callback to call when an event is captured.
*
* @param onEvent Called whenever an event is captured.
*/
void sentrycrashcm_setEventCallback(
void (*onEvent)(struct SentryCrash_MonitorContext *monitorContext));
void sentrycrashcm_setEventCallback(SentryCrashMonitorEventCallback onEvent);

/** Get the current SentryCrashMonitorEventCallback. Only needed for testing.
*
*/
SentryCrashMonitorEventCallback sentrycrashcm_getEventCallback(void);

// ============================================================================
#pragma mark - Internal API -
Expand Down
6 changes: 6 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrash.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ static NSString *const SENTRYCRASH_REPORT_ATTACHMENTS_ITEM = @"attachments";
*/
- (BOOL)install;

/**
* Its not really possible to uninstall SentryCrash. Best we can do is to deactivate all the
* monitors and clear the `onCrash` callback installed on the global handler.
*/
- (void)uninstall;

/** Send all outstanding crash reports to the current sink.
* It will only attempt to send the most recent 5 reports. All others will be
* deleted. Once the reports are successfully sent to the server, they may be
Expand Down
15 changes: 15 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrash.m
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

@property (nonatomic, readwrite, retain) NSString *bundleName;
@property (nonatomic, readwrite, retain) NSString *basePath;
@property (nonatomic, readwrite, assign) SentryCrashMonitorType monitoringWhenUninstalled;

@end

Expand Down Expand Up @@ -269,6 +270,12 @@ - (NSDictionary *)systemInfo

- (BOOL)install
{
// Restore previous monitors when uninstall was called previously
if (self.monitoringWhenUninstalled != SentryCrashMonitorTypeNone) {
[self setMonitoring:self.monitoringWhenUninstalled];
self.monitoringWhenUninstalled = SentryCrashMonitorTypeNone;
}

_monitoring = sentrycrash_install(self.bundleName.UTF8String, self.basePath.UTF8String);
if (self.monitoring == 0) {
return false;
Expand Down Expand Up @@ -320,6 +327,14 @@ - (BOOL)install
return true;
}

- (void)uninstall
{
self.monitoringWhenUninstalled = self.monitoring;
[self setMonitoring:SentryCrashMonitorTypeNone];
self.onCrash = NULL;
sentrycrash_uninstall();
}

- (void)sendAllReportsWithCompletion:(SentryCrashReportFilterCompletion)onCompletion
{
NSArray *reports = [self allReports];
Expand Down
8 changes: 8 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashC.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ sentrycrash_install(const char *appName, const char *const installPath)
return monitors;
}

void
sentrycrash_uninstall(void)
{
sentrycrashcm_setEventCallback(NULL);
g_installed = 0;
sentrycrashccd_close();
}

SentryCrashMonitorType
sentrycrash_setMonitoring(SentryCrashMonitorType monitors)
{
Expand Down
2 changes: 2 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashC.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ extern "C" {
*/
SentryCrashMonitorType sentrycrash_install(const char *appName, const char *const installPath);

void sentrycrash_uninstall(void);

/** Set the crash types that will be handled.
* Some crash types may not be enabled depending on circumstances (e.g. running
* in a debugger).
Expand Down
6 changes: 6 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashCachedData.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ sentrycrashccd_close()
}
}

bool
sentrycrashccd_hasThreadStarted(void)
{
return g_hasThreadStarted;
}

void
sentrycrashccd_freeze()
{
Expand Down
1 change: 1 addition & 0 deletions Sources/SentryCrash/Recording/SentryCrashCachedData.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

void sentrycrashccd_init(int pollingIntervalInSeconds);
void sentrycrashccd_close(void);
bool sentrycrashccd_hasThreadStarted(void);

void sentrycrashccd_freeze(void);
void sentrycrashccd_unfreeze(void);
Expand Down
Loading