Skip to content

Basics: make ObservabilityScope conform to Sendable #6089

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

Merged
merged 65 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
2d1950d
Basics: add AsyncFileSystem and AsyncLocalFileSystem
MaxDesiatov Jan 18, 2023
a0a13b8
Add `swift(>=5.5.2)` check for new actor types
MaxDesiatov Jan 19, 2023
979e1b9
Convert `AsyncFileSystem` to a wrapper actor
MaxDesiatov Jan 19, 2023
b864f61
[NFC] Basics: add `AsyncBox` and `TokenBucket` helpers
MaxDesiatov Jan 24, 2023
535d305
Apply `swiftformat` to new files
MaxDesiatov Jan 24, 2023
703c72a
Apply updated formatting rules
MaxDesiatov Jan 24, 2023
be62cfb
Update wrong copyright year in `AsyncBox.swift`
MaxDesiatov Jan 24, 2023
dd14a17
Rename `AsyncBox` to `SendableBox`, fix formatting
MaxDesiatov Jan 25, 2023
707f482
Fix bad merge
MaxDesiatov Jan 31, 2023
ec903bc
HTTPClient: add `async` overload for `execute` function
MaxDesiatov Jan 6, 2023
58cecf1
Basics: update `CMakeLists.txt` for `HTTPClient`-related changes
MaxDesiatov Jan 6, 2023
8c0c954
Add `@available` annotation on new `execute` overload
MaxDesiatov Jan 6, 2023
966bf98
Add `import _Concurrency`
MaxDesiatov Jan 6, 2023
0826ab0
Move `import _Concurrency` to an appropriate file
MaxDesiatov Jan 8, 2023
07606ac
Add tests for `async` code on `URLSessionHTTPClient`
MaxDesiatov Jan 10, 2023
115b35b
Fix all async tests
MaxDesiatov Jan 11, 2023
46eff7a
Fix tests relying on `HTTPClient` to validate response codes
MaxDesiatov Jan 11, 2023
ae3078c
Fix more tests relying `HTTPClient` status code validation
MaxDesiatov Jan 11, 2023
086c6d2
Fix formatting
MaxDesiatov Jan 12, 2023
394ef80
Rename `HTTPClient` to `LegacyHTTPClient`
MaxDesiatov Jan 19, 2023
bcca0ca
Fix async `withTemporaryDirectory` availability
MaxDesiatov Jan 19, 2023
5f1982c
Add missing `pow` import
MaxDesiatov Jan 20, 2023
cbf688b
Fix use of `LegacyHTTPClient` in `PackageCollectionsSigning`
MaxDesiatov Jan 20, 2023
439f156
Fix references to `HTTPClient` w/o Concurrency back-deployment
MaxDesiatov Jan 20, 2023
ac2d13a
`AsyncFileSystem` needs `#if swift(>=5.5.2)` check
MaxDesiatov Jan 20, 2023
a3b16d7
Allow non-Sendable LegacyHTTPClientConfiguration.AuthorizationProvider
MaxDesiatov Jan 20, 2023
5f92fbe
Make some `Sendable` types available without back-deployment
MaxDesiatov Jan 20, 2023
6627e96
Use `LegacyHTTPClientConfiguration` in `CertificatePolicy`
MaxDesiatov Jan 20, 2023
749f954
Fix `LegacyHTTPClientConfiguration` use in `CertificatePolicy`
MaxDesiatov Jan 20, 2023
bb6937e
Fix build error in `RegistryClient.swift`
MaxDesiatov Jan 20, 2023
87dbc4b
Make `Data: UnsafeSendable` for older compilers
MaxDesiatov Jan 20, 2023
4e2b897
Replace platform availability w/ Swift version availability
MaxDesiatov Jan 20, 2023
85c3384
Add tests for new `async`-friendly `HTTPClient`
MaxDesiatov Jan 20, 2023
c23e891
Start circuit breaker strategy implementation
MaxDesiatov Jan 21, 2023
ac95a81
Convert last remaining tests to `async`/`await`
MaxDesiatov Jan 23, 2023
43896e6
Fix doc comment and formatting
MaxDesiatov Jan 23, 2023
6ab505e
Remove duplicated `private actor TokenBucket`
MaxDesiatov Jan 24, 2023
219ede1
Fully replace uses of `AsyncBox` with `SendableBox`
MaxDesiatov Jan 31, 2023
933cb0a
Basics: make `ObservabilityScope` conform to `Sendable`
MaxDesiatov Jan 31, 2023
b2dda89
Removed unused `ThreadSafeKeyValueStore/forEach`
MaxDesiatov Jan 31, 2023
e7c2a10
Address PR feedback
MaxDesiatov Jan 31, 2023
9b01871
Reduce the diff
MaxDesiatov Jan 31, 2023
4f2be3f
More diff reductions
MaxDesiatov Jan 31, 2023
9294fd1
Add missing comment
MaxDesiatov Jan 31, 2023
1808755
Reduce the diff even more
MaxDesiatov Jan 31, 2023
38113f3
Re-enable `VFSTests` with all compiler versions
MaxDesiatov Jan 31, 2023
3f758bf
Reduce `JSONPackageCollectionProviderTests` diff
MaxDesiatov Jan 31, 2023
53ccd58
Diff reductions in tests
MaxDesiatov Jan 31, 2023
107d5a8
Merge branch 'maxd/httpclient-async-execute' of github.com:apple/swif…
MaxDesiatov Feb 1, 2023
59e4c29
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Feb 1, 2023
33d8f33
Add more `Sendable` requirements to `ObservabilityMetadata`
MaxDesiatov Feb 1, 2023
34f4f9e
Add more `Sendable` constraints
MaxDesiatov Feb 1, 2023
b8b502a
More `Sendable` constraints to get rid of warnings
MaxDesiatov Feb 1, 2023
a0121b9
Rename `implementation` -> `underlying`
MaxDesiatov Feb 1, 2023
acb2341
Make `ThreadSafeKeyValueStore` conditionally `Sendable`
MaxDesiatov Feb 1, 2023
fd35c01
Change `SendableTimeInterval` to typealias `DispatchTimeInterval`
MaxDesiatov Feb 2, 2023
1fcb4a4
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Feb 2, 2023
85c5de3
Use `UnsafeSendable` on `DispatchTimeInterval` when `swift(<5.7)`
MaxDesiatov Feb 2, 2023
2017a38
Make `FileLocation` conform to `Sendable`
MaxDesiatov Feb 2, 2023
1ec5ff9
Fix `SwiftToolObservabilityHandler.OutputHandler`
MaxDesiatov Feb 2, 2023
62b67ed
Change `InteractiveWriter` back to use of `OutputByteStream`
MaxDesiatov Feb 2, 2023
ed8f315
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Feb 2, 2023
79c4251
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Mar 10, 2023
61ee823
Reduce the diff by removing unrelated changes
MaxDesiatov Mar 10, 2023
1ad908f
Remove unused compiler version checks
MaxDesiatov Mar 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions Sources/Basics/HTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ public actor HTTPClient {
///
/// - Parameters:
/// - request: The ``HTTPClientRequest`` to perform.
/// - observabilityScope: the observability scope to emit diagnostics on.
/// - progress: A closure to handle response download progress.
/// - Returns: A response value returned by underlying ``HTTPClient.Implementation``.
public func execute(
_ request: Request,
observabilityScope: ObservabilityScope? = nil,
progress: ProgressHandler? = nil
) async throws -> Response {
// merge configuration
Expand Down Expand Up @@ -85,16 +87,18 @@ public actor HTTPClient {
request.headers.add(name: "Authorization", value: authorization)
}

return try await executeWithStrategies(request: request, progress: progress, requestNumber: 0)
return try await executeWithStrategies(request: request, requestNumber: 0, observabilityScope, progress)
}

private func executeWithStrategies(
request: Request,
progress: ProgressHandler?,
requestNumber: Int
requestNumber: Int,
_ observabilityScope: ObservabilityScope?,
_ progress: ProgressHandler?
) async throws -> Response {
// apply circuit breaker if necessary
if self.shouldCircuitBreak(request: request) {
observabilityScope?.emit(warning: "Circuit breaker triggered for \(request.url)")
throw HTTPClientError.circuitBreakerTriggered
}

Expand All @@ -115,17 +119,19 @@ public actor HTTPClient {
self.recordErrorIfNecessary(response: response, request: request)

// handle retry strategy
if let retryDelayInNanoseconds = self.calculateRetry(
if let retryDelay = self.calculateRetry(
response: response,
request: request,
requestNumber: requestNumber
)?.nanoseconds() {
), let retryDelayInNanoseconds = retryDelay.nanoseconds() {
observabilityScope?.emit(warning: "\(request.url) failed, retrying in \(retryDelay)")
try await Task.sleep(nanoseconds: UInt64(retryDelayInNanoseconds))

return try await self.executeWithStrategies(
request: request,
progress: progress,
requestNumber: requestNumber + 1
requestNumber: requestNumber + 1,
observabilityScope,
progress
)
}
// check for valid response codes
Expand Down
36 changes: 18 additions & 18 deletions Sources/Basics/Observability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@ public class ObservabilitySystem {
}

/// Create an ObservabilitySystem with a single diagnostics handler.
public convenience init(_ handler: @escaping (ObservabilityScope, Diagnostic) -> Void) {
public convenience init(_ handler: @escaping @Sendable (ObservabilityScope, Diagnostic) -> Void) {
self.init(SingleDiagnosticsHandler(handler))
}

private struct SingleDiagnosticsHandler: ObservabilityHandlerProvider, DiagnosticsHandler {
var diagnosticsHandler: DiagnosticsHandler { self }
var diagnosticsHandler: DiagnosticsHandler { self }

let underlying: (ObservabilityScope, Diagnostic) -> Void
let underlying: @Sendable (ObservabilityScope, Diagnostic) -> Void

init(_ underlying: @escaping (ObservabilityScope, Diagnostic) -> Void) {
init(_ underlying: @escaping @Sendable (ObservabilityScope, Diagnostic) -> Void) {
self.underlying = underlying
}

Expand All @@ -61,12 +61,12 @@ public protocol ObservabilityHandlerProvider {

// MARK: - ObservabilityScope

public final class ObservabilityScope: DiagnosticsEmitterProtocol, CustomStringConvertible {
public final class ObservabilityScope: DiagnosticsEmitterProtocol, Sendable, CustomStringConvertible {
public let description: String
private let parent: ObservabilityScope?
private let metadata: ObservabilityMetadata?

private var diagnosticsHandler: DiagnosticsHandlerWrapper
private let diagnosticsHandler: DiagnosticsHandlerWrapper

fileprivate init(
description: String,
Expand Down Expand Up @@ -150,7 +150,7 @@ public final class ObservabilityScope: DiagnosticsEmitterProtocol, CustomStringC

// MARK: - Diagnostics

public protocol DiagnosticsHandler {
public protocol DiagnosticsHandler: Sendable {
func handleDiagnostic(scope: ObservabilityScope, diagnostic: Diagnostic)
}

Expand Down Expand Up @@ -252,7 +252,7 @@ public struct DiagnosticsEmitter: DiagnosticsEmitterProtocol {
}
}

public struct Diagnostic: CustomStringConvertible {
public struct Diagnostic: Sendable, CustomStringConvertible {
public let severity: Severity
public let message: String
public internal (set) var metadata: ObservabilityMetadata?
Expand Down Expand Up @@ -324,7 +324,7 @@ public struct Diagnostic: CustomStringConvertible {
Self(severity: .debug, message: message.description, metadata: metadata)
}

public enum Severity: Comparable {
public enum Severity: Comparable, Sendable {
case error
case warning
case info
Expand Down Expand Up @@ -380,10 +380,10 @@ public struct Diagnostic: CustomStringConvertible {
// FIXME: we currently require that Value conforms to CustomStringConvertible which sucks
// ideally Value would conform to Equatable but that has generic requirement
// luckily, this is about to change so we can clean this up soon
public struct ObservabilityMetadata: CustomDebugStringConvertible {
public struct ObservabilityMetadata: Sendable, CustomDebugStringConvertible {
public typealias Key = ObservabilityMetadataKey

private var _storage = [AnyKey: Any]()
private var _storage = [AnyKey: Sendable]()

public init() {}

Expand Down Expand Up @@ -414,7 +414,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {
///
/// - Parameter body: The closure to be invoked for each item stored in this `ObservabilityMetadata`,
/// passing the type-erased key and the associated value.
public func forEach(_ body: (AnyKey, Any) throws -> Void) rethrows {
public func forEach(_ body: (AnyKey, Sendable) throws -> Void) rethrows {
try self._storage.forEach { key, value in
try body(key, value)
}
Expand Down Expand Up @@ -473,7 +473,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {
}

/// A type-erased `ObservabilityMetadataKey` used when iterating through the `ObservabilityMetadata` using its `forEach` method.
public struct AnyKey {
public struct AnyKey: Sendable {
/// The key's type represented erased to an `Any.Type`.
public let keyType: Any.Type

Expand All @@ -485,7 +485,7 @@ public struct ObservabilityMetadata: CustomDebugStringConvertible {

public protocol ObservabilityMetadataKey {
/// The type of value uniquely identified by this key.
associatedtype Value
associatedtype Value: Sendable
}

extension ObservabilityMetadata.AnyKey: Hashable {
Expand Down Expand Up @@ -569,11 +569,11 @@ extension ObservabilityMetadata {
}
}

private enum LegacyLocationKey: Key {
private enum LegacyLocationKey: Key, Sendable {
typealias Value = DiagnosticLocationWrapper
}

public struct DiagnosticLocationWrapper: CustomStringConvertible {
public struct DiagnosticLocationWrapper: Sendable, CustomStringConvertible {
let underlying: DiagnosticLocation

public init (_ underlying: DiagnosticLocation) {
Expand All @@ -597,11 +597,11 @@ extension ObservabilityMetadata {
}
}

private enum LegacyDataKey: Key {
private enum LegacyDataKey: Key, Sendable {
typealias Value = DiagnosticDataWrapper
}

struct DiagnosticDataWrapper: CustomStringConvertible {
struct DiagnosticDataWrapper: Sendable, CustomStringConvertible {
let underlying: DiagnosticData

public init (_ underlying: DiagnosticData) {
Expand Down
105 changes: 4 additions & 101 deletions Sources/Basics/SendableTimeInterval.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,106 +11,9 @@
//===----------------------------------------------------------------------===//

import enum Dispatch.DispatchTimeInterval
import struct Foundation.TimeInterval

public enum SendableTimeInterval: Equatable, Sendable {
case seconds(Int)
case milliseconds(Int)
case microseconds(Int)
case nanoseconds(Int)
case never
extension DispatchTimeInterval: @unchecked Sendable {}

public func timeInterval() -> TimeInterval? {
switch self {
case .seconds(let value):
return Double(value)
case .milliseconds(let value):
return Double(value) / 1000
case .microseconds(let value):
return Double(value) / 1_000_000
case .nanoseconds(let value):
return Double(value) / 1_000_000_000
default:
return nil
}
}

public func nanoseconds() -> Int? {
switch self {
case .seconds(let value):
return value.multipliedReportingOverflow(by: 1_000_000_000).partialValue
case .milliseconds(let value):
return value.multipliedReportingOverflow(by: 1_000_000).partialValue
case .microseconds(let value):
return value.multipliedReportingOverflow(by: 1000).partialValue
case .nanoseconds(let value):
return value
default:
return nil
}
}

public func milliseconds() -> Int? {
switch self {
case .seconds(let value):
return value.multipliedReportingOverflow(by: 1000).partialValue
case .milliseconds(let value):
return value
case .microseconds(let value):
return Int(Double(value) / 1000)
case .nanoseconds(let value):
return Int(Double(value) / 1_000_000)
default:
return nil
}
}

public func seconds() -> Int? {
switch self {
case .seconds(let value):
return value
case .milliseconds(let value):
return Int(Double(value) / 1000)
case .microseconds(let value):
return Int(Double(value) / 1_000_000)
case .nanoseconds(let value):
return Int(Double(value) / 1_000_000_000)
default:
return nil
}
}

public var descriptionInSeconds: String {
switch self {
case .seconds(let value):
return "\(value)s"
case .milliseconds(let value):
return String(format: "%.2f", Double(value)/Double(1000)) + "s"
case .microseconds(let value):
return String(format: "%.2f", Double(value)/Double(1_000_000)) + "s"
case .nanoseconds(let value):
return String(format: "%.2f", Double(value)/Double(1_000_000_000)) + "s"
case .never:
return "n/a"
}
}
}

public extension SendableTimeInterval {
init(_ interval: DispatchTimeInterval) {
switch interval {
case let .seconds(s):
self = .seconds(s)
case let .milliseconds(ms):
self = .milliseconds(ms)
case let .microseconds(us):
self = .microseconds(us)
case let .nanoseconds(ns):
self = .nanoseconds(ns)
case .never:
self = .never
@unknown default:
fatalError("unknown case of `DispatchTimeInterval`")
}
}
}
/// This typealias hides `DispatchTimeInterval` as an implementation detail until we can use `Swift.Duration`, as the
/// latter requires macOS 13.
public typealias SendableTimeInterval = DispatchTimeInterval
5 changes: 4 additions & 1 deletion Sources/CoreCommands/SwiftToolObservabilityHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public struct SwiftToolObservabilityHandler: ObservabilityHandlerProvider {
self.outputHandler.wait(timeout: timeout)
}

struct OutputHandler: DiagnosticsHandler {
struct OutputHandler {
private let logLevel: Diagnostic.Severity
internal let outputStream: ThreadSafeOutputByteStream
private let writer: InteractiveWriter
Expand Down Expand Up @@ -159,6 +159,9 @@ public struct SwiftToolObservabilityHandler: ObservabilityHandlerProvider {
}
}

extension SwiftToolObservabilityHandler.OutputHandler: @unchecked Sendable {}
extension SwiftToolObservabilityHandler.OutputHandler: DiagnosticsHandler {}

/// This type is used to write on the underlying stream.
///
/// If underlying stream is a not tty, the string will be written in without any
Expand Down
2 changes: 1 addition & 1 deletion Sources/SPMBuildCore/PluginInvocation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ extension ObservabilityMetadata {
}
}

public struct FileLocation: Equatable, CustomStringConvertible {
public struct FileLocation: Equatable, CustomStringConvertible, Sendable {
public let file: AbsolutePath
public let line: Int?

Expand Down