Skip to content

Commit 549d781

Browse files
MaxDesiatovMaxDesiatov
authored
Fix sendability warnings in URLSessionHTTPClient (#7441)
Achieved by converting `DownloadTask` and `DataTask` to value types and adding more `Sendable` annotations where needed.
1 parent f4ab9a4 commit 549d781

File tree

5 files changed

+32
-28
lines changed

5 files changed

+32
-28
lines changed

‎Sources/Basics/AuthorizationProvider.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ import struct Foundation.URL
1717
import Security
1818
#endif
1919

20-
public protocol AuthorizationProvider {
21-
@Sendable
20+
public protocol AuthorizationProvider: Sendable {
2221
func authentication(for url: URL) -> (user: String, password: String)?
2322
}
2423

@@ -80,7 +79,7 @@ extension AuthorizationProvider {
8079

8180
// MARK: - netrc
8281

83-
public class NetrcAuthorizationProvider: AuthorizationProvider, AuthorizationWriter {
82+
public final class NetrcAuthorizationProvider: AuthorizationProvider, AuthorizationWriter {
8483
// marked internal for testing
8584
internal let path: AbsolutePath
8685
private let fileSystem: FileSystem
@@ -202,7 +201,7 @@ public class NetrcAuthorizationProvider: AuthorizationProvider, AuthorizationWri
202201
// MARK: - Keychain
203202

204203
#if canImport(Security)
205-
public class KeychainAuthorizationProvider: AuthorizationProvider, AuthorizationWriter {
204+
public final class KeychainAuthorizationProvider: AuthorizationProvider, AuthorizationWriter {
206205
private let observabilityScope: ObservabilityScope
207206

208207
private let cache = ThreadSafeKeyValueStore<String, (user: String, password: String)>()

‎Sources/Basics/HTTPClient/LegacyHTTPClient.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ public final class LegacyHTTPClient: Cancellable {
2626
public typealias Request = LegacyHTTPClientRequest
2727
public typealias Response = HTTPClientResponse
2828
public typealias Handler = (Request, ProgressHandler?, @escaping (Result<Response, Error>) -> Void) -> Void
29-
public typealias ProgressHandler = (_ bytesReceived: Int64, _ totalBytes: Int64?) throws -> Void
30-
public typealias CompletionHandler = (Result<HTTPClientResponse, Error>) -> Void
29+
public typealias ProgressHandler = @Sendable (_ bytesReceived: Int64, _ totalBytes: Int64?) throws -> Void
30+
public typealias CompletionHandler = @Sendable (Result<HTTPClientResponse, Error>) -> Void
3131

3232
public var configuration: LegacyHTTPClientConfiguration
3333
private let underlying: Handler
@@ -121,7 +121,7 @@ public final class LegacyHTTPClient: Cancellable {
121121
requestNumber: 0,
122122
observabilityScope: observabilityScope,
123123
progress: progress.map { handler in
124-
{ received, expected in
124+
{ @Sendable received, expected in
125125
// call back on the requested queue
126126
callbackQueue.async {
127127
do {
@@ -312,7 +312,7 @@ extension LegacyHTTPClient {
312312
headers: HTTPClientHeaders = .init(),
313313
options: Request.Options = .init(),
314314
observabilityScope: ObservabilityScope? = .none,
315-
completion: @escaping (Result<Response, Error>) -> Void
315+
completion: @Sendable @escaping (Result<Response, Error>) -> Void
316316
) {
317317
self.execute(
318318
Request(method: .head, url: url, headers: headers, body: nil, options: options),
@@ -326,7 +326,7 @@ extension LegacyHTTPClient {
326326
headers: HTTPClientHeaders = .init(),
327327
options: Request.Options = .init(),
328328
observabilityScope: ObservabilityScope? = .none,
329-
completion: @escaping (Result<Response, Error>) -> Void
329+
completion: @Sendable @escaping (Result<Response, Error>) -> Void
330330
) {
331331
self.execute(
332332
Request(method: .get, url: url, headers: headers, body: nil, options: options),
@@ -341,7 +341,7 @@ extension LegacyHTTPClient {
341341
headers: HTTPClientHeaders = .init(),
342342
options: Request.Options = .init(),
343343
observabilityScope: ObservabilityScope? = .none,
344-
completion: @escaping (Result<Response, Error>) -> Void
344+
completion: @Sendable @escaping (Result<Response, Error>) -> Void
345345
) {
346346
self.execute(
347347
Request(method: .put, url: url, headers: headers, body: body, options: options),
@@ -356,7 +356,7 @@ extension LegacyHTTPClient {
356356
headers: HTTPClientHeaders = .init(),
357357
options: Request.Options = .init(),
358358
observabilityScope: ObservabilityScope? = .none,
359-
completion: @escaping (Result<Response, Error>) -> Void
359+
completion: @Sendable @escaping (Result<Response, Error>) -> Void
360360
) {
361361
self.execute(
362362
Request(method: .post, url: url, headers: headers, body: body, options: options),
@@ -370,7 +370,7 @@ extension LegacyHTTPClient {
370370
headers: HTTPClientHeaders = .init(),
371371
options: Request.Options = .init(),
372372
observabilityScope: ObservabilityScope? = .none,
373-
completion: @escaping (Result<Response, Error>) -> Void
373+
completion: @Sendable @escaping (Result<Response, Error>) -> Void
374374
) {
375375
self.execute(
376376
Request(method: .delete, url: url, headers: headers, body: nil, options: options),
@@ -383,7 +383,7 @@ extension LegacyHTTPClient {
383383
// MARK: - LegacyHTTPClientConfiguration
384384

385385
public struct LegacyHTTPClientConfiguration {
386-
public typealias AuthorizationProvider = (URL) -> String?
386+
public typealias AuthorizationProvider = @Sendable (URL) -> String?
387387

388388
public var requestHeaders: HTTPClientHeaders?
389389
public var requestTimeout: DispatchTimeInterval?

‎Sources/Basics/HTTPClient/URLSessionHTTPClient.swift

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import struct TSCUtility.Versioning
1919
import FoundationNetworking
2020
#endif
2121

22-
final class URLSessionHTTPClient {
22+
final class URLSessionHTTPClient: Sendable {
2323
private let dataTaskManager: DataTaskManager
2424
private let downloadTaskManager: DownloadTaskManager
2525

@@ -42,7 +42,7 @@ final class URLSessionHTTPClient {
4242
urlRequest: urlRequest,
4343
authorizationProvider: request.options.authorizationProvider,
4444
progress: progress,
45-
completion: continuation.resume(with:)
45+
completion: { continuation.resume(with: $0) }
4646
)
4747
case .download(_, let destination):
4848
task = self.downloadTaskManager.makeTask(
@@ -52,13 +52,14 @@ final class URLSessionHTTPClient {
5252
fileSystem: localFileSystem,
5353
destination: destination,
5454
progress: progress,
55-
completion: continuation.resume(with:)
55+
completion: { continuation.resume(with: $0) }
5656
)
5757
}
5858
task.resume()
5959
}
6060
}
6161

62+
@Sendable
6263
public func execute(
6364
_ request: LegacyHTTPClient.Request,
6465
progress: LegacyHTTPClient.ProgressHandler?,
@@ -140,8 +141,8 @@ private class WeakDataTaskManager: NSObject, URLSessionDataDelegate {
140141
}
141142
}
142143

143-
private class DataTaskManager {
144-
private var tasks = ThreadSafeKeyValueStore<Int, DataTask>()
144+
private final class DataTaskManager: @unchecked Sendable {
145+
private let tasks = ThreadSafeKeyValueStore<Int, DataTask>()
145146
private let delegateQueue: OperationQueue
146147
private var session: URLSession!
147148

@@ -179,11 +180,13 @@ private class DataTaskManager {
179180
didReceive response: URLResponse,
180181
completionHandler: @escaping (URLSession.ResponseDisposition) -> Void
181182
) {
182-
guard let task = self.tasks[dataTask.taskIdentifier] else {
183+
guard var task = self.tasks[dataTask.taskIdentifier] else {
183184
return completionHandler(.cancel)
184185
}
185186
task.response = response as? HTTPURLResponse
186187
task.expectedContentLength = response.expectedContentLength
188+
self.tasks[dataTask.taskIdentifier] = task
189+
187190
do {
188191
try task.progressHandler?(0, response.expectedContentLength)
189192
completionHandler(.allow)
@@ -193,14 +196,15 @@ private class DataTaskManager {
193196
}
194197

195198
public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
196-
guard let task = self.tasks[dataTask.taskIdentifier] else {
199+
guard var task = self.tasks[dataTask.taskIdentifier] else {
197200
return
198201
}
199202
if task.buffer != nil {
200203
task.buffer?.append(data)
201204
} else {
202205
task.buffer = data
203206
}
207+
self.tasks[dataTask.taskIdentifier] = task
204208

205209
do {
206210
// safe since created in the line above
@@ -246,7 +250,7 @@ private class DataTaskManager {
246250
completionHandler(request)
247251
}
248252

249-
class DataTask {
253+
struct DataTask: Sendable {
250254
let task: URLSessionDataTask
251255
let completionHandler: LegacyHTTPClient.CompletionHandler
252256
/// A strong reference to keep the `DataTaskManager` alive so it can handle the callbacks from the
@@ -318,9 +322,11 @@ private class WeakDownloadTaskManager: NSObject, URLSessionDownloadDelegate {
318322
}
319323
}
320324

321-
private class DownloadTaskManager {
322-
private var tasks = ThreadSafeKeyValueStore<Int, DownloadTask>()
325+
private final class DownloadTaskManager: @unchecked Sendable {
326+
private let tasks = ThreadSafeKeyValueStore<Int, DownloadTask>()
323327
private let delegateQueue: OperationQueue
328+
329+
// FIXME: can't be `let` instead of `var`, as `URLSession` holds a reference to `self`.
324330
private var session: URLSession!
325331

326332
init(configuration: URLSessionConfiguration) {
@@ -379,7 +385,7 @@ private class DownloadTaskManager {
379385
downloadTask: URLSessionDownloadTask,
380386
didFinishDownloadingTo location: URL
381387
) {
382-
guard let task = self.tasks[downloadTask.taskIdentifier] else {
388+
guard var task = self.tasks[downloadTask.taskIdentifier] else {
383389
return
384390
}
385391

@@ -392,6 +398,7 @@ private class DownloadTaskManager {
392398
try task.fileSystem.move(from: path, to: task.destination)
393399
} catch {
394400
task.moveFileError = error
401+
self.tasks[downloadTask.taskIdentifier] = task
395402
}
396403
}
397404

@@ -419,7 +426,7 @@ private class DownloadTaskManager {
419426
}
420427
}
421428

422-
class DownloadTask {
429+
struct DownloadTask: Sendable {
423430
let task: URLSessionDownloadTask
424431
let fileSystem: FileSystem
425432
let destination: AbsolutePath

‎Sources/Build/BuildOperation.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212

1313
import Basics
1414

15-
import Build
16-
1715
import LLBuildManifest
1816
import PackageGraph
1917
import PackageLoading

‎Sources/PackageRegistry/RegistryClient.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1651,7 +1651,7 @@ public final class RegistryClient: Cancellable {
16511651
result.tryMap { response in
16521652
observabilityScope
16531653
.emit(
1654-
debug: "server response for \(request.url): \(response.statusCode) in \(start.distance(to: .now()).descriptionInSeconds)"
1654+
debug: "server response for \(url): \(response.statusCode) in \(start.distance(to: .now()).descriptionInSeconds)"
16551655
)
16561656
switch response.statusCode {
16571657
case 201:

0 commit comments

Comments
 (0)