fix: Don't use main thread for app hang screenshot and view hierarchy

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+### Fixes
+
+- Don't use main thread for app hang screenshot and view hierarchy (#3547)
 ### Fixes 
 
 - Move header reference out of "extern C" (#3538)

Sources/Sentry/PrivateSentrySDKOnly.mm

@@ -207,7 +207,7 @@ + (SentryScreenFrames *)currentScreenFrames
 + (NSArray<NSData *> *)captureScreenshots
 {
 #if SENTRY_HAS_UIKIT
-    return [SentryDependencyContainer.sharedInstance.screenshot takeScreenshots];
+    return [SentryDependencyContainer.sharedInstance.screenshot appScreenshots];
 #else
     SENTRY_LOG_DEBUG(
         @"PrivateSentrySDKOnly.captureScreenshots only works with UIKit enabled. Ensure you're "
@@ -219,7 +219,7 @@ + (SentryScreenFrames *)currentScreenFrames
 + (NSData *)captureViewHierarchy
 {
 #if SENTRY_HAS_UIKIT
-    return [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy];
+    return [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy];
 #else
     SENTRY_LOG_DEBUG(
         @"PrivateSentrySDKOnly.captureViewHierarchy only works with UIKit enabled. Ensure you're "

Sources/Sentry/SentryEvent.m

@@ -201,6 +201,10 @@ - (BOOL)isMetricKitEvent
 
 #endif // SENTRY_HAS_METRIC_KIT
 
+- (bool)isAppHangEvent {
+    return self.exceptions.count == 1 && [self.exceptions.firstObject.type isEqualToString:@"App Hanging"];
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Sources/Sentry/SentryScreenshot.m

@@ -10,11 +10,11 @@
 
 @implementation SentryScreenshot
 
-- (NSArray<NSData *> *)appScreenshots
+- (NSArray<NSData *> *)appScreenshotsFromMainThread
 {
     __block NSArray *result;
 
-    void (^takeScreenShot)(void) = ^{ result = [self takeScreenshots]; };
+    void (^takeScreenShot)(void) = ^{ result = [self appScreenshots]; };
 
     [[SentryDependencyContainer sharedInstance].dispatchQueueWrapper
         dispatchSyncOnMainQueue:takeScreenShot];
@@ -29,7 +29,7 @@ - (void)saveScreenShots:(NSString *)imagesDirectoryPath
     // We did it this way because we use this function to save screenshots
     // during signal handling, and if we dispatch it to the main thread,
     // that is probably blocked by the crash event, we freeze the application.
-    [[self takeScreenshots] enumerateObjectsUsingBlock:^(NSData *obj, NSUInteger idx, BOOL *stop) {
+    [[self appScreenshots] enumerateObjectsUsingBlock:^(NSData *obj, NSUInteger idx, BOOL *stop) {
         NSString *name = idx == 0
             ? @"screenshot.png"
             : [NSString stringWithFormat:@"screenshot-%li.png", (unsigned long)idx + 1];
@@ -38,7 +38,7 @@ - (void)saveScreenShots:(NSString *)imagesDirectoryPath
     }];
 }
 
-- (NSArray<NSData *> *)takeScreenshots
+- (NSArray<NSData *> *)appScreenshots
 {
     NSArray<UIWindow *> *windows = [SentryDependencyContainer.sharedInstance.application windows];
 

Sources/Sentry/SentryScreenshotIntegration.m

@@ -6,6 +6,7 @@
 #    import "SentryCrashC.h"
 #    import "SentryDependencyContainer.h"
 #    import "SentryEvent+Private.h"
+#    import "SentryException.h"
 #    import "SentryHub+Private.h"
 #    import "SentrySDK+Private.h"
 
@@ -63,7 +64,15 @@ - (void)uninstall
         return attachments;
     }
 
-    NSArray *screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots];
+    NSArray *screenshot;
+
+    // If the event is an App hanging event, we cant take the
+    // screenshot in the main thread because it's blocked.
+    if (event.isAppHangEvent) {
+        screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots];
+    } else {
+        screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshotsFromMainThread];
+    }
 
     NSMutableArray *result =
         [NSMutableArray arrayWithCapacity:attachments.count + screenshot.count];

Sources/Sentry/SentryViewHierarchy.m

@@ -5,6 +5,7 @@
 #    import "SentryCrashFileUtils.h"
 #    import "SentryCrashJSONCodec.h"
 #    import "SentryDependencyContainer.h"
+#    import "SentryDispatchQueueWrapper.h"
 #    import "SentryLog.h"
 #    import "SentryUIApplication.h"
 #    import <UIKit/UIKit.h>
@@ -46,26 +47,28 @@ - (BOOL)saveViewHierarchy:(NSString *)filePath
     return result;
 }
 
-- (NSData *)fetchViewHierarchy
+- (NSData *)appViewHierarchyFromMainThread
 {
-    __block NSMutableData *result = [[NSMutableData alloc] init];
+    __block NSData *result;
 
-    void (^save)(void) = ^{
-        NSArray<UIWindow *> *windows =
-            [SentryDependencyContainer.sharedInstance.application windows];
+    void (^fetchViewHierarchy)(void) = ^{ result = [self appViewHierarchy]; };
 
-        if (![self processViewHierarchy:windows
-                            addFunction:writeJSONDataToMemory
-                               userData:(__bridge void *)(result)]) {
+    [[SentryDependencyContainer sharedInstance].dispatchQueueWrapper
+        dispatchSyncOnMainQueue:fetchViewHierarchy];
 
-            result = nil;
-        }
-    };
+    return result;
+}
+
+- (NSData *)appViewHierarchy
+{
+    NSMutableData *result = [[NSMutableData alloc] init];
+    NSArray<UIWindow *> *windows = [SentryDependencyContainer.sharedInstance.application windows];
+
+    if (![self processViewHierarchy:windows
+                        addFunction:writeJSONDataToMemory
+                           userData:(__bridge void *)(result)]) {
 
-    if ([NSThread isMainThread]) {
-        save();
-    } else {
-        dispatch_sync(dispatch_get_main_queue(), save);
+        result = nil;
     }
 
     return result;

Sources/Sentry/SentryViewHierarchyIntegration.m

@@ -5,10 +5,10 @@
 #    import "SentryCrashC.h"
 #    import "SentryDependencyContainer.h"
 #    import "SentryEvent+Private.h"
+#    import "SentryException.h"
 #    import "SentryHub+Private.h"
 #    import "SentrySDK+Private.h"
 #    import "SentryViewHierarchy.h"
-
 #    if SENTRY_HAS_METRIC_KIT
 #        import "SentryMetricKitIntegration.h"
 #    endif // SENTRY_HAS_METRIC_KIT
@@ -71,8 +71,16 @@ - (void)uninstall
 
     NSMutableArray<SentryAttachment *> *result = [NSMutableArray arrayWithArray:attachments];
 
-    NSData *viewHierarchy =
-        [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy];
+    NSData *viewHierarchy;
+
+    // If the event is an App hanging event, we cant take the
+    // view hierarchy in the main thread because it's blocked.
+    if (event.isAppHangEvent) {
+        viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy];
+    } else {
+        viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchyFromMainThread];
+    }
+
     SentryAttachment *attachment =
         [[SentryAttachment alloc] initWithData:viewHierarchy
                                       filename:@"view-hierarchy.json"

Sources/Sentry/include/SentryEvent+Private.h

@@ -11,6 +11,11 @@ SentryEvent ()
  */
 @property (nonatomic) BOOL isCrashEvent;
 
+/**
+ * This indicates whether this event represents an app hang.
+ */
+@property (nonatomic, readonly) BOOL isAppHangEvent;
+
 /**
  * We're storing serialized breadcrumbs to disk in JSON, and when we're reading them back (in
  * the case of OOM), we end up with the serialized breadcrumbs again. Instead of turning those

Sources/Sentry/include/SentryScreenshot.h

@@ -10,7 +10,7 @@ NS_ASSUME_NONNULL_BEGIN
  * Get a screenshot of every open window in the app.
  * @return An array of @c NSData instances containing PNG images.
  */
-- (nullable NSArray<NSData *> *)appScreenshots;
+- (NSArray<NSData *> *)appScreenshotsFromMainThread;
 
 /**
  * Save the current app screen shots in the given directory.
@@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)saveScreenShots:(NSString *)imagesDirectoryPath;
 
-- (NSArray<NSData *> *)takeScreenshots;
+- (NSArray<NSData *> *)appScreenshots;
 @end
 
 NS_ASSUME_NONNULL_END

Sources/Sentry/include/SentryViewHierarchy.h

@@ -6,7 +6,16 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface SentryViewHierarchy : NSObject
 
-- (nullable NSData *)fetchViewHierarchy;
+/**
+ Get the view hierarchy in a json format.
+ Always runs in the main thread.
+ */
+- (nullable NSData *)appViewHierarchyFromMainThread;
+
+/**
+ Get the view hierarchy in a json format.
+ */
+- (nullable NSData *)appViewHierarchy;
 
 /**
  * Save the current app view hierarchy in the given file path.

Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift

@@ -171,6 +171,28 @@ class SentryScreenshotIntegrationTests: XCTestCase {
 
     }
 
+    func test_backgroundForAppHangs() {
+        let sut = fixture.getSut()
+        let testVH = TestSentryScreenshot()
+        SentryDependencyContainer.sharedInstance().screenshot = testVH
+
+        let event = Event()
+        event.exceptions = [Sentry.Exception(value: "test", type: "App Hanging")]
+
+        let ex = expectation(description: "Attachment Added")
+
+        testVH.processViewHierarchyCallback = {
+            ex.fulfill()
+            XCTAssertFalse(Thread.isMainThread)
+        }
+
+        DispatchQueue.global().async {
+            sut.processAttachments([], for: event)
+        }
+
+        wait(for: [ex], timeout: 1)
+    }
+
 }
 
 #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)

Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift

@@ -2,12 +2,17 @@
 
 class TestSentryScreenshot: SentryScreenshot {
 
-    var result: [Data]?
+    var result: [Data] = []
+    var processViewHierarchyCallback: (() -> Void)?
 
-    override func appScreenshots() -> [Data]? {
+    override func appScreenshots() -> [Data] {
+        processViewHierarchyCallback?()
+        return result
+    }
+
+    override func appScreenshotsFromMainThread() -> [Data] {
         return result
     }
-
 }
 
 #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)

Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift

@@ -3,6 +3,7 @@
 import Sentry
 import SentryTestUtils
 import XCTest
+import Nimble
 
 class SentryViewHierarchyIntegrationTests: XCTestCase {
 
@@ -123,6 +124,29 @@ class SentryViewHierarchyIntegrationTests: XCTestCase {
         XCTAssertEqual(newAttachmentList?.count, 1)
         XCTAssertEqual(newAttachmentList?.first, attachment)
     }
+
+    func test_backgroundForAppHangs() {
+        let sut = fixture.getSut()
+        let testVH = TestSentryViewHierarchy()
+        SentryDependencyContainer.sharedInstance().viewHierarchy = testVH
+
+        let event = Event()
+        event.exceptions = [Sentry.Exception(value: "test", type: "App Hanging")]
+
+        let ex = expectation(description: "Attachment Added")
+
+        testVH.processViewHierarchyCallback = {
+            ex.fulfill()
+            expect(Thread.isMainThread) == false
+        }
+
+        let dispatch = DispatchQueue(label: "background")
+        dispatch.async {
+            sut.processAttachments([], for: event)
+        }
+
+        wait(for: [ex], timeout: 1)
+    }
 }
 
 #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)

Tests/SentryTests/Integrations/ViewHierarchy/TestSentryViewHierarchy.swift

@@ -7,10 +7,10 @@ class TestSentryViewHierarchy: SentryViewHierarchy {
     var processViewHierarchyCallback: (() -> Void)?
     var saveFilePathUsed: String?
 
-    override func fetch() -> Data? {
+    override func appViewHierarchy() -> Data? {
         guard let result = self.result
         else {
-            return super.fetch()
+            return super.appViewHierarchy()
         }
         return result
     }

Tests/SentryTests/SentryScreenShotTests.swift

@@ -39,7 +39,7 @@ class SentryScreenShotTests: XCTestCase {
 
         let expect = expectation(description: "Screenshot")
         let _ = queue.async {
-            self.fixture.sut.appScreenshots()
+            self.fixture.sut.appScreenshotsFromMainThread()
             expect.fulfill()
         }
 
@@ -72,11 +72,7 @@ class SentryScreenShotTests: XCTestCase {
         let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 10, height: 10))
         fixture.uiApplication.windows = [testWindow]
 
-        guard let data = self.fixture.sut.appScreenshots() else {
-            XCTFail("Could not make window screenshot")
-            return
-        }
-
+        let data = self.fixture.sut.appScreenshots()
         let image = UIImage(data: data[0])
 
         XCTAssertEqual(image?.size.width, 10)
@@ -87,10 +83,7 @@ class SentryScreenShotTests: XCTestCase {
         let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 0, height: 0))
         fixture.uiApplication.windows = [testWindow]
 
-        guard let data = self.fixture.sut.appScreenshots() else {
-            XCTFail("Could not make window screenshot")
-            return
-        }
+        let data = self.fixture.sut.appScreenshots()
 
         XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero size.")
     }
@@ -99,10 +92,7 @@ class SentryScreenShotTests: XCTestCase {
         let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 0, height: 1_000))
         fixture.uiApplication.windows = [testWindow]
 
-        guard let data = self.fixture.sut.appScreenshots() else {
-            XCTFail("Could not make window screenshot")
-            return
-        }
+        let data = self.fixture.sut.appScreenshots()
 
         XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero width.")
     }
@@ -111,10 +101,7 @@ class SentryScreenShotTests: XCTestCase {
         let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 1_000, height: 0))
         fixture.uiApplication.windows = [testWindow]
 
-        guard let data = self.fixture.sut.appScreenshots() else {
-            XCTFail("Could not make window screenshot")
-            return
-        }
+        let data = self.fixture.sut.appScreenshots()
 
         XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero height.")
     }