Skip to content

Commit 56506bf

Browse files
authored
Revert "Adopt Mutex." (#1353)
Reverts #1351, this is failing to build in CI (https://ci.swift.org/job/oss-swift-pr-test-ubuntu-22_04/9662/)
1 parent d783642 commit 56506bf

File tree

5 files changed

+216
-96
lines changed

5 files changed

+216
-96
lines changed

Sources/Testing/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ add_library(Testing
8989
Support/Graph.swift
9090
Support/JSON.swift
9191
Support/Locked.swift
92+
Support/Locked+Platform.swift
9293
Support/VersionNumber.swift
9394
Support/Versions.swift
9495
Discovery+Macro.swift

Sources/Testing/ExitTests/WaitFor.swift

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -80,42 +80,7 @@ func wait(for pid: consuming pid_t) async throws -> ExitStatus {
8080
}
8181
#elseif SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD)
8282
/// A mapping of awaited child PIDs to their corresponding Swift continuations.
83-
private nonisolated(unsafe) let _childProcessContinuations = {
84-
let result = ManagedBuffer<[pid_t: CheckedContinuation<ExitStatus, any Error>], pthread_mutex_t>.create(
85-
minimumCapacity: 1,
86-
makingHeaderWith: { _ in [:] }
87-
)
88-
89-
result.withUnsafeMutablePointers { _, lock in
90-
_ = pthread_mutex_init(lock, nil)
91-
}
92-
93-
return result
94-
}()
95-
96-
/// Access the value in `_childProcessContinuations` while guarded by its lock.
97-
///
98-
/// - Parameters:
99-
/// - body: A closure to invoke while the lock is held.
100-
///
101-
/// - Returns: Whatever is returned by `body`.
102-
///
103-
/// - Throws: Whatever is thrown by `body`.
104-
private func _withLockedChildProcessContinuations<R>(
105-
_ body: (
106-
_ childProcessContinuations: inout [pid_t: CheckedContinuation<ExitStatus, any Error>],
107-
_ lock: UnsafeMutablePointer<pthread_mutex_t>
108-
) throws -> R
109-
) rethrows -> R {
110-
try _childProcessContinuations.withUnsafeMutablePointers { childProcessContinuations, lock in
111-
_ = pthread_mutex_lock(lock)
112-
defer {
113-
_ = pthread_mutex_unlock(lock)
114-
}
115-
116-
return try body(&childProcessContinuations.pointee, lock)
117-
}
118-
}
83+
private let _childProcessContinuations = LockedWith<pthread_mutex_t, [pid_t: CheckedContinuation<ExitStatus, any Error>]>()
11984

12085
/// A condition variable used to suspend the waiter thread created by
12186
/// `_createWaitThread()` when there are no child processes to await.
@@ -147,7 +112,7 @@ private let _createWaitThread: Void = {
147112
var siginfo = siginfo_t()
148113
if 0 == waitid(P_ALL, 0, &siginfo, WEXITED | WNOWAIT) {
149114
if case let pid = siginfo.si_pid, pid != 0 {
150-
let continuation = _withLockedChildProcessContinuations { childProcessContinuations, _ in
115+
let continuation = _childProcessContinuations.withLock { childProcessContinuations in
151116
childProcessContinuations.removeValue(forKey: pid)
152117
}
153118

@@ -168,7 +133,7 @@ private let _createWaitThread: Void = {
168133
// newly-scheduled waiter process. (If this condition is spuriously
169134
// woken, we'll just loop again, which is fine.) Note that we read errno
170135
// outside the lock in case acquiring the lock perturbs it.
171-
_withLockedChildProcessContinuations { childProcessContinuations, lock in
136+
_childProcessContinuations.withUnsafeUnderlyingLock { lock, childProcessContinuations in
172137
if childProcessContinuations.isEmpty {
173138
_ = pthread_cond_wait(_waitThreadNoChildrenCondition, lock)
174139
}
@@ -240,7 +205,7 @@ func wait(for pid: consuming pid_t) async throws -> ExitStatus {
240205
_createWaitThread
241206

242207
return try await withCheckedThrowingContinuation { continuation in
243-
_withLockedChildProcessContinuations { childProcessContinuations, _ in
208+
_childProcessContinuations.withLock { childProcessContinuations in
244209
// We don't need to worry about a race condition here because waitid()
245210
// does not clear the wait/zombie state of the child process. If it sees
246211
// the child process has terminated and manages to acquire the lock before
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
//
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2023–2025 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See https://swift.org/LICENSE.txt for license information
8+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
//
10+
11+
internal import _TestingInternals
12+
13+
extension Never: Lockable {
14+
static func initializeLock(at lock: UnsafeMutablePointer<Self>) {}
15+
static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {}
16+
static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {}
17+
static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {}
18+
}
19+
20+
#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK
21+
extension os_unfair_lock_s: Lockable {
22+
static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
23+
lock.initialize(to: .init())
24+
}
25+
26+
static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
27+
// No deinitialization needed.
28+
}
29+
30+
static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
31+
os_unfair_lock_lock(lock)
32+
}
33+
34+
static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
35+
os_unfair_lock_unlock(lock)
36+
}
37+
}
38+
#endif
39+
40+
#if os(FreeBSD) || os(OpenBSD)
41+
typealias pthread_mutex_t = _TestingInternals.pthread_mutex_t?
42+
typealias pthread_cond_t = _TestingInternals.pthread_cond_t?
43+
#endif
44+
45+
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android) || (os(WASI) && _runtime(_multithreaded))
46+
extension pthread_mutex_t: Lockable {
47+
static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
48+
_ = pthread_mutex_init(lock, nil)
49+
}
50+
51+
static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
52+
_ = pthread_mutex_destroy(lock)
53+
}
54+
55+
static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
56+
_ = pthread_mutex_lock(lock)
57+
}
58+
59+
static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
60+
_ = pthread_mutex_unlock(lock)
61+
}
62+
}
63+
#endif
64+
65+
#if os(Windows)
66+
extension SRWLOCK: Lockable {
67+
static func initializeLock(at lock: UnsafeMutablePointer<Self>) {
68+
InitializeSRWLock(lock)
69+
}
70+
71+
static func deinitializeLock(at lock: UnsafeMutablePointer<Self>) {
72+
// No deinitialization needed.
73+
}
74+
75+
static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>) {
76+
AcquireSRWLockExclusive(lock)
77+
}
78+
79+
static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>) {
80+
ReleaseSRWLockExclusive(lock)
81+
}
82+
}
83+
#endif
84+
85+
#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK
86+
typealias DefaultLock = os_unfair_lock
87+
#elseif SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android) || (os(WASI) && _runtime(_multithreaded))
88+
typealias DefaultLock = pthread_mutex_t
89+
#elseif os(Windows)
90+
typealias DefaultLock = SRWLOCK
91+
#elseif os(WASI)
92+
// No locks on WASI without multithreaded runtime.
93+
typealias DefaultLock = Never
94+
#else
95+
#warning("Platform-specific implementation missing: locking unavailable")
96+
typealias DefaultLock = Never
97+
#endif

Sources/Testing/Support/Locked.swift

Lines changed: 83 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,37 @@
99
//
1010

1111
internal import _TestingInternals
12-
private import Synchronization
12+
13+
/// A protocol defining a type, generally platform-specific, that satisfies the
14+
/// requirements of a lock or mutex.
15+
protocol Lockable {
16+
/// Initialize the lock at the given address.
17+
///
18+
/// - Parameters:
19+
/// - lock: A pointer to uninitialized memory that should be initialized as
20+
/// an instance of this type.
21+
static func initializeLock(at lock: UnsafeMutablePointer<Self>)
22+
23+
/// Deinitialize the lock at the given address.
24+
///
25+
/// - Parameters:
26+
/// - lock: A pointer to initialized memory that should be deinitialized.
27+
static func deinitializeLock(at lock: UnsafeMutablePointer<Self>)
28+
29+
/// Acquire the lock at the given address.
30+
///
31+
/// - Parameters:
32+
/// - lock: The address of the lock to acquire.
33+
static func unsafelyAcquireLock(at lock: UnsafeMutablePointer<Self>)
34+
35+
/// Relinquish the lock at the given address.
36+
///
37+
/// - Parameters:
38+
/// - lock: The address of the lock to relinquish.
39+
static func unsafelyRelinquishLock(at lock: UnsafeMutablePointer<Self>)
40+
}
41+
42+
// MARK: -
1343

1444
/// A type that wraps a value requiring access from a synchronous caller during
1545
/// concurrent execution.
@@ -22,48 +52,30 @@ private import Synchronization
2252
/// concurrency tools.
2353
///
2454
/// This type is not part of the public interface of the testing library.
25-
struct Locked<T> {
26-
/// A type providing storage for the underlying lock and wrapped value.
27-
#if SWT_TARGET_OS_APPLE && canImport(os)
28-
private typealias _Storage = ManagedBuffer<T, os_unfair_lock_s>
29-
#else
30-
private final class _Storage {
31-
let mutex: Mutex<T>
32-
33-
init(_ rawValue: consuming sending T) {
34-
mutex = Mutex(rawValue)
55+
struct LockedWith<L, T>: RawRepresentable where L: Lockable {
56+
/// A type providing heap-allocated storage for an instance of ``Locked``.
57+
private final class _Storage: ManagedBuffer<T, L> {
58+
deinit {
59+
withUnsafeMutablePointerToElements { lock in
60+
L.deinitializeLock(at: lock)
61+
}
3562
}
3663
}
37-
#endif
3864

3965
/// Storage for the underlying lock and wrapped value.
40-
private nonisolated(unsafe) var _storage: _Storage
41-
}
42-
43-
extension Locked: Sendable where T: Sendable {}
66+
private nonisolated(unsafe) var _storage: ManagedBuffer<T, L>
4467

45-
extension Locked: RawRepresentable {
4668
init(rawValue: T) {
47-
#if SWT_TARGET_OS_APPLE && canImport(os)
48-
_storage = .create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
69+
_storage = _Storage.create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
4970
_storage.withUnsafeMutablePointerToElements { lock in
50-
lock.initialize(to: .init())
71+
L.initializeLock(at: lock)
5172
}
52-
#else
53-
nonisolated(unsafe) let rawValue = rawValue
54-
_storage = _Storage(rawValue)
55-
#endif
5673
}
5774

5875
var rawValue: T {
59-
withLock { rawValue in
60-
nonisolated(unsafe) let rawValue = rawValue
61-
return rawValue
62-
}
76+
withLock { $0 }
6377
}
64-
}
6578

66-
extension Locked {
6779
/// Acquire the lock and invoke a function while it is held.
6880
///
6981
/// - Parameters:
@@ -76,27 +88,55 @@ extension Locked {
7688
/// This function can be used to synchronize access to shared data from a
7789
/// synchronous caller. Wherever possible, use actor isolation or other Swift
7890
/// concurrency tools.
79-
func withLock<R>(_ body: (inout T) throws -> sending R) rethrows -> sending R where R: ~Copyable {
80-
#if SWT_TARGET_OS_APPLE && canImport(os)
81-
nonisolated(unsafe) let result = try _storage.withUnsafeMutablePointers { rawValue, lock in
82-
os_unfair_lock_lock(lock)
91+
nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R where R: ~Copyable {
92+
try _storage.withUnsafeMutablePointers { rawValue, lock in
93+
L.unsafelyAcquireLock(at: lock)
8394
defer {
84-
os_unfair_lock_unlock(lock)
95+
L.unsafelyRelinquishLock(at: lock)
8596
}
8697
return try body(&rawValue.pointee)
8798
}
88-
return result
89-
#else
90-
try _storage.mutex.withLock { rawValue in
91-
try body(&rawValue)
99+
}
100+
101+
/// Acquire the lock and invoke a function while it is held, yielding both the
102+
/// protected value and a reference to the underlying lock guarding it.
103+
///
104+
/// - Parameters:
105+
/// - body: A closure to invoke while the lock is held.
106+
///
107+
/// - Returns: Whatever is returned by `body`.
108+
///
109+
/// - Throws: Whatever is thrown by `body`.
110+
///
111+
/// This function is equivalent to ``withLock(_:)`` except that the closure
112+
/// passed to it also takes a reference to the underlying lock guarding this
113+
/// instance's wrapped value. This function can be used when platform-specific
114+
/// functionality such as a `pthread_cond_t` is needed. Because the caller has
115+
/// direct access to the lock and is able to unlock and re-lock it, it is
116+
/// unsafe to modify the protected value.
117+
///
118+
/// - Warning: Callers that unlock the lock _must_ lock it again before the
119+
/// closure returns. If the lock is not acquired when `body` returns, the
120+
/// effect is undefined.
121+
nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<L>, T) throws -> R) rethrows -> R where R: ~Copyable {
122+
try withLock { value in
123+
try _storage.withUnsafeMutablePointerToElements { lock in
124+
try body(lock, value)
125+
}
92126
}
93-
#endif
94127
}
95128
}
96129

130+
extension LockedWith: Sendable where T: Sendable {}
131+
132+
/// A type that wraps a value requiring access from a synchronous caller during
133+
/// concurrent execution and which uses the default platform-specific lock type
134+
/// for the current platform.
135+
typealias Locked<T> = LockedWith<DefaultLock, T>
136+
97137
// MARK: - Additions
98138

99-
extension Locked where T: AdditiveArithmetic & Sendable {
139+
extension LockedWith where T: AdditiveArithmetic {
100140
/// Add something to the current wrapped value of this instance.
101141
///
102142
/// - Parameters:
@@ -112,7 +152,7 @@ extension Locked where T: AdditiveArithmetic & Sendable {
112152
}
113153
}
114154

115-
extension Locked where T: Numeric & Sendable {
155+
extension LockedWith where T: Numeric {
116156
/// Increment the current wrapped value of this instance.
117157
///
118158
/// - Returns: The sum of ``rawValue`` and `1`.
@@ -132,7 +172,7 @@ extension Locked where T: Numeric & Sendable {
132172
}
133173
}
134174

135-
extension Locked {
175+
extension LockedWith {
136176
/// Initialize an instance of this type with a raw value of `nil`.
137177
init<V>() where T == V? {
138178
self.init(rawValue: nil)
@@ -148,10 +188,3 @@ extension Locked {
148188
self.init(rawValue: [])
149189
}
150190
}
151-
152-
// MARK: - POSIX conveniences
153-
154-
#if os(FreeBSD) || os(OpenBSD)
155-
typealias pthread_mutex_t = _TestingInternals.pthread_mutex_t?
156-
typealias pthread_cond_t = _TestingInternals.pthread_cond_t?
157-
#endif

0 commit comments

Comments
 (0)