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: Crash for Call Should be on Main Thread #1371

Merged
merged 10 commits into from
Oct 12, 2021
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- fix: Crash for Call Should be on Main Thread (#1371)

## 7.4.3

- fix: Crash for Custom ViewController init on iOS 15 (#1361)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
637AFDDC243B036D0034958B /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 637AFDDB243B036D0034958B /* main.m */; };
7B3427FB25876DDC00056519 /* Tongariro.jpg in Resources */ = {isa = PBXBuildFile; fileRef = 7B3427F925876DDB00056519 /* Tongariro.jpg */; };
7B64387A26A6C69F000D0F65 /* LaunchUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B64387926A6C69F000D0F65 /* LaunchUITests.swift */; };
7B8A5C9E2715B5DE008ACD3B /* InitializerViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 7B8A5C9D2715B5DE008ACD3B /* InitializerViewController.m */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -81,6 +82,8 @@
7B64387726A6C69F000D0F65 /* iOS-ObjectiveCUITests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "iOS-ObjectiveCUITests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
7B64387926A6C69F000D0F65 /* LaunchUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LaunchUITests.swift; sourceTree = "<group>"; };
7B64387B26A6C69F000D0F65 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
7B8A5C9C2715B5CF008ACD3B /* InitializerViewController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = InitializerViewController.h; sourceTree = "<group>"; };
7B8A5C9D2715B5DE008ACD3B /* InitializerViewController.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = InitializerViewController.m; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -146,6 +149,8 @@
637AFDCA243B036B0034958B /* AppDelegate.m */,
637AFDCF243B036B0034958B /* ViewController.h */,
637AFDD0243B036B0034958B /* ViewController.m */,
7B8A5C9C2715B5CF008ACD3B /* InitializerViewController.h */,
7B8A5C9D2715B5DE008ACD3B /* InitializerViewController.m */,
637AFDD2243B036B0034958B /* Main.storyboard */,
637AFDD5243B036D0034958B /* Assets.xcassets */,
7B3427F925876DDB00056519 /* Tongariro.jpg */,
Expand Down Expand Up @@ -293,6 +298,7 @@
buildActionMask = 2147483647;
files = (
637AFDD1243B036B0034958B /* ViewController.m in Sources */,
7B8A5C9E2715B5DE008ACD3B /* InitializerViewController.m in Sources */,
637AFDCB243B036B0034958B /* AppDelegate.m in Sources */,
637AFDDC243B036D0034958B /* main.m in Sources */,
);
Expand Down
11 changes: 11 additions & 0 deletions Samples/iOS-ObjectiveC/iOS-ObjectiveC/InitializerViewController.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#import <UIKit/UIKit.h>

/**
* An empty ViewController to ensure the swizzling of the SentrySDK doesn't call the initialize
* method from a background thread. The initializer method is called before the runtime sends its
* first message to the class, which is also the case when swizzling a class. For more information
* checkout SentryUIViewControllerSwizziling.
*/
@interface InitializerViewController : UIViewController

@end
11 changes: 11 additions & 0 deletions Samples/iOS-ObjectiveC/iOS-ObjectiveC/InitializerViewController.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#import "InitializerViewController.h"
#import <Foundation/Foundation.h>

@implementation InitializerViewController

+ (void)initialize
{
NSAssert([NSThread isMainThread], @"Initializer must only be called from the main thread.");
}

@end
12 changes: 0 additions & 12 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,6 @@
7BECF42826145CD900D9826E /* SentryMechanismMeta.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BECF42726145CD900D9826E /* SentryMechanismMeta.m */; };
7BECF432261463E600D9826E /* SentryMechanismMetaTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BECF431261463E600D9826E /* SentryMechanismMetaTests.swift */; };
7BED3576266F7BFF00EAA70D /* TestSentryCrashAdapter.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BED3575266F7BFF00EAA70D /* TestSentryCrashAdapter.m */; };
7BEF4951270C3F5700F8F30E /* SentrySubClassFinder.h in Headers */ = {isa = PBXBuildFile; fileRef = 7BEF4950270C3F5700F8F30E /* SentrySubClassFinder.h */; };
7BEF4953270C3F7F00F8F30E /* SentrySubClassFinder.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BEF4952270C3F7F00F8F30E /* SentrySubClassFinder.m */; };
7BEF4955270C3FEC00F8F30E /* SentrySubClassFinderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BEF4954270C3FEC00F8F30E /* SentrySubClassFinderTests.swift */; };
7BEF4957270C4B9D00F8F30E /* SentryUIViewControllerSwizzlingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BEF4956270C4B9D00F8F30E /* SentryUIViewControllerSwizzlingTests.swift */; };
7BEFB044270B0F630025F808 /* SentryTracerObjCTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 7BEFB043270B0F630025F808 /* SentryTracerObjCTests.m */; };
7BF536D124BDF3E7004FA6A2 /* SentryEnvelopeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BF536D024BDF3E7004FA6A2 /* SentryEnvelopeTests.swift */; };
Expand Down Expand Up @@ -1006,9 +1003,6 @@
7BECF431261463E600D9826E /* SentryMechanismMetaTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryMechanismMetaTests.swift; sourceTree = "<group>"; };
7BED3574266F7BC600EAA70D /* TestSentryCrashAdapter.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestSentryCrashAdapter.h; sourceTree = "<group>"; };
7BED3575266F7BFF00EAA70D /* TestSentryCrashAdapter.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = TestSentryCrashAdapter.m; sourceTree = "<group>"; };
7BEF4950270C3F5700F8F30E /* SentrySubClassFinder.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SentrySubClassFinder.h; sourceTree = "<group>"; };
7BEF4952270C3F7F00F8F30E /* SentrySubClassFinder.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySubClassFinder.m; sourceTree = "<group>"; };
7BEF4954270C3FEC00F8F30E /* SentrySubClassFinderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySubClassFinderTests.swift; sourceTree = "<group>"; };
7BEF4956270C4B9D00F8F30E /* SentryUIViewControllerSwizzlingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIViewControllerSwizzlingTests.swift; sourceTree = "<group>"; };
7BEFB043270B0F630025F808 /* SentryTracerObjCTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryTracerObjCTests.m; sourceTree = "<group>"; };
7BF536D024BDF3E7004FA6A2 /* SentryEnvelopeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryEnvelopeTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1427,8 +1421,6 @@
7B98D7E725FB7BCD00C5A389 /* SentryAppState.m */,
7BD86EC4264A63F6005439DB /* SentrySysctl.h */,
7BD86EC6264A641D005439DB /* SentrySysctl.m */,
7BEF4950270C3F5700F8F30E /* SentrySubClassFinder.h */,
7BEF4952270C3F7F00F8F30E /* SentrySubClassFinder.m */,
);
name = Helper;
sourceTree = "<group>";
Expand Down Expand Up @@ -1975,7 +1967,6 @@
69BEE6F62620729E006DF9DF /* UrlSessionDelegateSpy.swift */,
7BD86ECA264A6DB5005439DB /* TestSysctl.swift */,
7B16FD012654F86B008177D3 /* SentrySysctlTests.swift */,
7BEF4954270C3FEC00F8F30E /* SentrySubClassFinderTests.swift */,
);
path = Helper;
sourceTree = "<group>";
Expand Down Expand Up @@ -2254,7 +2245,6 @@
7B04A9AF24EAC02C00E710B1 /* SentryRetryAfterHeaderParser.h in Headers */,
7DC83100239826280043DD9A /* SentryIntegrationProtocol.h in Headers */,
7B98D7BC25FB607300C5A389 /* SentryOutOfMemoryTracker.h in Headers */,
7BEF4951270C3F5700F8F30E /* SentrySubClassFinder.h in Headers */,
7BA61CB9247BC57B00C130A8 /* SentryCrashDefaultBinaryImageProvider.h in Headers */,
8E4E7C7D25DAB287006AB9E2 /* SentryTracer.h in Headers */,
7BC8523724588115005A70F0 /* SentryRateLimitCategory.h in Headers */,
Expand Down Expand Up @@ -2417,7 +2407,6 @@
15360CD62432832400112302 /* SentryAutoSessionTrackingIntegration.m in Sources */,
7BE3C7692445C1A800A38442 /* SentryCurrentDate.m in Sources */,
63EED6C02237923600E02400 /* SentryOptions.m in Sources */,
7BEF4953270C3F7F00F8F30E /* SentrySubClassFinder.m in Sources */,
63AA769E1EB9C57A00D153DE /* SentryError.m in Sources */,
7B8713B026415B22006D6004 /* SentryAppStartTrackingIntegration.m in Sources */,
8E133FA225E72DEF00ABD0BF /* SentrySamplingContext.m in Sources */,
Expand Down Expand Up @@ -2582,7 +2571,6 @@
63FE720D20DA66EC00CDBAE8 /* NSError+SimpleConstructor_Tests.m in Sources */,
69BEE6F72620729E006DF9DF /* UrlSessionDelegateSpy.swift in Sources */,
7B98D7EC25FB7C4900C5A389 /* SentryAppStateTests.swift in Sources */,
7BEF4955270C3FEC00F8F30E /* SentrySubClassFinderTests.swift in Sources */,
8EE017A126704CD500470616 /* SentryUIPerformanceTrackerTests.swift in Sources */,
7B5B94352657AD21002E474B /* SentryFramesTrackingIntegrationTests.swift in Sources */,
7BAF3DC8243DB90E008A5414 /* TestTransport.swift in Sources */,
Expand Down
9 changes: 9 additions & 0 deletions Sources/Sentry/SentryDispatchQueueWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ - (void)dispatchAsyncWithBlock:(void (^)(void))block
});
}

- (void)dispatchOnMainQueue:(void (^)(void))block
{
dispatch_async(dispatch_get_main_queue(), ^{
@autoreleasepool {
block();
}
});
}

- (void)dispatchOnce:(dispatch_once_t *)predicate block:(void (^)(void))block
{
dispatch_once(predicate, block);
Expand Down
15 changes: 0 additions & 15 deletions Sources/Sentry/SentrySubClassFinder.h

This file was deleted.

38 changes: 0 additions & 38 deletions Sources/Sentry/SentrySubClassFinder.m

This file was deleted.

79 changes: 62 additions & 17 deletions Sources/Sentry/SentryUIViewControllerSwizziling.m
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#import "SentryUIViewControllerSwizziling.h"
#import "SentryLog.h"
#import "SentryPerformanceTracker.h"
#import "SentrySubClassFinder.h"
#import "SentrySwizzle.h"
#import "SentryUIViewControllerPerformanceTracker.h"
#import <SentryDispatchQueueWrapper.h>
Expand All @@ -25,25 +24,71 @@ + (void)startWithOptions:(SentryOptions *)options

[SentryUIViewControllerSwizziling swizzleRootViewController];

// Swizzle all custom UIViewControllers. First, fetch all subclasses of UIViewController and
// then swizzle them. As there is no straightforward way to get all sub-classes in Objective-C,
// the code first retrieves all classes in the runtime, iterates over all classes, and checks
// for every class if UIViewController is a parent. Cause loading all classes can take a few
// milliseconds, do this on a background thread, which should be fine because the SDK swizzles
// the root view controller and its children above. Previously, the code intercepted the
// ViewController initializers with swizzling to swizzle the lifecycle methods. This approach
// led to UIViewControllers crashing when using a convenience initializer, see GH-1355. The
// error occurred because our swizzling logic adds the method to swizzle if the class doesn't
// implement it. It seems like adding an extra initializer causes problems with the rules for
// initialization in Swift, see
// https://docs.swift.org/swift-book/LanguageGuide/Initialization.html#ID216.
[SentryUIViewControllerSwizziling
swizzleSubclassesOf:[UIViewController class]
dispatchQueue:dispatchQueue
swizzleBlock:^(Class class) {
[SentryUIViewControllerSwizziling swizzleViewControllerSubClass:class];
}];
}

/**
* To be able to test this we put the logic in a extra method.
*
* Swizzle all custom UIViewControllers. First, fetch all subclasses of UIViewController and then
* swizzle them. As there is no straightforward way to get all sub-classes in Objective-C, the code
* first retrieves all classes in the runtime, iterates over all classes, and checks for every class
* if UIViewController is a parent. Cause loading all classes can take a few milliseconds, do this
* on a background thread, which should be fine because the SDK swizzles the root view controller
* and its children above. After finding all subclasses of the UIViewController, this method
* swizzles them on the main thread. Swizzling the UIViewControllers on a background thread led to
* crashes, see GH-1366.
*
* Previously, the code intercepted the ViewController initializers with
* swizzling to swizzle the lifecycle methods. This approach led to UIViewControllers crashing when
* using a convenience initializer, see GH-1355. The error occurred because our swizzling logic adds
* the method to swizzle if the class doesn't implement it. It seems like adding an extra
* initializer causes problems with the rules for initialization in Swift, see
* https://docs.swift.org/swift-book/LanguageGuide/Initialization.html#ID216.
*/
+ (void)swizzleSubclassesOf:(Class)parentClass
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
swizzleBlock:(void (^)(Class))block
{
[dispatchQueue dispatchAsyncWithBlock:^{
NSArray<Class> *viewControllers =
[SentrySubClassFinder getSubclassesOf:[UIViewController class]];
for (Class viewController in viewControllers) {
[SentryUIViewControllerSwizziling swizzleViewControllerSubClass:viewController];
int numClasses = objc_getClassList(NULL, 0);
Class *classes = (__unsafe_unretained Class *)malloc(sizeof(Class) * numClasses);
numClasses = objc_getClassList(classes, numClasses);

if (numClasses <= 0) {
[SentryLog logWithMessage:@"No classes found when retrieving class list."
andLevel:kSentryLevelError];
}

// Storing the actual classes in an NSArray would call initialize of the class, which we
// must avoid as we are on a background thread here and dealing with UIViewControllers,
// which assume they are running on the main thread. Therefore, we store the indexes instead
// so we can search for the subclasses on a background thread.
NSMutableArray *indexesToSwizzle = [NSMutableArray new];
for (NSInteger i = 0; i < numClasses; i++) {
Class superClass = classes[i];
do {
superClass = class_getSuperclass(superClass);
} while (superClass && superClass != parentClass);

if (superClass != nil) {
[indexesToSwizzle addObject:@(i)];
}
}

// We must swizzle the UIViewControllers on the main thread. Otherwise, we could crash.
[dispatchQueue dispatchOnMainQueue:^{
for (NSNumber *i in indexesToSwizzle) {
NSInteger index = [i integerValue];
block(classes[index]);
}
free(classes);
}];
}];
}

Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryDispatchQueueWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ NS_ASSUME_NONNULL_BEGIN

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

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

- (void)dispatchOnce:(dispatch_once_t *)predicate block:(void (^)(void))block;

@end
Expand Down
42 changes: 0 additions & 42 deletions Tests/SentryTests/Helper/SentrySubClassFinderTests.swift

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ NS_ASSUME_NONNULL_BEGIN

+ (void)swizzleViewControllerSubClass:(Class)class;

+ (void)swizzleSubclassesOf:(Class)parentClass
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
swizzleBlock:(void (^)(Class))block;

@end

#endif
Expand Down
Loading