Use os_unfair_lock on Darwin when available. (#942)

grynspan · web-flow · commit 63eb1d97e305 · 2025-02-06T09:42:59.000-05:00
This PR refactors the `Locked` type to be generic over the type of lock it uses, allowing us to adopt `os_unfair_lock` in most places, but also allowing us to use other lock types as needed (i.e. `pthread_mutex_t` in WaitFor.swift if libdispatch is unavailable.) Under `-O`, locking and unlocking is fully inlined, while initialization (which is relatively rare) is partially, but not completely, inlined. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
diff --git a/Sources/Testing/CMakeLists.txt b/Sources/Testing/CMakeLists.txt
@@ -79,6 +79,7 @@ add_library(Testing
   Support/Graph.swift
   Support/JSON.swift
   Support/Locked.swift
+  Support/Locked+Platform.swift
   Support/SystemError.swift
   Support/Versions.swift
   Discovery.swift
diff --git a/Sources/Testing/ExitTests/WaitFor.swift b/Sources/Testing/ExitTests/WaitFor.swift
@@ -80,7 +80,7 @@ func wait(for pid: consuming pid_t) async throws -> ExitCondition {
 }
 #elseif SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD)
 /// A mapping of awaited child PIDs to their corresponding Swift continuations.
-private let _childProcessContinuations = Locked<[pid_t: CheckedContinuation<ExitCondition, any Error>]>()
+private let _childProcessContinuations = LockedWith<pthread_mutex_t, [pid_t: CheckedContinuation<ExitCondition, any Error>]>()
 
 /// A condition variable used to suspend the waiter thread created by
 /// `_createWaitThread()` when there are no child processes to await.
@@ -137,7 +137,7 @@ private let _createWaitThread: Void = {
       // newly-scheduled waiter process. (If this condition is spuriously
       // woken, we'll just loop again, which is fine.) Note that we read errno
       // outside the lock in case acquiring the lock perturbs it.
-      _childProcessContinuations.withUnsafePlatformLock { lock, childProcessContinuations in
+      _childProcessContinuations.withUnsafeUnderlyingLock { lock, childProcessContinuations in
         if childProcessContinuations.isEmpty {
           _ = pthread_cond_wait(_waitThreadNoChildrenCondition, lock)
         }
diff --git a/Sources/Testing/Support/Locked+Platform.swift b/Sources/Testing/Support/Locked+Platform.swift
@@ -0,0 +1,96 @@
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2023–2025 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+//
+
+internal import _TestingInternals
+
+extension Never: Lockable {
+  static func initializeLock(at lock: UnsafeMutablePointer<Self>) {}
+  static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {}
+  static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {}
+  static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {}
+}
+
+#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK
+extension os_unfair_lock_s: Lockable {
+  static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
+    lock.initialize(to: .init())
+  }
+
+  static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
+    // No deinitialization needed.
+  }
+
+  static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
+    os_unfair_lock_lock(lock)
+  }
+
+  static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
+    os_unfair_lock_unlock(lock)
+  }
+}
+#endif
+
+#if os(FreeBSD) || os(OpenBSD)
+typealias pthread_mutex_t = _TestingInternals.pthread_mutex_t?
+#endif
+
+#if SWT_TARGET_OS_APPLE || os(Linux) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded)) || os(FreeBSD) || os(OpenBSD)
+extension pthread_mutex_t: Lockable {
+  static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
+    _ = pthread_mutex_init(lock, nil)
+  }
+
+  static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
+    _ = pthread_mutex_destroy(lock)
+  }
+
+  static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
+    _ = pthread_mutex_lock(lock)
+  }
+
+  static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
+    _ = pthread_mutex_unlock(lock)
+  }
+}
+#endif
+
+#if os(Windows)
+extension SRWLOCK: Lockable {
+  static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
+    InitializeSRWLock(lock)
+  }
+
+  static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
+    // No deinitialization needed.
+  }
+
+  static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
+    AcquireSRWLockExclusive(lock)
+  }
+
+  static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
+    ReleaseSRWLockExclusive(lock)
+  }
+}
+#endif
+
+#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK
+typealias DefaultLock = os_unfair_lock
+#elseif SWT_TARGET_OS_APPLE || os(Linux) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded)) || os(FreeBSD) || os(OpenBSD)
+typealias DefaultLock = pthread_mutex_t
+#elseif os(Windows)
+typealias DefaultLock = SRWLOCK
+#elseif os(WASI)
+// No locks on WASI without multithreaded runtime.
+typealias DefaultLock = Never
+#else
+#warning("Platform-specific implementation missing: locking unavailable")
+typealias DefaultLock = Never
+#endif
diff --git a/Sources/Testing/Support/Locked.swift b/Sources/Testing/Support/Locked.swift
@@ -1,7 +1,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2023 Apple Inc. and the Swift project authors
+// Copyright (c) 2023–2025 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -10,6 +10,37 @@
 
 internal import _TestingInternals
 
+/// A protocol defining a type, generally platform-specific, that satisfies the
+/// requirements of a lock or mutex.
+protocol Lockable {
+  /// Initialize the lock at the given address.
+  ///
+  /// - Parameters:
+  ///   - lock: A pointer to uninitialized memory that should be initialized as
+  ///     an instance of this type.
+  static func initializeLock(at lock: UnsafeMutablePointer<Self>)
+
+  /// Deinitialize the lock at the given address.
+  ///
+  /// - Parameters:
+  ///   - lock: A pointer to initialized memory that should be deinitialized.
+  static func deinitializeLock(at lock: UnsafeMutablePointer<Self>)
+
+  /// Acquire the lock at the given address.
+  ///
+  /// - Parameters:
+  ///   - lock: The address of the lock to acquire.
+  static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>)
+
+  /// Relinquish the lock at the given address.
+  ///
+  /// - Parameters:
+  ///   - lock: The address of the lock to relinquish.
+  static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>)
+}
+
+// MARK: -
+
 /// A type that wraps a value requiring access from a synchronous caller during
 /// concurrent execution.
 ///
@@ -21,67 +52,23 @@ internal import _TestingInternals
 /// concurrency tools.
 ///
 /// This type is not part of the public interface of the testing library.
-///
-/// - Bug: The state protected by this type should instead be protected using
-///     actor isolation, but actor-isolated functions cannot be called from
-///     synchronous functions. ([83888717](rdar://83888717))
-struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
-  /// The platform-specific type to use for locking.
-  ///
-  /// It would be preferable to implement this lock in Swift, however there is
-  /// no standard lock or mutex type available across all platforms that is
-  /// visible in Swift. C11 has a standard `mtx_t` type, but it is not widely
-  /// supported and so cannot be relied upon.
-  ///
-  /// To keep the implementation of this type as simple as possible,
-  /// `pthread_mutex_t` is used on Apple platforms instead of `os_unfair_lock`
-  /// or `OSAllocatedUnfairLock`.
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
-  typealias PlatformLock = pthread_mutex_t
-#elseif os(FreeBSD) || os(OpenBSD)
-  typealias PlatformLock = pthread_mutex_t?
-#elseif os(Windows)
-  typealias PlatformLock = SRWLOCK
-#elseif os(WASI)
-  // No locks on WASI without multithreaded runtime.
-  typealias PlatformLock = Never
-#else
-#warning("Platform-specific implementation missing: locking unavailable")
-  typealias PlatformLock = Never
-#endif
-
+struct LockedWith<L, T>: RawRepresentable where L: Lockable {
   /// A type providing heap-allocated storage for an instance of ``Locked``.
-  private final class _Storage: ManagedBuffer<T, PlatformLock> {
+  private final class _Storage: ManagedBuffer<T, L> {
     deinit {
       withUnsafeMutablePointerToElements { lock in
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
-        _ = pthread_mutex_destroy(lock)
-#elseif os(Windows)
-        // No deinitialization needed.
-#elseif os(WASI)
-        // No locks on WASI without multithreaded runtime.
-#else
-#warning("Platform-specific implementation missing: locking unavailable")
-#endif
+        L.deinitializeLock(at: lock)
       }
     }
   }
 
   /// Storage for the underlying lock and wrapped value.
-  private nonisolated(unsafe) var _storage: ManagedBuffer<T, PlatformLock>
+  private nonisolated(unsafe) var _storage: ManagedBuffer<T, L>
 
   init(rawValue: T) {
     _storage = _Storage.create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
     _storage.withUnsafeMutablePointerToElements { lock in
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
-      _ = pthread_mutex_init(lock, nil)
-#elseif os(Windows)
-      InitializeSRWLock(lock)
-#elseif os(WASI)
-      // No locks on WASI without multithreaded runtime.
-#else
-#warning("Platform-specific implementation missing: locking unavailable")
-#endif
+      L.initializeLock(at: lock)
     }
   }
 
@@ -103,28 +90,16 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   /// concurrency tools.
   nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R {
     try _storage.withUnsafeMutablePointers { rawValue, lock in
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
-      _ = pthread_mutex_lock(lock)
+      L.unsafelyAcquireLock(at: lock)
       defer {
-        _ = pthread_mutex_unlock(lock)
+        L.unsafelyRelinquishLock(at: lock)
       }
-#elseif os(Windows)
-      AcquireSRWLockExclusive(lock)
-      defer {
-        ReleaseSRWLockExclusive(lock)
-      }
-#elseif os(WASI)
-      // No locks on WASI without multithreaded runtime.
-#else
-#warning("Platform-specific implementation missing: locking unavailable")
-#endif
-
       return try body(&rawValue.pointee)
     }
   }
 
   /// Acquire the lock and invoke a function while it is held, yielding both the
-  /// protected value and a reference to the lock itself.
+  /// protected value and a reference to the underlying lock guarding it.
   ///
   /// - Parameters:
   ///   - body: A closure to invoke while the lock is held.
@@ -134,16 +109,16 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   /// - Throws: Whatever is thrown by `body`.
   ///
   /// This function is equivalent to ``withLock(_:)`` except that the closure
-  /// passed to it also takes a reference to the underlying platform lock. This
-  /// function can be used when platform-specific functionality such as a
-  /// `pthread_cond_t` is needed. Because the caller has direct access to the
-  /// lock and is able to unlock and re-lock it, it is unsafe to modify the
-  /// protected value.
+  /// passed to it also takes a reference to the underlying lock guarding this
+  /// instance's wrapped value. This function can be used when platform-specific
+  /// functionality such as a `pthread_cond_t` is needed. Because the caller has
+  /// direct access to the lock and is able to unlock and re-lock it, it is
+  /// unsafe to modify the protected value.
   ///
   /// - Warning: Callers that unlock the lock _must_ lock it again before the
   ///   closure returns. If the lock is not acquired when `body` returns, the
   ///   effect is undefined.
-  nonmutating func withUnsafePlatformLock<R>(_ body: (UnsafeMutablePointer<PlatformLock>, T) throws -> R) rethrows -> R {
+  nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<L>, T) throws -> R) rethrows -> R {
     try withLock { value in
       try _storage.withUnsafeMutablePointerToElements { lock in
         try body(lock, value)
@@ -152,7 +127,16 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   }
 }
 
-extension Locked where T: AdditiveArithmetic {
+extension LockedWith: Sendable where T: Sendable {}
+
+/// A type that wraps a value requiring access from a synchronous caller during
+/// concurrent execution and which uses the default platform-specific lock type
+/// for the current platform.
+typealias Locked<T> = LockedWith<DefaultLock, T>
+
+// MARK: - Additions
+
+extension LockedWith where T: AdditiveArithmetic {
   /// Add something to the current wrapped value of this instance.
   ///
   /// - Parameters:
@@ -168,7 +152,7 @@ extension Locked where T: AdditiveArithmetic {
   }
 }
 
-extension Locked where T: Numeric {
+extension LockedWith where T: Numeric {
   /// Increment the current wrapped value of this instance.
   ///
   /// - Returns: The sum of ``rawValue`` and `1`.
@@ -188,7 +172,7 @@ extension Locked where T: Numeric {
   }
 }
 
-extension Locked {
+extension LockedWith {
   /// Initialize an instance of this type with a raw value of `nil`.
   init<V>() where T == V? {
     self.init(rawValue: nil)
@@ -198,4 +182,9 @@ extension Locked {
   init<K, V>() where T == Dictionary<K, V> {
     self.init(rawValue: [:])
   }
+
+  /// Initialize an instance of this type with a raw value of `[]`.
+  init<V>() where T == [V] {
+    self.init(rawValue: [])
+  }
 }
diff --git a/Sources/_TestingInternals/include/Includes.h b/Sources/_TestingInternals/include/Includes.h
@@ -131,6 +131,10 @@
 #if !SWT_NO_DYNAMIC_LINKING
 #include <mach-o/dyld.h>
 #endif
+
+#if !SWT_NO_OS_UNFAIR_LOCK
+#include <os/lock.h>
+#endif
 #endif
 
 #if defined(__FreeBSD__)
diff --git a/Tests/TestingTests/Support/LockTests.swift b/Tests/TestingTests/Support/LockTests.swift
diff --git a/Tests/TestingTests/Traits/ParallelizationTraitTests.swift b/Tests/TestingTests/Traits/ParallelizationTraitTests.swift
