Skip to content

Commit

Permalink
fix: Call UIDevice methods on the main thread (#2369)
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinrenskers authored Nov 9, 2022
1 parent f444dc4 commit 725723e
Show file tree
Hide file tree
Showing 17 changed files with 96 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Fix issue with invalid profiles uploading (#2358 and #2359)
- Call UIDevice methods on the main thread (#2369)

## 7.30.0

Expand Down
12 changes: 8 additions & 4 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
0A1B497328E597DD00D7BFA3 /* TestLogOutput.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A1B497228E597DD00D7BFA3 /* TestLogOutput.swift */; };
0A1C3592287D7107007D01E3 /* SentryMetaTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A1C3591287D7107007D01E3 /* SentryMetaTests.swift */; };
0A2690B72885C2E000E4432D /* TestSentryPermissionsObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0AABE2EF2885C2120057ED69 /* TestSentryPermissionsObserver.swift */; };
0A283E79291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A283E78291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift */; };
0A2D8D5B289815C0008720F6 /* SentryBaseIntegration.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A2D8D5A289815C0008720F6 /* SentryBaseIntegration.m */; };
0A2D8D5D289815EB008720F6 /* SentryBaseIntegration.h in Headers */ = {isa = PBXBuildFile; fileRef = 0A2D8D5C289815EB008720F6 /* SentryBaseIntegration.h */; };
0A2D8D8728992260008720F6 /* SentryBaseIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A2D8D8628992260008720F6 /* SentryBaseIntegrationTests.swift */; };
Expand Down Expand Up @@ -668,7 +669,7 @@
A8AFFCD42907E0CA00967CD7 /* SentryRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A8AFFCD32907E0CA00967CD7 /* SentryRequestTests.swift */; };
A8F17B2E2901765900990B25 /* SentryRequest.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B2D2901765900990B25 /* SentryRequest.m */; };
A8F17B342902870300990B25 /* SentryHttpStatusCodeRange.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B332902870300990B25 /* SentryHttpStatusCodeRange.m */; };
D8019910286B089000C277F0 /* SentryCrashReportSinkTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = D801990F286B089000C277F0 /* SentryCrashReportSinkTest.swift */; };
D8019910286B089000C277F0 /* SentryCrashReportSinkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D801990F286B089000C277F0 /* SentryCrashReportSinkTests.swift */; };
D808FB88281AB33C009A2A33 /* SentryUIEventTrackerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB86281AB31D009A2A33 /* SentryUIEventTrackerTests.swift */; };
D808FB8B281BCE96009A2A33 /* TestSentrySwizzleWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB89281BCE46009A2A33 /* TestSentrySwizzleWrapper.swift */; };
D808FB92281BF6EC009A2A33 /* SentryUIEventTrackingIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D808FB90281BF6E9009A2A33 /* SentryUIEventTrackingIntegrationTests.swift */; };
Expand Down Expand Up @@ -766,6 +767,7 @@
03F9D37B2819A65C00602916 /* SentryProfilerTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryProfilerTests.mm; sourceTree = "<group>"; };
0A1B497228E597DD00D7BFA3 /* TestLogOutput.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestLogOutput.swift; sourceTree = "<group>"; };
0A1C3591287D7107007D01E3 /* SentryMetaTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryMetaTests.swift; sourceTree = "<group>"; };
0A283E78291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIDeviceWrapperTests.swift; sourceTree = "<group>"; };
0A2D8D5A289815C0008720F6 /* SentryBaseIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBaseIntegration.m; sourceTree = "<group>"; };
0A2D8D5C289815EB008720F6 /* SentryBaseIntegration.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryBaseIntegration.h; path = include/SentryBaseIntegration.h; sourceTree = "<group>"; };
0A2D8D8628992260008720F6 /* SentryBaseIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBaseIntegrationTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1459,7 +1461,7 @@
A8AFFCD32907E0CA00967CD7 /* SentryRequestTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryRequestTests.swift; sourceTree = "<group>"; };
A8F17B2D2901765900990B25 /* SentryRequest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryRequest.m; sourceTree = "<group>"; };
A8F17B332902870300990B25 /* SentryHttpStatusCodeRange.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryHttpStatusCodeRange.m; sourceTree = "<group>"; };
D801990F286B089000C277F0 /* SentryCrashReportSinkTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashReportSinkTest.swift; sourceTree = "<group>"; };
D801990F286B089000C277F0 /* SentryCrashReportSinkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashReportSinkTests.swift; sourceTree = "<group>"; };
D808FB86281AB31D009A2A33 /* SentryUIEventTrackerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIEventTrackerTests.swift; sourceTree = "<group>"; };
D808FB89281BCE46009A2A33 /* TestSentrySwizzleWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestSentrySwizzleWrapper.swift; sourceTree = "<group>"; };
D808FB90281BF6E9009A2A33 /* SentryUIEventTrackingIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIEventTrackingIntegrationTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2227,7 +2229,8 @@
7B6D98EA24C6E84F005502FA /* SentryCrashInstallationReporterTests.swift */,
7B0A542D2521C62400A71716 /* SentryFrameRemoverTests.swift */,
7BBC827825DFD7D7005F1ED8 /* SentryInAppLogicTests.swift */,
D801990F286B089000C277F0 /* SentryCrashReportSinkTest.swift */,
D801990F286B089000C277F0 /* SentryCrashReportSinkTests.swift */,
0A283E78291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift */,
7BED3574266F7BC600EAA70D /* TestSentryCrashWrapper.h */,
7BED3575266F7BFF00EAA70D /* TestSentryCrashWrapper.m */,
0AABE2EF2885C2120057ED69 /* TestSentryPermissionsObserver.swift */,
Expand Down Expand Up @@ -3556,6 +3559,7 @@
7B3B473E25D6CEA500D01640 /* SentryNSErrorTests.swift in Sources */,
632331F62404FFA8008D91D6 /* SentryScopeTests.m in Sources */,
D808FB88281AB33C009A2A33 /* SentryUIEventTrackerTests.swift in Sources */,
0A283E79291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift in Sources */,
63FE720D20DA66EC00CDBAE8 /* NSError+SimpleConstructor_Tests.m in Sources */,
69BEE6F72620729E006DF9DF /* UrlSessionDelegateSpy.swift in Sources */,
035E73C827D56757005EEB11 /* SentryBacktraceTests.mm in Sources */,
Expand Down Expand Up @@ -3612,7 +3616,7 @@
7B6C5ED6264E62CA0010D138 /* SentryTransactionTests.swift in Sources */,
D81FDF12280EA1060045E0E4 /* SentryScreenShotTests.swift in Sources */,
7BE3C7772445E50A00A38442 /* TestCurrentDateProvider.swift in Sources */,
D8019910286B089000C277F0 /* SentryCrashReportSinkTest.swift in Sources */,
D8019910286B089000C277F0 /* SentryCrashReportSinkTests.swift in Sources */,
D885266427739D01001269FC /* SentryFileIOTrackingIntegrationTests.swift in Sources */,
7BBD18992449DE9D00427C76 /* TestRateLimits.swift in Sources */,
8E4A038625F76A7600000D77 /* TypeMapping.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentryANRTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ - (void)detectANRs

ticksSinceUiUpdate++;

[self.dispatchQueueWrapper dispatchOnMainQueue:^{
[self.dispatchQueueWrapper dispatchAsyncOnMainQueue:^{
ticksSinceUiUpdate = 0;

if (reported) {
Expand Down
11 changes: 10 additions & 1 deletion Sources/Sentry/SentryDispatchQueueWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ - (void)dispatchAsyncWithBlock:(void (^)(void))block
});
}

- (void)dispatchOnMainQueue:(void (^)(void))block
- (void)dispatchAsyncOnMainQueue:(void (^)(void))block
{
dispatch_async(dispatch_get_main_queue(), ^{
@autoreleasepool {
Expand All @@ -47,6 +47,15 @@ - (void)dispatchOnMainQueue:(void (^)(void))block
});
}

- (void)dispatchSyncOnMainQueue:(void (^)(void))block
{
if ([NSThread isMainThread]) {
block();
} else {
dispatch_sync(dispatch_get_main_queue(), block);
}
}

- (void)dispatchAfter:(NSTimeInterval)interval block:(dispatch_block_t)block
{
dispatch_time_t delta = (int64_t)(interval * NSEC_PER_SEC);
Expand Down
8 changes: 3 additions & 5 deletions Sources/Sentry/SentryScreenshot.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "SentryScreenshot.h"
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryUIApplication.h"

#if SENTRY_HAS_UIKIT
Expand All @@ -13,11 +14,8 @@ @implementation SentryScreenshot

void (^takeScreenShot)(void) = ^{ result = [self takeScreenshots]; };

if ([NSThread isMainThread]) {
takeScreenShot();
} else {
dispatch_sync(dispatch_get_main_queue(), takeScreenShot);
}
[[SentryDependencyContainer sharedInstance].dispatchQueueWrapper
dispatchSyncOnMainQueue:takeScreenShot];

return result;
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentrySubClassFinder.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ - (void)actOnSubclassesOfViewControllerInImage:(NSString *)imageName block:(void
}

free(classes);
[self.dispatchQueue dispatchOnMainQueue:^{
[self.dispatchQueue dispatchAsyncOnMainQueue:^{
for (NSString *className in classesToSwizzle) {
block(NSClassFromString(className));
}
Expand Down
51 changes: 35 additions & 16 deletions Sources/Sentry/SentryUIDeviceWrapper.m
Original file line number Diff line number Diff line change
@@ -1,43 +1,62 @@
#import "SentryUIDeviceWrapper.h"
#import "SentryDependencyContainer.h"
#import "SentryDispatchQueueWrapper.h"

NS_ASSUME_NONNULL_BEGIN

@interface
SentryUIDeviceWrapper ()
@property (nonatomic) BOOL cleanupDeviceOrientationNotifications;
@property (nonatomic) BOOL cleanupBatteryMonitoring;
@property (strong, nonatomic) SentryDispatchQueueWrapper *dispatchQueueWrapper;
@end

@implementation SentryUIDeviceWrapper

#if TARGET_OS_IOS

- (instancetype)init
{
return [self initWithDispatchQueueWrapper:[SentryDependencyContainer sharedInstance]
.dispatchQueueWrapper];
}

- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper
{
if (self = [super init]) {
// Needed to read the device orientation on demand
if (!UIDevice.currentDevice.isGeneratingDeviceOrientationNotifications) {
self.cleanupDeviceOrientationNotifications = YES;
[UIDevice.currentDevice beginGeneratingDeviceOrientationNotifications];
}
self.dispatchQueueWrapper = dispatchQueueWrapper;
[self.dispatchQueueWrapper dispatchSyncOnMainQueue:^{
// Needed to read the device orientation on demand
if (!UIDevice.currentDevice.isGeneratingDeviceOrientationNotifications) {
self.cleanupDeviceOrientationNotifications = YES;
[UIDevice.currentDevice beginGeneratingDeviceOrientationNotifications];
}

// Needed so we can read the battery level
if (!UIDevice.currentDevice.isBatteryMonitoringEnabled) {
self.cleanupBatteryMonitoring = YES;
UIDevice.currentDevice.batteryMonitoringEnabled = YES;
}
// Needed so we can read the battery level
if (!UIDevice.currentDevice.isBatteryMonitoringEnabled) {
self.cleanupBatteryMonitoring = YES;
UIDevice.currentDevice.batteryMonitoringEnabled = YES;
}
}];
}
return self;
}

- (void)stop
{
[self.dispatchQueueWrapper dispatchSyncOnMainQueue:^{
if (self.cleanupDeviceOrientationNotifications) {
[UIDevice.currentDevice endGeneratingDeviceOrientationNotifications];
}
if (self.cleanupBatteryMonitoring) {
UIDevice.currentDevice.batteryMonitoringEnabled = NO;
}
}];
}

- (void)dealloc
{
if (self.cleanupDeviceOrientationNotifications) {
[UIDevice.currentDevice endGeneratingDeviceOrientationNotifications];
}
if (self.cleanupBatteryMonitoring) {
UIDevice.currentDevice.batteryMonitoringEnabled = NO;
}
[self stop];
}

- (UIDeviceOrientation)orientation
Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/include/SentryDispatchQueueWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ NS_ASSUME_NONNULL_BEGIN

- (void)dispatchAsyncWithBlock:(void (^)(void))block;

- (void)dispatchOnMainQueue:(void (^)(void))block;
- (void)dispatchAsyncOnMainQueue:(void (^)(void))block;

- (void)dispatchSyncOnMainQueue:(void (^)(void))block;

- (void)dispatchAfter:(NSTimeInterval)interval block:(dispatch_block_t)block;

Expand Down
5 changes: 5 additions & 0 deletions Sources/Sentry/include/SentryUIDeviceWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@

NS_ASSUME_NONNULL_BEGIN

@class SentryDispatchQueueWrapper;

@interface SentryUIDeviceWrapper : NSObject

#if TARGET_OS_IOS
- (instancetype)init;
- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper;
- (void)stop;
- (UIDeviceOrientation)orientation;
- (BOOL)isBatteryMonitoringEnabled;
- (UIDeviceBatteryState)batteryState;
Expand Down
1 change: 0 additions & 1 deletion Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class SentryFileManagerTests: XCTestCase {
init() {
currentDateProvider = TestCurrentDateProvider()
dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
dispatchQueueWrapper.dispatchAfterExecutesBlock = true

eventIds = (0...(maxCacheItems + 10)).map { _ in SentryId() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class SentryUIEventTrackerTests: XCTestCase {
let hub = SentryHub(client: TestClient(options: Options()), andScope: nil)
let dispatchQueue = TestSentryDispatchQueueWrapper()
let button = UIButton()

init () {
dispatchQueue.blockBeforeMainBlock = { false }
}

func getSut() -> SentryUIEventTracker {
return SentryUIEventTracker(swizzleWrapper: swizzleWrapper, dispatchQueueWrapper: dispatchQueue, idleTimeout: 3.0)
Expand Down
2 changes: 0 additions & 2 deletions Tests/SentryTests/Networking/SentryHttpTransportTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ class SentryHttpTransportTests: XCTestCase {
]
clientReportEnvelope = SentryEnvelope(id: event.eventId, items: clientReportEnvelopeItems)
clientReportRequest = buildRequest(clientReportEnvelope)

dispatchQueueWrapper.dispatchAfterExecutesBlock = true
}

var sut: SentryHttpTransport {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import Foundation
class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {

var dispatchAsyncCalled = 0
var dispatchAfterExecutesBlock = false

override func dispatchAsync(_ block: @escaping () -> Void) {
dispatchAsyncCalled += 1
Expand All @@ -13,7 +12,14 @@ class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {
var blockOnMainInvocations = Invocations<() -> Void>()
var blockBeforeMainBlock: () -> Bool = { true }

override func dispatch(onMainQueue block: @escaping () -> Void) {
override func dispatchAsync(onMainQueue block: @escaping () -> Void) {
blockOnMainInvocations.record(block)
if blockBeforeMainBlock() {
block()
}
}

override func dispatchSync(onMainQueue block: @escaping () -> Void) {
blockOnMainInvocations.record(block)
if blockBeforeMainBlock() {
block()
Expand All @@ -23,7 +29,7 @@ class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {
var dispatchAfterInvocations = Invocations<(interval: TimeInterval, block: () -> Void)>()
override func dispatch(after interval: TimeInterval, block: @escaping () -> Void) {
dispatchAfterInvocations.record((interval, block))
if dispatchAfterExecutesBlock {
if blockBeforeMainBlock() {
block()
}
}
Expand Down
2 changes: 2 additions & 0 deletions Tests/SentryTests/Performance/SentryTracerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class SentryTracerTests: XCTestCase {
#endif

init() {
dispatchQueue.blockBeforeMainBlock = { false }

CurrentDate.setCurrentDateProvider(currentDateProvider)
appStart = currentDateProvider.date()
appStartEnd = appStart.addingTimeInterval(0.5)
Expand Down
14 changes: 14 additions & 0 deletions Tests/SentryTests/SentryCrash/SentryUIDeviceWrapperTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import XCTest

#if os(iOS)
class SentryUIDeviceWrapperTests: XCTestCase {
func testExecutesLogicViaDispatchQueue() {
let dispatchQueue = TestSentryDispatchQueueWrapper()
let sut = SentryUIDeviceWrapper(dispatchQueueWrapper: dispatchQueue)
XCTAssertEqual(dispatchQueue.blockOnMainInvocations.count, 1)

sut.stop()
XCTAssertEqual(dispatchQueue.blockOnMainInvocations.count, 2)
}
}
#endif
4 changes: 0 additions & 4 deletions Tests/SentryTests/SentryCrash/TestSentryUIDeviceWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ class TestSentryUIDeviceWrapper: SentryUIDeviceWrapper {
var internalBatteryLevel: Float = 0.6
var interalBatteryState = UIDevice.BatteryState.charging

override init() {
// Do nothing
}

override func orientation() -> UIDeviceOrientation {
return internalOrientation
}
Expand Down

0 comments on commit 725723e

Please sign in to comment.