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: Accessing UI API on bg thread in enrichScope #4245

Merged
merged 8 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This bug caused unhandled/crash events to have the unhandled property and mach i
- Missing mach info for crash reports (#4230)
- Crash reports not generated on visionOS (#4229)
- Don’t force cast to `NSComparisonPredicate` in TERNARY operator (#4232)
- Fix accessing UI API on bg thread in enrichScope (#4245)
- EXC_BAD_ACCESS in SentryMetricProfiler (#4242)
- Missing '#include <sys/_types/_ucontext64.h>' (#4244)

Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/SentryCrashWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#import "SentryCrashMonitor_AppState.h"
#import "SentryCrashMonitor_System.h"
#import "SentryScope.h"
#import "SentryUIDeviceWrapper.h"
#import <Foundation/Foundation.h>
#import <SentryCrashCachedData.h>
#import <SentryCrashDebug.h>
Expand Down Expand Up @@ -126,7 +127,8 @@ - (void)enrichScope:(SentryScope *)scope
// For MacCatalyst the UIDevice returns the current version of MacCatalyst and not the
// macOSVersion. Therefore we have to use NSProcessInfo.
#if SENTRY_HAS_UIKIT && !TARGET_OS_MACCATALYST
[osData setValue:[UIDevice currentDevice].systemVersion forKey:@"version"];
[osData setValue:[SentryDependencyContainer.sharedInstance.uiDeviceWrapper getSystemVersion]
forKey:@"version"];
#else
NSOperatingSystemVersion version = [NSProcessInfo processInfo].operatingSystemVersion;
NSString *systemVersion = [NSString stringWithFormat:@"%d.%d.%d", (int)version.majorVersion,
Expand Down
12 changes: 3 additions & 9 deletions Sources/Sentry/SentryDependencyContainer.m
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,10 @@ - (SentryNSNotificationCenterWrapper *)notificationCenterWrapper
}
}

#if TARGET_OS_IOS
#if SENTRY_HAS_UIKIT
- (SentryUIDeviceWrapper *)uiDeviceWrapper SENTRY_DISABLE_THREAD_SANITIZER(
"double-checked lock produce false alarms")
{
# if SENTRY_HAS_UIKIT
if (_uiDeviceWrapper == nil) {
@synchronized(sentryDependencyContainerLock) {
if (_uiDeviceWrapper == nil) {
Expand All @@ -211,14 +210,9 @@ - (SentryUIDeviceWrapper *)uiDeviceWrapper SENTRY_DISABLE_THREAD_SANITIZER(
}
}
return _uiDeviceWrapper;
# else
SENTRY_LOG_DEBUG(
@"SentryDependencyContainer.uiDeviceWrapper only works with UIKit enabled. Ensure you're "
@"using the right configuration of Sentry that links UIKit.");
return nil;
# endif // SENTRY_HAS_UIKIT
}
#endif // TARGET_OS_IOS

#endif // SENTRY_HAS_UIKIT

#if SENTRY_UIKIT_AVAILABLE
- (SentryScreenshot *)screenshot SENTRY_DISABLE_THREAD_SANITIZER(
Expand Down
21 changes: 13 additions & 8 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -218,28 +218,33 @@ + (void)startWithOptions:(SentryOptions *)options

SentryScope *scope
= options.initialScope([[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
// The Hub needs to be initialized with a client so that closing a session
// can happen.
SentryHub *hub = [[SentryHub alloc] initWithClient:newClient andScope:scope];
[SentrySDK setCurrentHub:hub];
SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);

SENTRY_LOG_DEBUG(@"Dispatching init work required to run on main thread.");
[SentryDependencyContainer.sharedInstance.dispatchQueueWrapper dispatchAsyncOnMainQueue:^{
SENTRY_LOG_DEBUG(@"SDK main thread init started...");

// The UIDeviceWrapper needs to start before the Hub, because the Hub
// enriches the scope, which calls the UIDeviceWrapper.
#if SENTRY_HAS_UIKIT
[SentryDependencyContainer.sharedInstance.uiDeviceWrapper start];
#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT

// The Hub needs to be initialized with a client so that closing a session
// can happen.
SentryHub *hub = [[SentryHub alloc] initWithClient:newClient andScope:scope];
[SentrySDK setCurrentHub:hub];

[SentryCrashWrapper.sharedInstance startBinaryImageCache];
[SentryDependencyContainer.sharedInstance.binaryImageCache start];

[SentrySDK installIntegrations];
#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
[SentryDependencyContainer.sharedInstance.uiDeviceWrapper start];
#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT

#if SENTRY_TARGET_PROFILING_SUPPORTED
sentry_manageTraceProfilerOnStartSDK(options, hub);
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
}];

SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);
}

+ (void)startWithConfigureOptions:(void (^)(SentryOptions *options))configureOptions
Expand Down
19 changes: 17 additions & 2 deletions Sources/Sentry/SentryUIDeviceWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"

#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
#if SENTRY_HAS_UIKIT

NS_ASSUME_NONNULL_BEGIN

@interface
SentryUIDeviceWrapper ()
@property (nonatomic) BOOL cleanupDeviceOrientationNotifications;
@property (nonatomic) BOOL cleanupBatteryMonitoring;
@property (nonatomic, copy) NSString *systemVersion;
@end

@implementation SentryUIDeviceWrapper

- (void)start
{
[SentryDependencyContainer.sharedInstance.dispatchQueueWrapper dispatchAsyncOnMainQueue:^{

# if TARGET_OS_IOS
if (!UIDevice.currentDevice.isGeneratingDeviceOrientationNotifications) {
self.cleanupDeviceOrientationNotifications = YES;
[UIDevice.currentDevice beginGeneratingDeviceOrientationNotifications];
Expand All @@ -27,11 +30,15 @@ - (void)start
self.cleanupBatteryMonitoring = YES;
UIDevice.currentDevice.batteryMonitoringEnabled = YES;
}
# endif

self.systemVersion = [UIDevice currentDevice].systemVersion;
}];
}

- (void)stop
{
# if TARGET_OS_IOS
BOOL needsCleanUp = self.cleanupDeviceOrientationNotifications;
BOOL needsDisablingBattery = self.cleanupBatteryMonitoring;
UIDevice *device = [UIDevice currentDevice];
Expand All @@ -43,13 +50,15 @@ - (void)stop
device.batteryMonitoringEnabled = NO;
}
}];
# endif // TARGET_OS_IOS
}

- (void)dealloc
{
[self stop];
}

# if TARGET_OS_IOS
- (UIDeviceOrientation)orientation
{
return (UIDeviceOrientation)[UIDevice currentDevice].orientation;
Expand All @@ -69,9 +78,15 @@ - (float)batteryLevel
{
return [UIDevice currentDevice].batteryLevel;
}
# endif // TARGET_OS_IOS

- (NSString *)getSystemVersion
{
return self.systemVersion;
}

@end

NS_ASSUME_NONNULL_END

#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT
#endif // SENTRY_HAS_UIKIT
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
@class SentryViewHierarchy;
#endif // SENTRY_UIKIT_AVAILABLE

#if TARGET_OS_IOS
#if SENTRY_HAS_UIKIT
@class SentryUIDeviceWrapper;
#endif // TARGET_OS_IOS

Expand Down Expand Up @@ -80,7 +80,7 @@ SENTRY_NO_INIT
@property (nonatomic, strong) SentryUIApplication *application;
#endif // SENTRY_UIKIT_AVAILABLE

#if TARGET_OS_IOS
#if SENTRY_HAS_UIKIT
@property (nonatomic, strong) SentryUIDeviceWrapper *uiDeviceWrapper;
#endif // TARGET_OS_IOS

Expand Down
11 changes: 7 additions & 4 deletions Sources/Sentry/include/SentryUIDeviceWrapper.h
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
#import "SentryDefines.h"

#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
#if SENTRY_HAS_UIKIT

# import <UIKit/UIKit.h>

NS_ASSUME_NONNULL_BEGIN

@class SentryDispatchQueueWrapper;

@interface SentryUIDeviceWrapper : NSObject

- (void)start;
- (void)stop;

# if TARGET_OS_IOS
- (UIDeviceOrientation)orientation;
- (BOOL)isBatteryMonitoringEnabled;
- (UIDeviceBatteryState)batteryState;
- (float)batteryLevel;
# endif // TARGET_OS_IOS

- (NSString *)getSystemVersion;

@end

NS_ASSUME_NONNULL_END

#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT
#endif // SENTRY_HAS_UIKIT
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ class SentryCrashIntegrationTests: NotificationCenterTestCase {
}

private func givenSutWithGlobalHubAndCrashWrapper() -> (SentryCrashIntegration, SentryHub) {
#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
SentryDependencyContainer.sharedInstance().uiDeviceWrapper.start()
#endif
let sut = fixture.getSut(crashWrapper: SentryCrashWrapper.sharedInstance())
let hub = fixture.hub
SentrySDK.setCurrentHub(hub)
Expand Down
19 changes: 19 additions & 0 deletions Tests/SentryTests/SentrySDKTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,25 @@ class SentrySDKTests: XCTestCase {
SentrySDK.close()
XCTAssertFalse(deviceWrapper.started)
}

/// Ensure to start the UIDeviceWrapper before initializing the hub, so enrich scope sets the correct OS version.
func testStartSDK_ScopeContextContainsOSVersion() throws {
let expectation = expectation(description: "MainThreadTestIntegration install called")
MainThreadTestIntegration.expectation = expectation

DispatchQueue.global(qos: .background).async {
SentrySDK.start { options in
options.integrations = [ NSStringFromClass(MainThreadTestIntegration.self) ]
}
}

wait(for: [expectation], timeout: 1.0)

let os = try XCTUnwrap (SentrySDK.currentHub().scope.contextDictionary["os"] as? [String: Any])
#if !targetEnvironment(macCatalyst)
XCTAssertEqual(UIDevice.current.systemVersion, os["version"] as? String)
#endif
}
#endif

func testResumeAndPauseAppHangTracking() {
Expand Down
Loading