From 1d3fad62fcf6a6263147f7a79189aabcd53b29f1 Mon Sep 17 00:00:00 2001 From: ArielDemarco Date: Mon, 15 Dec 2025 11:37:07 -0300 Subject: [PATCH 1/2] Avoid instrumenting background tasks w/o delegates as it leads to crashes --- .../URLSessionInstrumentation.swift | 33 +++++++++++-------- .../URLSessionInstrumentationTests.swift | 29 ++++++++++++++++ Tests/Shared/TestUtils/XCTestCase+Wait.swift | 24 ++++++++++++++ 3 files changed, 73 insertions(+), 13 deletions(-) create mode 100644 Tests/Shared/TestUtils/XCTestCase+Wait.swift diff --git a/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift b/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift index ef72475b..d222a30d 100644 --- a/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift +++ b/Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift @@ -207,11 +207,6 @@ public class URLSessionInstrumentation { } } self.setIdKey(value: sessionTaskId, for: task) - - // We want to identify background tasks - if session.configuration.identifier == nil { - objc_setAssociatedObject(task, "IsBackground", true, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) - } return task } let swizzledIMP = imp_implementationWithBlock( @@ -741,15 +736,12 @@ public class URLSessionInstrumentation { return } - // We cannot instrument async background tasks because they crash if you assign a delegate - if #available(OSX 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) { - if objc_getAssociatedObject(task, "IsBackground") is Bool { - guard Task.basePriority == nil else { - return - } - } + // Background tasks cannot be instrumented because they crash if you assign a delegate + // They require the delegate to be set when creating the session + guard !task.isBackground else { + return } - + let taskId = idKeyForTask(task) if let request = task.currentRequest { queue.sync { @@ -876,3 +868,18 @@ class AsyncTaskDelegate: NSObject, URLSessionTaskDelegate { } } } + +extension URLSessionTask { + var isBackground: Bool { + [ + "__NSCFBackgroundAVAggregateAssetDownloadTask", + "__NSCFBackgroundAVAggregateAssetDownloadTaskNoChildTask", + "__NSCFBackgroundAVAssetDownloadTask", + "__NSCFBackgroundDataTask", + "__NSCFBackgroundDownloadTask", + "__NSCFBackgroundSessionTask", + "__NSCFBackgroundUploadTask" + ] + .contains(String(describing: type(of: self))) + } +} diff --git a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift index 6bb07a4e..405dd411 100644 --- a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift +++ b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift @@ -931,4 +931,33 @@ class URLSessionInstrumentationTests: XCTestCase { XCTAssertTrue(URLSessionInstrumentationTests.checker.createdRequestCalled, "createdRequest should be called") XCTAssertTrue(URLSessionInstrumentationTests.checker.receivedResponseCalled, "receivedResponse should be called") } + + // MARK: - Background session tests + @available(macOS 10.15, iOS 15.0, tvOS 15.0, watchOS 8.0, *) + public func testBackgroundSession_ShouldNotCrashWhenAssigningDelegate() async throws { + let request = URLRequest(url: URL(string: "http://localhost:33333/success")!) + + // Background sessions require a delegate to be set when creating the session. + // However, for this test, we set to nil so it tries to do it on resume (where the crash happens) + let session = URLSession( + configuration: URLSessionConfiguration.background( + withIdentifier: UUID().uuidString + ), + delegate: nil, + delegateQueue: .main + ) + + // Background sessions cannot use async/await or completion handlers + // Must use dataTask(with:) and call resume() + let task = session.dataTask(with: request) + task.resume() + + // Wait a bit for the task to attempt to complete + // The important thing is that it doesn't crash when the instrumentation + // checks if it should assign a fake delegate + try await Task.sleep(nanoseconds: 100_000_000) // 100ms + + // The test passes if tasks completes without crashing. + wait { task.state == .completed } + } } diff --git a/Tests/Shared/TestUtils/XCTestCase+Wait.swift b/Tests/Shared/TestUtils/XCTestCase+Wait.swift new file mode 100644 index 00000000..04ec495c --- /dev/null +++ b/Tests/Shared/TestUtils/XCTestCase+Wait.swift @@ -0,0 +1,24 @@ +// +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 +// + +import XCTest + +extension XCTestCase { + public func wait(timeout: TimeInterval = 3, interval: TimeInterval = 0.1, until block: @escaping () throws -> Bool) { + let expectation = expectation(description: "wait for block to pass") + let timer = Timer.scheduledTimer(withTimeInterval: interval, repeats: true) { _ in + do { + if try block() { + expectation.fulfill() + } + } catch { + fatalError("Waiting for operation that threw an error: \(error)") + } + } + + wait(for: [expectation], timeout: timeout) + timer.invalidate() + } +} From aff3be77673ccc5cb6de3189ee553b71750b8ad6 Mon Sep 17 00:00:00 2001 From: ArielDemarco Date: Mon, 15 Dec 2025 11:52:47 -0300 Subject: [PATCH 2/2] Removed first attempt of waiting in test (flakier than latest solution) --- .../URLSessionTests/URLSessionInstrumentationTests.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift index 9a49636e..d4d0d85a 100644 --- a/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift +++ b/Tests/InstrumentationTests/URLSessionTests/URLSessionInstrumentationTests.swift @@ -1078,11 +1078,6 @@ class URLSessionInstrumentationTests: XCTestCase { let task = session.dataTask(with: request) task.resume() - // Wait a bit for the task to attempt to complete - // The important thing is that it doesn't crash when the instrumentation - // checks if it should assign a fake delegate - try await Task.sleep(nanoseconds: 100_000_000) // 100ms - // The test passes if tasks completes without crashing. wait { task.state == .completed } }