Skip to content

Enable strict concurrency checking in CI #132

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 3 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ let package = Package(
),
.target(
name: "_TracingBenchmarkTools",
dependencies: [],
dependencies: [
.target(name: "Instrumentation"),
],
exclude: ["README_SWIFT.md"]
),
]
Expand Down
74 changes: 48 additions & 26 deletions Sources/Instrumentation/InstrumentationSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: We can SPI this, I'll take that on :)

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)
}
}
26 changes: 23 additions & 3 deletions Sources/Instrumentation/Locks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand All @@ -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)
}
}
}
17 changes: 9 additions & 8 deletions Sources/Tracing/NoOpTracer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
24 changes: 14 additions & 10 deletions Sources/Tracing/SpanProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions Sources/_TracingBenchmarkTools/ArgParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
//===----------------------------------------------------------------------===//

import Foundation
@_spi(Locking) import Instrumentation

enum ArgumentError: Error {
case missingValue(String)
Expand Down Expand Up @@ -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())
}()
Expand Down Expand Up @@ -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)")
}
}
Expand All @@ -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: "-") {
Expand Down
2 changes: 1 addition & 1 deletion Sources/_TracingBenchmarkTools/BenchmarkCategory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
35 changes: 24 additions & 11 deletions Sources/_TracingBenchmarkTools/BenchmarkTools.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import Glibc
#else
import Darwin
#endif
import Foundation
@_spi(Locking) import Instrumentation

extension BenchmarkCategory: CustomStringConvertible {
public var description: String {
Expand All @@ -36,7 +38,7 @@ extension BenchmarkCategory: Comparable {
}
}

public struct BenchmarkPlatformSet: OptionSet {
public struct BenchmarkPlatformSet: OptionSet, Sendable {
public let rawValue: Int

public init(rawValue: Int) {
Expand All @@ -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)? {
Expand All @@ -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.
Expand All @@ -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)? {
Expand All @@ -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
) {
Expand Down Expand Up @@ -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(
Expand Down
Loading