fix: avoid task lock during breadcrumbs

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Only expose `experimental.dataCollection` APIs in SDK V10 (#8435)
 
+### Fixes
+
+- Prevent Session Replay network-detail breadcrumbs from blocking URLSession cancellation on the task monitor (#8497)
+
 ## 9.22.0
 
 ### Features

Sources/Sentry/SentryNetworkTracker.m

@@ -527,14 +527,13 @@ - (void)addBreadcrumbForSessionTask:(NSURLSessionTask *)sessionTask
     }
 
 #if SENTRY_TARGET_REPLAY_SUPPORTED
-    // Check if network details was enabled for this url.
-    @synchronized(sessionTask) {
-        SentryReplayNetworkDetails *networkDetails
-            = objc_getAssociatedObject(sessionTask, &SentryNetworkDetailsKey);
-        if (networkDetails) {
-            // Store raw object; serialized at read time by SentrySRDefaultBreadcrumbConverter
-            breadcrumbData[SentryReplayNetworkDetails.replayNetworkDetailsKey] = networkDetails;
-        }
+    // Do not synchronize on sessionTask here. This method runs synchronously from the setState:
+    // swizzle, which can run on the main thread during cancellation.
+    SentryReplayNetworkDetails *networkDetails
+        = objc_getAssociatedObject(sessionTask, &SentryNetworkDetailsKey);
+    if (networkDetails) {
+        // Store raw object; serialized at read time by SentrySRDefaultBreadcrumbConverter
+        breadcrumbData[SentryReplayNetworkDetails.replayNetworkDetailsKey] = networkDetails;
     }
 #endif // SENTRY_TARGET_REPLAY_SUPPORTED
 
@@ -664,41 +663,41 @@ - (void)captureResponseDetails:(NSData *)data
         return;
     }
 
+    SentryReplayNetworkDetails *details;
+    // Keep response processing outside this critical section so cancellation does not wait for
+    // header and body processing while trying to acquire the task monitor.
     @synchronized(task) {
-        SentryReplayNetworkDetails *details
-            = objc_getAssociatedObject(task, &SentryNetworkDetailsKey);
-        if (!details) {
-            SENTRY_LOG_WARN(@"[NetworkCapture] No SentryReplayNetworkDetails found for %@ - "
-                            @"skipping response capture",
-                urlString);
-            return;
-        }
-
-        NSInteger statusCode = 0;
-        NSDictionary *allHeaders = nil;
-        NSString *contentType = nil;
-        if ([response isKindOfClass:[NSHTTPURLResponse class]]) {
-            NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;
-            statusCode = httpResponse.statusCode;
-            // sentry-lint:disable avoid_all_header_fields
-            // Safe: reading the whole dictionary, not a case-sensitive lookup.
-            allHeaders = httpResponse.allHeaderFields;
-            // sentry-lint:enable avoid_all_header_fields
-            contentType =
-                [SentryHTTPHeaderReader valueForHTTPHeaderFieldCaseInsensitive:@"content-type"
-                                                                    inResponse:httpResponse];
-        }
-
-        NSData *bodyData
-            = (options.sessionReplay.networkCaptureBodies && data.length > 0) ? data : nil;
-
-        [details setResponseWithStatusCode:statusCode
-                                      size:@(data ? data.length : 0)
-                                  bodyData:bodyData
-                               contentType:contentType
-                                allHeaders:allHeaders
-                         configuredHeaders:options.sessionReplay.networkResponseHeaders];
+        details = objc_getAssociatedObject(task, &SentryNetworkDetailsKey);
     }
+    if (!details) {
+        SENTRY_LOG_WARN(@"[NetworkCapture] No SentryReplayNetworkDetails found for %@ - "
+                        @"skipping response capture",
+            urlString);
+        return;
+    }
+
+    NSInteger statusCode = 0;
+    NSDictionary *allHeaders = nil;
+    NSString *contentType = nil;
+    if ([response isKindOfClass:[NSHTTPURLResponse class]]) {
+        NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;
+        statusCode = httpResponse.statusCode;
+        // sentry-lint:disable avoid_all_header_fields
+        // Safe: reading the whole dictionary, not a case-sensitive lookup.
+        allHeaders = httpResponse.allHeaderFields;
+        // sentry-lint:enable avoid_all_header_fields
+        contentType = [SentryHTTPHeaderReader valueForHTTPHeaderFieldCaseInsensitive:@"content-type"
+                                                                          inResponse:httpResponse];
+    }
+
+    NSData *bodyData = (options.sessionReplay.networkCaptureBodies && data.length > 0) ? data : nil;
+
+    [details setResponseWithStatusCode:statusCode
+                                  size:@(data ? data.length : 0)
+                              bodyData:bodyData
+                           contentType:contentType
+                            allHeaders:allHeaders
+                     configuredHeaders:options.sessionReplay.networkResponseHeaders];
 }
 
 - (void)captureRequestDetails:(NSURLSessionTask *)sessionTask

Sources/Swift/Integrations/SessionReplay/SentryReplayNetworkDetails.swift

@@ -17,9 +17,9 @@ enum NetworkBodyWarning: String {
 /// via `setRequest`/`setResponse`. Swift callers (SentrySRDefaultBreadcrumbConverter)
 /// consume it via `serialize()`.
 ///
-/// - Important: `setRequest` and `setResponse` can be called concurrently from
-///   `SentryNetworkTracker` because they write to independent properties.
-///   Adding shared mutable state between will require adding synchronization.
+/// - Important: Request and response parsing happens before publishing the result to the internal
+///   state. State access is nonblocking because these details are optional enrichment and must not
+///   delay URLSession callbacks or main-thread cancellation.
 
 @objc
 @_spi(Private) public class SentryReplayNetworkDetails: NSObject {
@@ -242,16 +242,32 @@ enum NetworkBodyWarning: String {
 
     // MARK: - Properties
 
-    private(set) var method: String?
-    private(set) var statusCode: NSNumber?
-    private(set) var request: Detail?
-    private(set) var response: Detail?
+    private struct State {
+        var statusCode: NSNumber?
+        var request: Detail?
+        var response: Detail?
+    }
+
+    let method: String?
+    private let state = SentryMutex(State())
+
+    private var stateSnapshot: State? {
+        state.withLockIfAvailable { $0 }
+    }
+
+    var statusCode: NSNumber? {
+        stateSnapshot?.statusCode
+    }
 
     /// Request body size in bytes, derived from request details.
-    var requestBodySize: NSNumber? { request?.size }
+    var requestBodySize: NSNumber? {
+        stateSnapshot?.request?.size
+    }
 
     /// Response body size in bytes, derived from response details.
-    var responseBodySize: NSNumber? { response?.size }
+    var responseBodySize: NSNumber? {
+        stateSnapshot?.response?.size
+    }
 
     // MARK: - Initialization
 
@@ -277,11 +293,12 @@ enum NetworkBodyWarning: String {
     ///   - configuredHeaders: Header names to extract, matched case-insensitively.
     @objc
     public func setRequest(size: NSNumber?, bodyData: Data?, contentType: String?, allHeaders: [String: Any]?, configuredHeaders: [String]?) {
-        self.request = Detail(
+        let request = Detail(
             size: size,
             body: bodyData.flatMap { Body(data: $0, contentType: contentType) },
             headers: SentryReplayNetworkDetails.extractHeaders(from: allHeaders, matching: configuredHeaders)
         )
+        state.withLockIfAvailable { $0.request = request }
     }
 
     /// Sets response details from raw body data.
@@ -298,12 +315,15 @@ enum NetworkBodyWarning: String {
     ///   - configuredHeaders: Header names to extract, matched case-insensitively.
     @objc
     public func setResponse(statusCode: Int, size: NSNumber?, bodyData: Data?, contentType: String?, allHeaders: [String: Any]?, configuredHeaders: [String]?) {
-        self.statusCode = NSNumber(value: statusCode)
-        self.response = Detail(
+        let response = Detail(
             size: size,
             body: bodyData.flatMap { Body(data: $0, contentType: contentType) },
             headers: SentryReplayNetworkDetails.extractHeaders(from: allHeaders, matching: configuredHeaders)
         )
+        state.withLockIfAvailable {
+            $0.statusCode = NSNumber(value: statusCode)
+            $0.response = response
+        }
     }
 
     // MARK: - Header Extraction
@@ -337,11 +357,16 @@ enum NetworkBodyWarning: String {
     @objc public func serialize() -> [String: Any] {
         var result = [String: Any]()
         if let method { result["method"] = method }
-        if let statusCode { result["statusCode"] = statusCode }
-        if let requestBodySize { result["requestBodySize"] = requestBodySize }
-        if let responseBodySize { result["responseBodySize"] = responseBodySize }
-        if let request { result["request"] = request.serialize() }
-        if let response { result["response"] = response.serialize() }
+
+        guard let snapshot = stateSnapshot else {
+            return result
+        }
+
+        if let statusCode = snapshot.statusCode { result["statusCode"] = statusCode }
+        if let requestBodySize = snapshot.request?.size { result["requestBodySize"] = requestBodySize }
+        if let responseBodySize = snapshot.response?.size { result["responseBodySize"] = responseBodySize }
+        if let request = snapshot.request { result["request"] = request.serialize() }
+        if let response = snapshot.response { result["response"] = response.serialize() }
         return result
     }
 

Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift

@@ -607,6 +607,83 @@ class SentryNetworkTrackerTests: XCTestCase {
     }
 
 #if os(iOS) || os(tvOS)
+    func testSetState_whenTaskMonitorHeldDuringBreadcrumb_shouldNotBlockCallingThread() {
+        // -- Arrange --
+        XCTAssertTrue(Thread.isMainThread)
+
+        let tracker = fixture.getSut()
+        let task = ContendedMonitorTaskMock(request: URLRequest(url: Self.fullUrl))
+        let releaseMonitor = DispatchSemaphore(value: 0)
+        let monitorAcquired = DispatchSemaphore(value: 0)
+        let monitorReleased = DispatchSemaphore(value: 0)
+
+        task.countOfBytesReceivedAccessed = { [weak task] in
+            guard let task else { return }
+            task.countOfBytesReceivedAccessed = nil
+            DispatchQueue.global().async {
+                task.holdMonitor(untilReleased: releaseMonitor, monitorAcquired: monitorAcquired)
+                monitorReleased.signal()
+            }
+            XCTAssertEqual(monitorAcquired.wait(timeout: .now() + 1), .success)
+        }
+
+        let releaseWatchdog = DispatchWorkItem {
+            releaseMonitor.signal()
+        }
+        DispatchQueue.global().asyncAfter(deadline: .now() + 1, execute: releaseWatchdog)
+
+        // -- Act --
+        let start = ProcessInfo.processInfo.systemUptime
+        tracker.urlSessionTask(task, setState: .canceling)
+        let duration = ProcessInfo.processInfo.systemUptime - start
+
+        // -- Assert --
+        releaseMonitor.signal()
+        XCTAssertEqual(monitorReleased.wait(timeout: .now() + 1), .success)
+        releaseWatchdog.cancel()
+        XCTAssertLessThan(duration, 0.5, "setState blocked while another thread held the task monitor")
+    }
+
+    func testCaptureResponseDetails_whenReadingResponse_shouldNotHoldTaskMonitor() throws {
+        guard #available(iOS 16.0, tvOS 16.0, *) else { return }
+
+        // -- Arrange --
+        fixture.options.sessionReplay.networkDetailAllowUrls = ["www.domain.com"]
+        let tracker = fixture.getSut()
+        let task = ContendedMonitorTaskMock(request: URLRequest(url: Self.fullUrl))
+        tracker.urlSessionTask(task, setState: .running)
+
+        let response = try XCTUnwrap(MonitorObservingHTTPURLResponse(
+            url: Self.fullUrl,
+            statusCode: 200,
+            httpVersion: "1.1",
+            headerFields: ["Content-Type": "application/json"]
+        ))
+        let releaseMonitor = DispatchSemaphore(value: 0)
+        let monitorAcquired = DispatchSemaphore(value: 0)
+        let monitorReleased = DispatchSemaphore(value: 0)
+
+        response.headersAccessed = { [weak response] in
+            response?.headersAccessed = nil
+            DispatchQueue.global().async {
+                task.holdMonitor(untilReleased: releaseMonitor, monitorAcquired: monitorAcquired)
+                monitorReleased.signal()
+            }
+            XCTAssertEqual(
+                monitorAcquired.wait(timeout: .now() + 0.5),
+                .success,
+                "captureResponseDetails held the task monitor while reading the response"
+            )
+            releaseMonitor.signal()
+        }
+
+        // -- Act --
+        tracker.captureResponseDetails(Data(), response: response, request: Self.fullUrl, task: task)
+
+        // -- Assert --
+        XCTAssertEqual(monitorReleased.wait(timeout: .now() + 1), .success)
+    }
+
     /// Simple case - when network details are enabled, `addBreadcrumbForSessionTask` will include
     /// serialized network details in the breadcrumb data.
     func testAddBreadcrumb_withNetworkDetails_shouldIncludeSerializedDetailsInBreadcrumbData() throws {
@@ -750,6 +827,18 @@ class SentryNetworkTrackerTests: XCTestCase {
         clearTestState()
     }
 
+    private final class MonitorObservingHTTPURLResponse: HTTPURLResponse, @unchecked Sendable {
+        var headersAccessed: (() -> Void)?
+
+        // Intentionally observes reading the whole header dictionary; no case-sensitive lookup.
+        // swiftlint:disable avoid_all_header_fields
+        override var allHeaderFields: [AnyHashable: Any] {
+            headersAccessed?()
+            return super.allHeaderFields
+        }
+        // swiftlint:enable avoid_all_header_fields
+    }
+
     /// `HTTPURLResponse` whose `allHeaderFields` returns the exact (lowercased) casing a server
     /// sends over HTTP/2 or HTTP/3. The public initializer canonicalizes well-known headers such as
     /// `Content-Type`, so overriding `allHeaderFields` is the only way to model the wire casing and

Tests/SentryTests/Integrations/Performance/Network/URLSessionTaskMock.h

@@ -86,4 +86,18 @@ static int64_t const DATA_BYTES_SENT = 652;
 
 @end
 
+/**
+ * A mock that coordinates contention on the Objective-C monitor used by
+ * @synchronized(sessionTask).
+ */
+@interface ContendedMonitorTaskMock : URLSessionDataTaskMock
+
+@property (nonatomic, copy, nullable) dispatch_block_t countOfBytesReceivedAccessed;
+
+- (void)holdMonitorUntilReleased:(dispatch_semaphore_t)releaseSemaphore
+                 monitorAcquired:(dispatch_semaphore_t)monitorAcquiredSemaphore
+    NS_SWIFT_NAME(holdMonitor(untilReleased:monitorAcquired:));
+
+@end
+
 NS_ASSUME_NONNULL_END

Tests/SentryTests/Integrations/Performance/Network/URLSessionTaskMock.m

@@ -325,3 +325,24 @@ - (NSURLRequest *)currentRequest
 }
 
 @end
+
+@implementation ContendedMonitorTaskMock
+
+- (int64_t)countOfBytesReceived
+{
+    if (self.countOfBytesReceivedAccessed != nil) {
+        self.countOfBytesReceivedAccessed();
+    }
+    return [super countOfBytesReceived];
+}
+
+- (void)holdMonitorUntilReleased:(dispatch_semaphore_t)releaseSemaphore
+                 monitorAcquired:(dispatch_semaphore_t)monitorAcquiredSemaphore
+{
+    @synchronized(self) {
+        dispatch_semaphore_signal(monitorAcquiredSemaphore);
+        dispatch_semaphore_wait(releaseSemaphore, DISPATCH_TIME_FOREVER);
+    }
+}
+
+@end