Enable strict concurrency checking in CI (#132)

Author: czechboy0

Package.swift

@@ -57,7 +57,9 @@ let package = Package(
         ),
         .target(
             name: "_TracingBenchmarkTools",
-            dependencies: [],
+            dependencies: [
+                .target(name: "Instrumentation"),
+            ],
             exclude: ["README_SWIFT.md"]
         ),
     ]

Sources/Instrumentation/InstrumentationSystem.swift

@@ -24,56 +24,78 @@ import ServiceContextModule
 /// ``instrument``: Returns whatever you passed to ``bootstrap(_:)`` as an ``Instrument``.
 @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) // for TaskLocal ServiceContext
 public enum InstrumentationSystem {
-    private static let lock = ReadWriteLock()
-    private static var _instrument: Instrument = NoOpInstrument()
-    private static var isInitialized = false
+    /// Marked as @unchecked Sendable due to the synchronization being
+    /// performed manually using locks.
+    private final class Storage: @unchecked Sendable {
+        private let lock = ReadWriteLock()
+        private var _instrument: Instrument = NoOpInstrument()
+        private var _isInitialized = false
+
+        func bootstrap(_ instrument: Instrument) {
+            self.lock.withWriterLock {
+                precondition(
+                    !self._isInitialized, """
+                    InstrumentationSystem can only be initialized once per process. Consider using MultiplexInstrument if
+                    you need to use multiple instruments.
+                    """
+                )
+                self._instrument = instrument
+                self._isInitialized = true
+            }
+        }
+
+        func bootstrapInternal(_ instrument: Instrument?) {
+            self.lock.withWriterLock {
+                self._instrument = instrument ?? NoOpInstrument()
+            }
+        }
+
+        var instrument: Instrument {
+            self.lock.withReaderLock { self._instrument }
+        }
+
+        func _findInstrument(where predicate: (Instrument) -> Bool) -> Instrument? {
+            self.lock.withReaderLock {
+                if let multiplex = self._instrument as? MultiplexInstrument {
+                    return multiplex.firstInstrument(where: predicate)
+                } else if predicate(self._instrument) {
+                    return self._instrument
+                } else {
+                    return nil
+                }
+            }
+        }
+    }
+
+    private static let shared = Storage()
 
     /// Globally select the desired ``Instrument`` implementation.
     ///
     /// - Parameter instrument: The ``Instrument`` you want to share globally within your system.
     /// - Warning: Do not call this method more than once. This will lead to a crash.
     public static func bootstrap(_ instrument: Instrument) {
-        self.lock.withWriterLock {
-            precondition(
-                !self.isInitialized, """
-                InstrumentationSystem can only be initialized once per process. Consider using MultiplexInstrument if
-                you need to use multiple instruments.
-                """
-            )
-            self._instrument = instrument
-            self.isInitialized = true
-        }
+        self.shared.bootstrap(instrument)
     }
 
     /// For testing scenarios one may want to set instruments multiple times, rather than the set-once semantics enforced by ``bootstrap(_:)``.
     ///
     /// - Parameter instrument: the instrument to boostrap the system with, if `nil` the ``NoOpInstrument`` is bootstrapped.
     internal static func bootstrapInternal(_ instrument: Instrument?) {
-        self.lock.withWriterLock {
-            self._instrument = instrument ?? NoOpInstrument()
-        }
+        self.shared.bootstrapInternal(instrument)
     }
 
     /// Returns the globally configured ``Instrument``.
     ///
     /// Defaults to a no-op ``Instrument`` if ``bootstrap(_:)`` wasn't called before.
     public static var instrument: Instrument {
-        self.lock.withReaderLock { self._instrument }
+        shared.instrument
     }
 }
 
 @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) // for TaskLocal ServiceContext
 extension InstrumentationSystem {
     /// INTERNAL API: Do Not Use
     public static func _findInstrument(where predicate: (Instrument) -> Bool) -> Instrument? {
-        self.lock.withReaderLock {
-            if let multiplex = self._instrument as? MultiplexInstrument {
-                return multiplex.firstInstrument(where: predicate)
-            } else if predicate(self._instrument) {
-                return self._instrument
-            } else {
-                return nil
-            }
-        }
+        self.shared._findInstrument(where: predicate)
     }
 }

Sources/Instrumentation/Locks.swift

@@ -37,7 +37,8 @@ import Glibc
 /// This object provides a lock on top of a single `pthread_mutex_t`. This kind
 /// of lock is safe to use with `libpthread`-based threading models, such as the
 /// one used by NIO.
-internal final class ReadWriteLock {
+@_spi(Locking) /* Use the `package` access modifier once min Swift version is increased. */
+public final class ReadWriteLock {
     private let rwlock: UnsafeMutablePointer<pthread_rwlock_t> = UnsafeMutablePointer.allocate(capacity: 1)
 
     /// Create a new lock.
@@ -90,7 +91,7 @@ extension ReadWriteLock {
     /// - Parameter body: The block to execute while holding the lock.
     /// - Returns: The value returned by the block.
     @inlinable
-    func withReaderLock<T>(_ body: () throws -> T) rethrows -> T {
+    public func withReaderLock<T>(_ body: () throws -> T) rethrows -> T {
         self.lockRead()
         defer {
             self.unlock()
@@ -107,11 +108,30 @@ extension ReadWriteLock {
     /// - Parameter body: The block to execute while holding the lock.
     /// - Returns: The value returned by the block.
     @inlinable
-    func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
+    public func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
         self.lockWrite()
         defer {
             self.unlock()
         }
         return try body()
     }
 }
+
+/// A wrapper providing locked access to a value.
+///
+/// Marked as @unchecked Sendable due to the synchronization being
+/// performed manually using locks.
+@_spi(Locking) /* Use the `package` access modifier once min Swift version is increased. */
+public final class LockedValueBox<Value: Sendable>: @unchecked Sendable {
+    private let lock = ReadWriteLock()
+    private var value: Value
+    public init(_ value: Value) {
+        self.value = value
+    }
+
+    public func withValue<R>(_ work: (inout Value) throws -> R) rethrows -> R {
+        try self.lock.withWriterLock {
+            try work(&self.value)
+        }
+    }
+}

Sources/Tracing/NoOpTracer.swift

@@ -23,14 +23,15 @@ public struct NoOpTracer: LegacyTracer {
 
     public init() {}
 
-    public func startAnySpan<Instant: TracerInstant>(_ operationName: String,
-                                                     context: @autoclosure () -> ServiceContext,
-                                                     ofKind kind: SpanKind,
-                                                     at instant: @autoclosure () -> Instant,
-                                                     function: String,
-                                                     file fileID: String,
-                                                     line: UInt) -> any Tracing.Span
-    {
+    public func startAnySpan<Instant: TracerInstant>(
+        _ operationName: String,
+        context: @autoclosure () -> ServiceContext,
+        ofKind kind: SpanKind,
+        at instant: @autoclosure () -> Instant,
+        function: String,
+        file fileID: String,
+        line: UInt
+    ) -> any Tracing.Span {
         NoOpSpan(context: context())
     }
 

Sources/Tracing/SpanProtocol.swift

@@ -79,9 +79,11 @@ public protocol Span: _SwiftTracingSendableSpan {
     ///   - error: The error to be recorded
     ///   - attributes: Additional attributes describing the error
     ///   - instant: the time instant at which the event occurred
-    func recordError<Instant: TracerInstant>(_ error: Error,
-                                             attributes: SpanAttributes,
-                                             at instant: @autoclosure () -> Instant)
+    func recordError<Instant: TracerInstant>(
+        _ error: Error,
+        attributes: SpanAttributes,
+        at instant: @autoclosure () -> Instant
+    )
 
     /// The attributes describing this `Span`.
     var attributes: SpanAttributes {
@@ -185,18 +187,20 @@ public struct SpanEvent: Equatable {
     ///   - name: The human-readable name of this event.
     ///   - attributes: attributes describing this event. Defaults to no attributes.
     ///   - instant: the time instant at which the event occurred
-    public init<Instant: TracerInstant>(name: String,
-                                        at instant: @autoclosure () -> Instant,
-                                        attributes: SpanAttributes = [:])
-    {
+    public init<Instant: TracerInstant>(
+        name: String,
+        at instant: @autoclosure () -> Instant,
+        attributes: SpanAttributes = [:]
+    ) {
         self.name = name
         self.attributes = attributes
         self.nanosecondsSinceEpoch = instant().nanosecondsSinceEpoch
     }
 
-    public init(name: String,
-                attributes: SpanAttributes = [:])
-    {
+    public init(
+        name: String,
+        attributes: SpanAttributes = [:]
+    ) {
         self.name = name
         self.attributes = attributes
         self.nanosecondsSinceEpoch = DefaultTracerClock.now.nanosecondsSinceEpoch

Sources/_TracingBenchmarkTools/ArgParser.swift

@@ -25,6 +25,7 @@
 //===----------------------------------------------------------------------===//
 
 import Foundation
+@_spi(Locking) import Instrumentation
 
 enum ArgumentError: Error {
     case missingValue(String)
@@ -81,7 +82,7 @@ class ArgumentParser<U> {
     private var arguments: [Argument] = []
     private let programName: String = {
         // Strip full path from the program name.
-        let r = CommandLine.arguments[0].reversed()
+        let r = ProcessInfo.processInfo.arguments[0].reversed()
         let ss = r[r.startIndex ..< (r.firstIndex(of: "/") ?? r.endIndex)]
         return String(ss.reversed())
     }()
@@ -150,10 +151,14 @@ class ArgumentParser<U> {
             try self.arguments.forEach { try $0.apply() } // type-check and store values
             return self.result
         } catch let error as ArgumentError {
-            fputs("error: \(error)\n", stderr)
+            lockedStderr.withValue { stderr in
+                _ = fputs("error: \(error)\n", stderr)
+            }
             exit(1)
         } catch {
-            fflush(stdout)
+            lockedStdout.withValue { stdout in
+                _ = fflush(stdout)
+            }
             fatalError("\(error)")
         }
     }
@@ -173,7 +178,8 @@ class ArgumentParser<U> {
     ///     the supported argument syntax.
     private func parseArgs() throws {
         // For each argument we are passed...
-        for arg in CommandLine.arguments[1 ..< CommandLine.arguments.count] {
+        let arguments = ProcessInfo.processInfo.arguments
+        for arg in arguments[1 ..< arguments.count] {
             // If the argument doesn't match the optional argument pattern. Add
             // it to the positional argument list and continue...
             if !arg.starts(with: "-") {

Sources/_TracingBenchmarkTools/BenchmarkCategory.swift

@@ -18,7 +18,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-public enum BenchmarkCategory: String {
+public enum BenchmarkCategory: String, Sendable {
     // Most benchmarks are assumed to be "stable" and will be regularly tracked at
     // each commit. A handful may be marked unstable if continually tracking them is
     // counterproductive.

Sources/_TracingBenchmarkTools/BenchmarkTools.swift

@@ -23,6 +23,8 @@ import Glibc
 #else
 import Darwin
 #endif
+import Foundation
+@_spi(Locking) import Instrumentation
 
 extension BenchmarkCategory: CustomStringConvertible {
     public var description: String {
@@ -36,7 +38,7 @@ extension BenchmarkCategory: Comparable {
     }
 }
 
-public struct BenchmarkPlatformSet: OptionSet {
+public struct BenchmarkPlatformSet: OptionSet, Sendable {
     public let rawValue: Int
 
     public init(rawValue: Int) {
@@ -59,12 +61,12 @@ public struct BenchmarkPlatformSet: OptionSet {
     }
 }
 
-public struct BenchmarkInfo {
+public struct BenchmarkInfo: Sendable {
     /// The name of the benchmark that should be displayed by the harness.
     public var name: String
 
     /// Shadow static variable for runFunction.
-    private var _runFunction: (Int) -> Void
+    private var _runFunction: @Sendable (Int) -> Void
 
     /// A function that invokes the specific benchmark routine.
     public var runFunction: ((Int) -> Void)? {
@@ -83,7 +85,7 @@ public struct BenchmarkInfo {
     private var unsupportedPlatforms: BenchmarkPlatformSet
 
     /// Shadow variable for setUpFunction.
-    private var _setUpFunction: (() -> Void)?
+    private var _setUpFunction: (@Sendable () -> Void)?
 
     /// An optional function that if non-null is run before benchmark samples
     /// are timed.
@@ -95,7 +97,7 @@ public struct BenchmarkInfo {
     }
 
     /// Shadow static variable for computed property tearDownFunction.
-    private var _tearDownFunction: (() -> Void)?
+    private var _tearDownFunction: (@Sendable () -> Void)?
 
     /// An optional function that if non-null is run after samples are taken.
     public var tearDownFunction: (() -> Void)? {
@@ -108,9 +110,9 @@ public struct BenchmarkInfo {
     public var legacyFactor: Int?
 
     public init(
-        name: String, runFunction: @escaping (Int) -> Void, tags: [BenchmarkCategory],
-        setUpFunction: (() -> Void)? = nil,
-        tearDownFunction: (() -> Void)? = nil,
+        name: String, runFunction: @escaping @Sendable (Int) -> Void, tags: [BenchmarkCategory],
+        setUpFunction: (@Sendable () -> Void)? = nil,
+        tearDownFunction: (@Sendable () -> Void)? = nil,
         unsupportedPlatforms: BenchmarkPlatformSet = [],
         legacyFactor: Int? = nil
     ) {
@@ -168,17 +170,28 @@ struct LFSR {
     }
 }
 
-var lfsrRandomGenerator = LFSR()
+let lfsrRandomGenerator: LockedValueBox<LFSR> = .init(LFSR())
 
 // Start the generator from the beginning
 public func SRand() {
-    lfsrRandomGenerator = LFSR()
+    lfsrRandomGenerator.withValue { lfsr in
+        lfsr = LFSR()
+    }
 }
 
 public func Random() -> Int64 {
-    lfsrRandomGenerator.randInt()
+    lfsrRandomGenerator.withValue { lfsr in
+        lfsr.randInt()
+    }
 }
 
+// Can't access stdout/stderr directly in strict concurrency checking mode.
+// let lockedStdout = LockedValueBox(stdout)
+// let lockedStderr = LockedValueBox(stderr)
+
+let lockedStdout = LockedValueBox(fdopen(STDOUT_FILENO, "w"))
+let lockedStderr = LockedValueBox(fdopen(STDERR_FILENO, "w"))
+
 @inlinable
 @inline(__always)
 public func CheckResults(