Skip to content

Adopt Mutex. #1351

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.

Sign up for GitHub

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 4 commits into from
Oct 4, 2025
Merged

Adopt Mutex. #1351

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39e2c37
Adopt `Mutex`.
grynspan Oct 3, 2025
7b07669
Fix typo
grynspan Oct 3, 2025
2ab69cf
Merge branch 'main' into jgrynspan/538-mutex
grynspan Oct 4, 2025
5e9c8bc
Add a test
grynspan Oct 4, 2025
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
1 change: 0 additions & 1 deletion Sources/Testing/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ add_library(Testing
Support/Graph.swift
Support/JSON.swift
Support/Locked.swift
Support/Locked+Platform.swift
Support/VersionNumber.swift
Support/Versions.swift
Discovery+Macro.swift
Expand Down
43 changes: 39 additions & 4 deletions Sources/Testing/ExitTests/WaitFor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,42 @@ func wait(for pid: consuming pid_t) async throws -> ExitStatus {
}
#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 = LockedWith<pthread_mutex_t, [pid_t: CheckedContinuation<ExitStatus, any Error>]>()
private nonisolated(unsafe) let _childProcessContinuations = {
let result = ManagedBuffer<[pid_t: CheckedContinuation<ExitStatus, any Error>], pthread_mutex_t>.create(
minimumCapacity: 1,
makingHeaderWith: { _ in [:] }
)

result.withUnsafeMutablePointers { _, lock in
_ = pthread_mutex_init(lock, nil)
}

return result
}()

/// Access the value in `_childProcessContinuations` while guarded by its lock.
///
/// - Parameters:
/// - body: A closure to invoke while the lock is held.
///
/// - Returns: Whatever is returned by `body`.
///
/// - Throws: Whatever is thrown by `body`.
private func _withLockedChildProcessContinuations<R>(
_ body: (
_ childProcessContinuations: inout [pid_t: CheckedContinuation<ExitStatus, any Error>],
_ lock: UnsafeMutablePointer<pthread_mutex_t>
) throws -> R
) rethrows -> R {
try _childProcessContinuations.withUnsafeMutablePointers { childProcessContinuations, lock in
_ = pthread_mutex_lock(lock)
defer {
_ = pthread_mutex_unlock(lock)
}

return try body(&childProcessContinuations.pointee, lock)
}
}

/// A condition variable used to suspend the waiter thread created by
/// `_createWaitThread()` when there are no child processes to await.
Expand Down Expand Up @@ -116,7 +151,7 @@ private let _createWaitThread: Void = {
var siginfo = siginfo_t()
if 0 == waitid(P_ALL, 0, &siginfo, WEXITED | WNOWAIT) {
if case let pid = siginfo.si_pid, pid != 0 {
let continuation = _childProcessContinuations.withLock { childProcessContinuations in
let continuation = _withLockedChildProcessContinuations { childProcessContinuations, _ in
childProcessContinuations.removeValue(forKey: pid)
}

Expand All @@ -137,7 +172,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.withUnsafeUnderlyingLock { lock, childProcessContinuations in
_withLockedChildProcessContinuations { childProcessContinuations, lock in
if childProcessContinuations.isEmpty {
_ = pthread_cond_wait(_waitThreadNoChildrenCondition, lock)
}
Expand Down Expand Up @@ -209,7 +244,7 @@ func wait(for pid: consuming pid_t) async throws -> ExitStatus {
_createWaitThread

return try await withCheckedThrowingContinuation { continuation in
_childProcessContinuations.withLock { childProcessContinuations in
_withLockedChildProcessContinuations { childProcessContinuations, _ in
// We don't need to worry about a race condition here because waitid()
// does not clear the wait/zombie state of the child process. If it sees
// the child process has terminated and manages to acquire the lock before
Expand Down
96 changes: 0 additions & 96 deletions Sources/Testing/Support/Locked+Platform.swift
View file
Open in desktop

This file was deleted.

134 changes: 50 additions & 84 deletions Sources/Testing/Support/Locked.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,8 @@
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
//

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: -
private import _TestingInternals
private import Synchronization

/// A type that wraps a value requiring access from a synchronous caller during
/// concurrent execution.
Expand All @@ -52,30 +22,48 @@ protocol Lockable {
/// concurrency tools.
///
/// This type is not part of the public interface of the testing library.
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, L> {
deinit {
withUnsafeMutablePointerToElements { lock in
L.deinitializeLock(at: lock)
}
struct Locked<T> {
/// A type providing storage for the underlying lock and wrapped value.
#if SWT_TARGET_OS_APPLE && canImport(os)
private typealias _Storage = ManagedBuffer<T, os_unfair_lock_s>
#else
private final class _Storage {
let mutex: Mutex<T>

init(_ rawValue: consuming sending T) {
mutex = Mutex(rawValue)
}
}
#endif

/// Storage for the underlying lock and wrapped value.
private nonisolated(unsafe) var _storage: ManagedBuffer<T, L>
private nonisolated(unsafe) var _storage: _Storage
}

extension Locked: Sendable where T: Sendable {}

extension Locked: RawRepresentable {
init(rawValue: T) {
_storage = _Storage.create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
#if SWT_TARGET_OS_APPLE && canImport(os)
_storage = .create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
_storage.withUnsafeMutablePointerToElements { lock in
L.initializeLock(at: lock)
lock.initialize(to: .init())
}
#else
nonisolated(unsafe) let rawValue = rawValue
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's necessary for us to write nonisolated(unsafe) here and a few other places because T isn't universally Sendable. We have some types (pointers) that we need to guard with locks which are not sendable, and the compiler is concerned there may be data races. We know what we're doing (famous last words) but the compiler can't reason about it.

_storage = _Storage(rawValue)
#endif
}

var rawValue: T {
withLock { $0 }
withLock { rawValue in
nonisolated(unsafe) let rawValue = rawValue
return rawValue
}
}
}

extension Locked {
/// Acquire the lock and invoke a function while it is held.
///
/// - Parameters:
Expand All @@ -88,55 +76,27 @@ struct LockedWith<L, T>: RawRepresentable where L: Lockable {
/// This function can be used to synchronize access to shared data from a
/// synchronous caller. Wherever possible, use actor isolation or other Swift
/// concurrency tools.
nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R where R: ~Copyable {
try _storage.withUnsafeMutablePointers { rawValue, lock in
L.unsafelyAcquireLock(at: lock)
func withLock<R>(_ body: (inout T) throws -> sending R) rethrows -> sending R where R: ~Copyable {
#if SWT_TARGET_OS_APPLE && canImport(os)
nonisolated(unsafe) let result = try _storage.withUnsafeMutablePointers { rawValue, lock in
os_unfair_lock_lock(lock)
defer {
L.unsafelyRelinquishLock(at: lock)
os_unfair_lock_unlock(lock)
}
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 underlying lock guarding it.
///
/// - Parameters:
/// - body: A closure to invoke while the lock is held.
///
/// - Returns: Whatever is returned by `body`.
///
/// - 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 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 withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<L>, T) throws -> R) rethrows -> R where R: ~Copyable {
try withLock { value in
try _storage.withUnsafeMutablePointerToElements { lock in
try body(lock, value)
}
return result
#else
try _storage.mutex.withLock { rawValue in
try body(&rawValue)
}
#endif
}
}

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 {
extension Locked where T: AdditiveArithmetic & Sendable {
/// Add something to the current wrapped value of this instance.
///
/// - Parameters:
Expand All @@ -152,7 +112,7 @@ extension LockedWith where T: AdditiveArithmetic {
}
}

extension LockedWith where T: Numeric {
extension Locked where T: Numeric & Sendable {
/// Increment the current wrapped value of this instance.
///
/// - Returns: The sum of ``rawValue`` and `1`.
Expand All @@ -172,7 +132,7 @@ extension LockedWith where T: Numeric {
}
}

extension LockedWith {
extension Locked {
/// Initialize an instance of this type with a raw value of `nil`.
init<V>() where T == V? {
self.init(rawValue: nil)
Expand All @@ -188,3 +148,9 @@ extension LockedWith {
self.init(rawValue: [])
}
}

// MARK: - POSIX conveniences

#if os(FreeBSD) || os(OpenBSD)
typealias pthread_mutex_t = _TestingInternals.pthread_mutex_t?
#endif
Loading