Skip to content

Commit

Permalink
feat: Core data operation in the main thread (#2879)
Browse files Browse the repository at this point in the history
Adding extra information to a core data span when running in the main thread
  • Loading branch information
brustolin committed Apr 13, 2023
1 parent a6f8b18 commit 4d68229
Show file tree
Hide file tree
Showing 17 changed files with 180 additions and 52 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- feat: Core data operation in the main thread (#2879)

### Fixes

- Crash when serializing invalid objects (#2858)
Expand Down
13 changes: 2 additions & 11 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,9 @@ - (instancetype)initWithOptions:(SentryOptions *)options
transportAdapter:(SentryTransportAdapter *)transportAdapter

{
SentryInAppLogic *inAppLogic =
[[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes
inAppExcludes:options.inAppExcludes];
SentryCrashStackEntryMapper *crashStackEntryMapper =
[[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
SentryStacktraceBuilder *stacktraceBuilder =
[[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];
id<SentryCrashMachineContextWrapper> machineContextWrapper =
[[SentryCrashDefaultMachineContextWrapper alloc] init];
SentryThreadInspector *threadInspector =
[[SentryThreadInspector alloc] initWithStacktraceBuilder:stacktraceBuilder
andMachineContextWrapper:machineContextWrapper];
[[SentryThreadInspector alloc] initWithOptions:options];

SentryExtraContextProvider *extraContextProvider = [SentryExtraContextProvider sharedInstance];

return [self initWithOptions:options
Expand Down
30 changes: 28 additions & 2 deletions Sources/Sentry/SentryCoreDataTracker.m
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@

#import "SentryCoreDataTracker.h"
#import "SentryFrame.h"
#import "SentryHub+Private.h"
#import "SentryInternalDefines.h"
#import "SentryLog.h"
#import "SentryNSProcessInfoWrapper.h"
#import "SentryPredicateDescriptor.h"
#import "SentrySDK+Private.h"
#import "SentryScope+Private.h"
#import "SentrySpanProtocol.h"
#import "SentryStacktrace.h"
#import "SentryThreadInspector.h"

@implementation SentryCoreDataTracker {
SentryPredicateDescriptor *predicateDescriptor;
SentryThreadInspector *_threadInspector;
SentryNSProcessInfoWrapper *_processInfoWrapper;
}

- (instancetype)init
- (instancetype)initWithThreadInspector:(SentryThreadInspector *)threadInspector
processInfoWrapper:(SentryNSProcessInfoWrapper *)processInfoWrapper;
{
if (self = [super init]) {
predicateDescriptor = [[SentryPredicateDescriptor alloc] init];
_threadInspector = threadInspector;
_processInfoWrapper = processInfoWrapper;
}
return self;
}
Expand Down Expand Up @@ -47,8 +57,9 @@ - (NSArray *)managedObjectContext:(NSManagedObjectContext *)context
NSArray *result = original(request, error);

if (fetchSpan) {
[fetchSpan setDataValue:[NSNumber numberWithInteger:result.count] forKey:@"read_count"];
[self mainThreadExtraInfo:fetchSpan];

[fetchSpan setDataValue:[NSNumber numberWithInteger:result.count] forKey:@"read_count"];
[fetchSpan
finishWithStatus:result == nil ? kSentrySpanStatusInternalError : kSentrySpanStatusOk];

Expand Down Expand Up @@ -91,6 +102,7 @@ - (BOOL)managedObjectContext:(NSManagedObjectContext *)context
BOOL result = original(error);

if (fetchSpan) {
[self mainThreadExtraInfo:fetchSpan];
[fetchSpan finishWithStatus:result ? kSentrySpanStatusOk : kSentrySpanStatusInternalError];

SENTRY_LOG_DEBUG(@"SentryCoreDataTracker automatically finished span with status: %@",
Expand All @@ -100,6 +112,20 @@ - (BOOL)managedObjectContext:(NSManagedObjectContext *)context
return result;
}

- (void)mainThreadExtraInfo:(SentrySpan *)span
{
BOOL isMainThread = [NSThread isMainThread];

[span setDataValue:@(isMainThread) forKey:BLOCKED_MAIN_THREAD];

if (!isMainThread) {
return;
}

SentryStacktrace *stackTrace = [_threadInspector stacktraceForCurrentThreadAsyncUnsafe];
[span setFrames:stackTrace.frames];
}

- (NSString *)descriptionForOperations:
(NSDictionary<NSString *, NSDictionary<NSString *, NSNumber *> *> *)operations
inContext:(NSManagedObjectContext *)context
Expand Down
7 changes: 6 additions & 1 deletion Sources/Sentry/SentryCoreDataTrackingIntegration.m
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
#import "SentryCoreDataTrackingIntegration.h"
#import "SentryCoreDataSwizzling.h"
#import "SentryCoreDataTracker.h"
#import "SentryDependencyContainer.h"
#import "SentryLog.h"
#import "SentryNSDataSwizzling.h"
#import "SentryNSProcessInfoWrapper.h"
#import "SentryOptions.h"
#import "SentryThreadInspector.h"

@interface
SentryCoreDataTrackingIntegration ()
Expand All @@ -20,7 +23,9 @@ - (BOOL)installWithOptions:(SentryOptions *)options
return NO;
}

self.tracker = [[SentryCoreDataTracker alloc] init];
self.tracker = [[SentryCoreDataTracker alloc]
initWithThreadInspector:[[SentryThreadInspector alloc] initWithOptions:options]
processInfoWrapper:[SentryDependencyContainer.sharedInstance processInfoWrapper]];
[SentryCoreDataSwizzling.sharedInstance startWithMiddleware:self.tracker];

return YES;
Expand Down
13 changes: 13 additions & 0 deletions Sources/Sentry/SentryDependencyContainer.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "SentryANRTracker.h"
#import "SentryDefaultCurrentDateProvider.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryNSProcessInfoWrapper.h"
#import "SentryUIApplication.h"
#import <SentryAppStateManager.h>
#import <SentryClient+Private.h>
Expand Down Expand Up @@ -229,6 +230,18 @@ - (SentryMXManager *)metricKitManager
return _metricKitManager;
}

- (SentryNSProcessInfoWrapper *)processInfoWrapper
{
if (_processInfoWrapper == nil) {
@synchronized(sentryDependencyContainerLock) {
if (_processInfoWrapper == nil) {
_processInfoWrapper = [[SentryNSProcessInfoWrapper alloc] init];
}
}
}
return _processInfoWrapper;
}

#endif

@end
8 changes: 5 additions & 3 deletions Sources/Sentry/SentryExtraContextProvider.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "SentryExtraContextProvider.h"
#import "SentryCrashIntegration.h"
#import "SentryCrashWrapper.h"
#import "SentryDependencyContainer.h"
#import "SentryNSProcessInfoWrapper.h"
#import "SentryUIDeviceWrapper.h"

Expand All @@ -25,9 +26,10 @@ + (instancetype)sharedInstance

- (instancetype)init
{
return [self initWithCrashWrapper:[SentryCrashWrapper sharedInstance]
deviceWrapper:[[SentryUIDeviceWrapper alloc] init]
processInfoWrapper:[[SentryNSProcessInfoWrapper alloc] init]];
return
[self initWithCrashWrapper:[SentryCrashWrapper sharedInstance]
deviceWrapper:[[SentryUIDeviceWrapper alloc] init]
processInfoWrapper:[SentryDependencyContainer.sharedInstance processInfoWrapper]];
}

- (instancetype)initWithCrashWrapper:(id)crashWrapper
Expand Down
20 changes: 3 additions & 17 deletions Sources/Sentry/SentryNSDataSwizzling.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#import "SentryCrashDefaultMachineContextWrapper.h"
#import "SentryCrashMachineContextWrapper.h"
#import "SentryCrashStackEntryMapper.h"
#import "SentryDependencyContainer.h"
#import "SentryInAppLogic.h"
#import "SentryNSDataTracker.h"
#import "SentryNSProcessInfoWrapper.h"
Expand Down Expand Up @@ -32,8 +33,8 @@ + (SentryNSDataSwizzling *)shared
- (void)startWithOptions:(SentryOptions *)options
{
self.dataTracker = [[SentryNSDataTracker alloc]
initWithThreadInspector:[self buildThreadInspectorForOptions:options]
processInfoWrapper:[[SentryNSProcessInfoWrapper alloc] init]];
initWithThreadInspector:[[SentryThreadInspector alloc] initWithOptions:options]
processInfoWrapper:[SentryDependencyContainer.sharedInstance processInfoWrapper]];
[self.dataTracker enable];
[SentryNSDataSwizzling swizzleNSData];
}
Expand All @@ -43,21 +44,6 @@ - (void)stop
[self.dataTracker disable];
}

- (SentryThreadInspector *)buildThreadInspectorForOptions:(SentryOptions *)options
{
SentryInAppLogic *inAppLogic =
[[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes
inAppExcludes:options.inAppExcludes];
SentryCrashStackEntryMapper *crashStackEntryMapper =
[[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
SentryStacktraceBuilder *stacktraceBuilder =
[[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];
id<SentryCrashMachineContextWrapper> machineContextWrapper =
[[SentryCrashDefaultMachineContextWrapper alloc] init];
return [[SentryThreadInspector alloc] initWithStacktraceBuilder:stacktraceBuilder
andMachineContextWrapper:machineContextWrapper];
}

// SentrySwizzleInstanceMethod declaration shadows a local variable. The swizzling is working
// fine and we accept this warning.
#pragma clang diagnostic push
Expand Down
5 changes: 3 additions & 2 deletions Sources/Sentry/SentryNSDataTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#import "SentryFileManager.h"
#import "SentryFrame.h"
#import "SentryHub+Private.h"
#import "SentryInternalDefines.h"
#import "SentryLog.h"
#import "SentryNSProcessInfoWrapper.h"
#import "SentryOptions.h"
Expand Down Expand Up @@ -187,7 +188,7 @@ - (void)mainThreadExtraInfo:(id<SentrySpan>)span
{
BOOL isMainThread = [NSThread isMainThread];

[span setDataValue:@(isMainThread) forKey:@"blocked_main_thread"];
[span setDataValue:@(isMainThread) forKey:BLOCKED_MAIN_THREAD];

if (!isMainThread) {
return;
Expand All @@ -207,7 +208,7 @@ - (void)mainThreadExtraInfo:(id<SentrySpan>)span
// and only the 'main' frame remains in the stack
// therefore, there is nothing to do about it
// and we should not report it as an issue.
[span setDataValue:@(NO) forKey:@"blocked_main_thread"];
[span setDataValue:@(NO) forKey:BLOCKED_MAIN_THREAD];
} else {
[((SentrySpan *)span) setFrames:frames];
}
Expand Down
8 changes: 8 additions & 0 deletions Sources/Sentry/SentryNSProcessInfoWrapper.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

@implementation SentryNSProcessInfoWrapper

+ (SentryNSProcessInfoWrapper *)shared
{
static SentryNSProcessInfoWrapper *instance = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{ instance = [[self alloc] init]; });
return instance;
}

- (NSString *)processDirectoryPath
{
return NSBundle.mainBundle.bundlePath;
Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/SentryPerformanceTrackingIntegration.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "SentryPerformanceTrackingIntegration.h"
#import "SentryDefaultObjCRuntimeWrapper.h"
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryLog.h"
#import "SentryNSProcessInfoWrapper.h"
Expand Down Expand Up @@ -40,7 +41,7 @@ - (BOOL)installWithOptions:(SentryOptions *)options
dispatchQueue:dispatchQueue
objcRuntimeWrapper:[SentryDefaultObjCRuntimeWrapper sharedInstance]
subClassFinder:subClassFinder
processInfoWrapper:[[SentryNSProcessInfoWrapper alloc] init]];
processInfoWrapper:[SentryDependencyContainer.sharedInstance processInfoWrapper]];

[self.swizzling start];
SentryUIViewControllerPerformanceTracker.shared.enableWaitForFullDisplay
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ - (void)startMetricProfiler
_gCurrentSystemWrapper = [[SentrySystemWrapper alloc] init];
}
if (_gCurrentProcessInfoWrapper == nil) {
_gCurrentProcessInfoWrapper = [[SentryNSProcessInfoWrapper alloc] init];
_gCurrentProcessInfoWrapper = [SentryDependencyContainer.sharedInstance processInfoWrapper];
}
if (_gCurrentTimerWrapper == nil) {
_gCurrentTimerWrapper = [[SentryNSTimerWrapper alloc] init];
Expand Down
19 changes: 19 additions & 0 deletions Sources/Sentry/SentryThreadInspector.m
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
#import "SentryThreadInspector.h"
#import "SentryCrashDefaultMachineContextWrapper.h"
#import "SentryCrashStackCursor.h"
#include "SentryCrashStackCursor_MachineContext.h"
#import "SentryCrashStackEntryMapper.h"
#include "SentryCrashSymbolicator.h"
#import "SentryFrame.h"
#import "SentryInAppLogic.h"
#import "SentryOptions.h"
#import "SentryStacktrace.h"
#import "SentryStacktraceBuilder.h"
#import "SentryThread.h"
Expand Down Expand Up @@ -59,6 +63,21 @@ - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
return self;
}

- (instancetype)initWithOptions:(SentryOptions *)options
{
SentryInAppLogic *inAppLogic =
[[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes
inAppExcludes:options.inAppExcludes];
SentryCrashStackEntryMapper *crashStackEntryMapper =
[[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
SentryStacktraceBuilder *stacktraceBuilder =
[[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];
id<SentryCrashMachineContextWrapper> machineContextWrapper =
[[SentryCrashDefaultMachineContextWrapper alloc] init];
return [self initWithStacktraceBuilder:stacktraceBuilder
andMachineContextWrapper:machineContextWrapper];
}

- (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
{
return [self.stacktraceBuilder buildStacktraceForCurrentThreadAsyncUnsafe];
Expand Down
6 changes: 6 additions & 0 deletions Sources/Sentry/include/SentryCoreDataTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ NS_ASSUME_NONNULL_BEGIN
static NSString *const SENTRY_COREDATA_FETCH_OPERATION = @"db.sql.query";
static NSString *const SENTRY_COREDATA_SAVE_OPERATION = @"db.sql.transaction";

@class SentryThreadInspector, SentryNSProcessInfoWrapper;

@interface SentryCoreDataTracker : NSObject <SentryCoreDataMiddleware>
SENTRY_NO_INIT

- (instancetype)initWithThreadInspector:(SentryThreadInspector *)threadInspector
processInfoWrapper:(SentryNSProcessInfoWrapper *)processInfoWrapper;

@end

Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/include/SentryDependencyContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

@class SentryAppStateManager, SentryCrashWrapper, SentryThreadWrapper, SentrySwizzleWrapper,
SentryDispatchQueueWrapper, SentryDebugImageProvider, SentryANRTracker,
SentryNSNotificationCenterWrapper, SentryMXManager;
SentryNSNotificationCenterWrapper, SentryMXManager, SentryNSProcessInfoWrapper;

#if SENTRY_HAS_UIKIT
@class SentryScreenshot, SentryUIApplication, SentryViewHierarchy;
Expand Down Expand Up @@ -32,6 +32,7 @@ SENTRY_NO_INIT
@property (nonatomic, strong) SentryNSNotificationCenterWrapper *notificationCenterWrapper;
@property (nonatomic, strong) SentryDebugImageProvider *debugImageProvider;
@property (nonatomic, strong) SentryANRTracker *anrTracker;
@property (nonatomic, strong) SentryNSProcessInfoWrapper *processInfoWrapper;

#if SENTRY_HAS_UIKIT
@property (nonatomic, strong) SentryScreenshot *screenshot;
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryInternalDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,5 @@ static NSString *const SentryDebugImageType = @"macho";
} \
(__cond_result); \
})

#define BLOCKED_MAIN_THREAD @"blocked_main_thread"
4 changes: 3 additions & 1 deletion Sources/Sentry/include/SentryThreadInspector.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#import "SentryDefines.h"
#import <Foundation/Foundation.h>

@class SentryThread, SentryStacktraceBuilder, SentryStacktrace;
@class SentryThread, SentryStacktraceBuilder, SentryStacktrace, SentryOptions;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -12,6 +12,8 @@ SENTRY_NO_INIT
- (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper;

- (instancetype)initWithOptions:(SentryOptions *)options;

- (nullable SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe;

/**
Expand Down

0 comments on commit 4d68229

Please sign in to comment.