Skip to content

Commit e07ef28

Browse files
grynspangrynspan
authored
Only block a single thread on Linux (and none on Windows) when waiting for exit tests. (#357)
This PR refines the implementation of exit tests on Linux so that multiple exit tests running concurrently only need to block a single thread rather than one thread per child process. This PR also refines the implementation of exit tests on Windows to use `RegisterWaitForSingleObject()` for asynchronous notification of process termination rather than requiring a thread per child process. ### 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.
1 parent e486787 commit e07ef28

File tree

3 files changed

+213
-105
lines changed

3 files changed

+213
-105
lines changed

‎Sources/Testing/ExitTests/WaitFor.swift

Lines changed: 163 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,123 @@
1212
internal import TestingInternals
1313

1414
#if SWT_TARGET_OS_APPLE || os(Linux)
15-
/// Wait for a given PID to exit and report its status.
16-
///
17-
/// - Parameters:
18-
/// - pid: The PID to wait for.
19-
///
20-
/// - Returns: The exit condition of `pid`.
21-
///
22-
/// - Throws: Any error encountered calling `waitpid()` except for `EINTR`,
23-
/// which is ignored.
15+
extension ExitCondition {
16+
/// Initialize an instance of this type from an instance of the POSIX
17+
/// `siginfo_t` type.
18+
///
19+
/// - Parameters:
20+
/// - siginfo: The instance of `siginfo_t` to initialize from.
21+
///
22+
/// - Throws: If `siginfo.si_code` does not equal either `CLD_EXITED`,
23+
/// `CLD_KILLED`, or `CLD_DUMPED` (i.e. it does not represent an exit
24+
/// condition.)
25+
fileprivate init(_ siginfo: siginfo_t) throws {
26+
switch siginfo.si_code {
27+
case .init(CLD_EXITED):
28+
self = .exitCode(siginfo.si_status)
29+
case .init(CLD_KILLED), .init(CLD_DUMPED):
30+
self = .signal(siginfo.si_status)
31+
default:
32+
throw SystemError(description: "Unexpected siginfo_t value. Please file a bug report at https://github.com/apple/swift-testing/issues/new and include this information: \(String(reflecting: siginfo))")
33+
}
34+
}
35+
}
36+
37+
#if !(SWT_TARGET_OS_APPLE && !SWT_NO_LIBDISPATCH)
38+
/// A mapping of awaited child PIDs to their corresponding Swift continuations.
39+
private let _childProcessContinuations = Locked<[pid_t: CheckedContinuation<ExitCondition, any Error>]>()
40+
41+
/// A condition variable used to suspend the waiter thread created by
42+
/// `_createWaitThread()` when there are no child processes to await.
43+
private let _waitThreadNoChildrenCondition = {
44+
let result = UnsafeMutablePointer<pthread_cond_t>.allocate(capacity: 1)
45+
_ = pthread_cond_init(result, nil)
46+
return UncheckedSendable(rawValue: result)
47+
}()
48+
49+
#if os(Linux)
50+
/// Set the name of the current thread.
2451
///
25-
/// This function blocks the calling thread on `waitpid()`. External callers
26-
/// should use ``wait(for:)`` instead to avoid deadlocks.
27-
private func _blockAndWait(for pid: pid_t) throws -> ExitCondition {
28-
while true {
29-
var status: CInt = 0
30-
if waitpid(pid, &status, 0) >= 0 {
31-
if swt_WIFSIGNALED(status) {
32-
return .signal(swt_WTERMSIG(status))
33-
} else if swt_WIFEXITED(status) {
34-
return .exitCode(swt_WEXITSTATUS(status))
35-
} else {
36-
// Unreachable: neither signalled nor exited, but waitpid()
37-
// and libdispatch indicate that the process has died.
38-
throw SystemError(description: "Unexpected waitpid() result \(status). Please file a bug report at https://github.com/apple/swift-testing/issues/new")
52+
/// This function declaration is provided because `pthread_setname_np()` is
53+
/// only declared if `_GNU_SOURCE` is set, but setting it causes build errors
54+
/// due to conflicts with Swift's Glibc module.
55+
@_extern(c) func pthread_setname_np(_: pthread_t, _: UnsafePointer<CChar>) -> CInt
56+
#endif
57+
58+
/// The implementation of `_createWaitThread()`, run only once.
59+
private let _createWaitThreadImpl: Void = {
60+
// The body of the thread's run loop.
61+
func waitForAnyChild() {
62+
// Listen for child process exit events. WNOWAIT means we don't perturb the
63+
// state of a terminated (zombie) child process, allowing us to fetch the
64+
// continuation (if available) before reaping.
65+
var siginfo = siginfo_t()
66+
if 0 == waitid(P_ALL, 0, &siginfo, WEXITED | WNOWAIT) {
67+
if case let pid = siginfo.si_pid, pid != 0 {
68+
let continuation = _childProcessContinuations.withLock { childProcessContinuations in
69+
childProcessContinuations.removeValue(forKey: pid)
70+
}
71+
72+
// If we had a continuation for this PID, allow the process to be reaped
73+
// and pass the resulting exit condition back to the calling task. If
74+
// there is no continuation, then either it hasn't been stored yet or
75+
// this child process is not tracked by the waiter thread.
76+
if let continuation, 0 == waitid(P_PID, id_t(pid), &siginfo, WEXITED) {
77+
let result = Result {
78+
try ExitCondition(siginfo)
79+
}
80+
continuation.resume(with: result)
81+
}
82+
}
83+
} else if case let errorCode = swt_errno(), errorCode == ECHILD {
84+
// We got ECHILD. If there are no continuations added right now, we should
85+
// suspend this thread on the no-children condition until it's awoken by a
86+
// newly-scheduled waiter process. (If this condition is spuriously
87+
// woken, we'll just loop again, which is fine.) Note that we read errno
88+
// outside the lock in case acquiring the lock perturbs it.
89+
_childProcessContinuations.withUnsafeUnderlyingLock { lock, childProcessContinuations in
90+
if childProcessContinuations.isEmpty {
91+
_ = pthread_cond_wait(_waitThreadNoChildrenCondition.rawValue, lock)
92+
}
3993
}
40-
} else if swt_errno() != EINTR {
41-
throw CError(rawValue: swt_errno())
4294
}
4395
}
96+
97+
// Create the thread. We immediately detach it upon success to allow the
98+
// system to reclaim its resources when done.
99+
#if SWT_TARGET_OS_APPLE
100+
var thread: pthread_t?
101+
#else
102+
var thread = pthread_t()
103+
#endif
104+
_ = pthread_create(
105+
&thread,
106+
nil,
107+
{ _ in
108+
// Set the thread name to help with diagnostics.
109+
let threadName = "swift-testing exit test monitor"
110+
#if SWT_TARGET_OS_APPLE
111+
_ = pthread_setname_np(threadName)
112+
#else
113+
_ = pthread_setname_np(pthread_self(), threadName)
114+
#endif
115+
116+
// Run an infinite loop that waits for child processes to terminate and
117+
// captures their exit statuses.
118+
while true {
119+
waitForAnyChild()
120+
}
121+
},
122+
nil
123+
)
124+
}()
125+
126+
/// Create a waiter thread that is responsible for waiting for child processes
127+
/// to exit.
128+
private func _createWaitThread() {
129+
_createWaitThreadImpl
44130
}
131+
#endif
45132

46133
/// Wait for a given PID to exit and report its status.
47134
///
@@ -53,7 +140,7 @@ private func _blockAndWait(for pid: pid_t) throws -> ExitCondition {
53140
/// - Throws: Any error encountered calling `waitpid()` except for `EINTR`,
54141
/// which is ignored.
55142
func wait(for pid: pid_t) async throws -> ExitCondition {
56-
#if SWT_TARGET_OS_APPLE
143+
#if SWT_TARGET_OS_APPLE && !SWT_NO_LIBDISPATCH
57144
let source = DispatchSource.makeProcessSource(identifier: pid, eventMask: .exit)
58145
defer {
59146
source.cancel()
@@ -65,37 +152,32 @@ func wait(for pid: pid_t) async throws -> ExitCondition {
65152
source.resume()
66153
}
67154
withExtendedLifetime(source) {}
68-
return try _blockAndWait(for: pid)
155+
156+
// Get the exit status of the process or throw an error (other than EINTR.)
157+
while true {
158+
var siginfo = siginfo_t()
159+
if 0 == waitid(P_PID, id_t(pid), &siginfo, WEXITED) {
160+
return try ExitCondition(siginfo)
161+
} else if case let errorCode = swt_errno(), errorCode != EINTR {
162+
throw CError(rawValue: errorCode)
163+
}
164+
}
69165
#else
70-
// On Linux, spin up a background thread and waitpid() there.
166+
// Ensure the waiter thread is running.
167+
_createWaitThread()
168+
71169
return try await withCheckedThrowingContinuation { continuation in
72-
// Create a structure to hold the state needed by the thread, and box it
73-
// as a refcounted pointer that we can pass to libpthread.
74-
struct Context {
75-
var pid: pid_t
76-
var continuation: CheckedContinuation<ExitCondition, any Error>
77-
}
78-
let context = Unmanaged.passRetained(
79-
Context(pid: pid, continuation: continuation) as AnyObject
80-
).toOpaque()
81-
82-
// The body of the thread: unwrap and take ownership of the context we
83-
// created above, then call waitpid() and report the result/error.
84-
let body: @convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer? = { contextp in
85-
let context = Unmanaged<AnyObject>.fromOpaque(contextp!).takeRetainedValue() as! Context
86-
let result = Result { try _blockAndWait(for: context.pid) }
87-
context.continuation.resume(with: result)
88-
return nil
89-
}
170+
_childProcessContinuations.withLock { childProcessContinuations in
171+
// We don't need to worry about a race condition here because waitid()
172+
// does not clear the wait/zombie state of the child process. If it sees
173+
// the child process has terminated and manages to acquire the lock before
174+
// we add this continuation to the dictionary, then it will simply loop
175+
// and report the status again.
176+
let oldContinuation = childProcessContinuations.updateValue(continuation, forKey: pid)
177+
assert(oldContinuation == nil, "Unexpected continuation found for PID \(pid). Please file a bug report at https://github.com/apple/swift-testing/issues/new")
90178

91-
// Create the thread. We immediately detach it upon success to allow the
92-
// system to reclaim its resources when done.
93-
var thread = pthread_t()
94-
switch pthread_create(&thread, nil, body, context) {
95-
case 0:
96-
_ = pthread_detach(thread)
97-
case let errorCode:
98-
continuation.resume(throwing: CError(rawValue: errorCode))
179+
// Wake up the waiter thread if it is waiting for more child processes.
180+
_ = pthread_cond_signal(_waitThreadNoChildrenCondition.rawValue)
99181
}
100182
}
101183
#endif
@@ -110,13 +192,34 @@ func wait(for pid: pid_t) async throws -> ExitCondition {
110192
///
111193
/// - Throws: Any error encountered calling `WaitForSingleObject()` or
112194
/// `GetExitCodeProcess()`.
113-
///
114-
/// This function blocks the calling thread on `WaitForSingleObject()`. External
115-
/// callers should use ``wait(for:)`` instead to avoid deadlocks.
116-
private func _blockAndWait(for processHandle: HANDLE) throws -> ExitCondition {
117-
if WAIT_FAILED == WaitForSingleObject(processHandle, INFINITE) {
118-
throw Win32Error(rawValue: GetLastError())
195+
func wait(for processHandle: HANDLE) async throws -> ExitCondition {
196+
// Once the continuation resumes, it will need to unregister the wait, so
197+
// yield the wait handle back to the calling scope.
198+
var waitHandle: HANDLE?
199+
defer {
200+
if let waitHandle {
201+
_ = UnregisterWait(waitHandle)
202+
}
203+
}
204+
205+
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, any Error>) in
206+
// Set up a callback that immediately resumes the continuation and does no
207+
// other work.
208+
let context = Unmanaged.passRetained(continuation as AnyObject).toOpaque()
209+
let callback: WAITORTIMERCALLBACK = { context, _ in
210+
let continuation = Unmanaged<AnyObject>.fromOpaque(context!).takeRetainedValue() as! CheckedContinuation<Void, any Error>
211+
continuation.resume()
212+
}
213+
214+
// We only want the callback to fire once (and not be rescheduled.) Waiting
215+
// may take an arbitrarily long time, so let the thread pool know that too.
216+
let flags = ULONG(WT_EXECUTEONLYONCE | WT_EXECUTELONGFUNCTION)
217+
guard RegisterWaitForSingleObject(&waitHandle, processHandle, callback, context, INFINITE, flags) else {
218+
continuation.resume(throwing: Win32Error(rawValue: GetLastError()))
219+
return
220+
}
119221
}
222+
120223
var status: DWORD = 0
121224
guard GetExitCodeProcess(processHandle, &status) else {
122225
// The child process terminated but we couldn't get its status back.
@@ -127,35 +230,5 @@ private func _blockAndWait(for processHandle: HANDLE) throws -> ExitCondition {
127230
// FIXME: handle SEH/VEH uncaught exceptions.
128231
return .exitCode(CInt(bitPattern: status))
129232
}
130-
131-
/// Wait for a given process handle to exit and report its status.
132-
///
133-
/// - Parameters:
134-
/// - processHandle: The handle to wait for.
135-
///
136-
/// - Returns: The exit condition of `processHandle`.
137-
///
138-
/// - Throws: Any error encountered calling `WaitForSingleObject()` or
139-
/// `GetExitCodeProcess()`.
140-
func wait(for processHandle: HANDLE) async throws -> ExitCondition {
141-
try await withCheckedThrowingContinuation { continuation in
142-
// Create a structure to hold the state needed by the thread, and box it
143-
// as a refcounted pointer that we can pass to libpthread.
144-
struct Context {
145-
var processHandle: HANDLE
146-
var continuation: CheckedContinuation<ExitCondition, any Error>
147-
}
148-
let context = Unmanaged.passRetained(
149-
Context(processHandle: processHandle, continuation: continuation) as AnyObject
150-
).toOpaque()
151-
152-
let body: _beginthread_proc_type = { contextp in
153-
let context = Unmanaged<AnyObject>.fromOpaque(contextp!).takeRetainedValue() as! Context
154-
let result = Result { try _blockAndWait(for: context.processHandle) }
155-
context.continuation.resume(with: result)
156-
}
157-
_ = _beginthread(body, 0, context)
158-
}
159-
}
160233
#endif
161234
#endif

‎Sources/Testing/Support/Locked.swift

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
99
//
1010

11-
private import TestingInternals
11+
internal import TestingInternals
1212

1313
/// A type that wraps a value requiring access from a synchronous caller during
1414
/// concurrent execution.
@@ -120,6 +120,36 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
120120
return try body(&rawValue.pointee)
121121
}
122122
}
123+
124+
#if SWT_TARGET_OS_APPLE || os(Linux) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
125+
/// Acquire the lock and invoke a function while it is held, yielding both the
126+
/// protected value and a reference to the lock itself.
127+
///
128+
/// - Parameters:
129+
/// - body: A closure to invoke while the lock is held.
130+
///
131+
/// - Returns: Whatever is returned by `body`.
132+
///
133+
/// - Throws: Whatever is thrown by `body`.
134+
///
135+
/// This function is equivalent to ``withLock(_:)`` except that the closure
136+
/// passed to it also takes a reference to the underlying platform lock. This
137+
/// function can be used when platform-specific functionality such as a
138+
/// `pthread_cond_t` is needed. Because the caller has direct access to the
139+
/// lock and is able to unlock and re-lock it, it is unsafe to modify the
140+
/// protected value.
141+
///
142+
/// - Warning: Callers that unlock the lock _must_ lock it again before the
143+
/// closure returns. If the lock is not acquired when `body` returns, the
144+
/// effect is undefined.
145+
nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<pthread_mutex_t>, T) throws -> R) rethrows -> R {
146+
try withLock { value in
147+
try _storage.rawValue.withUnsafeMutablePointerToElements { lock in
148+
try body(lock, value)
149+
}
150+
}
151+
}
152+
#endif
123153
}
124154

125155
extension Locked where T: AdditiveArithmetic {

‎Sources/TestingInternals/include/Stubs.h

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,28 @@ static char *_Nullable *_Null_unspecified swt_environ(void) {
9090
}
9191
#endif
9292

93-
#if __has_include(<sys/wait.h>)
94-
static bool swt_WIFSIGNALED(int exitCode) {
95-
return WIFSIGNALED(exitCode);
96-
}
97-
98-
static int swt_WTERMSIG(int exitCode) {
99-
return WTERMSIG(exitCode);
100-
}
101-
102-
static bool swt_WIFEXITED(int exitCode) {
103-
return WIFEXITED(exitCode);
93+
#if __has_include(<signal.h>) && defined(si_pid)
94+
/// Get the value of the `si_pid` field of a `siginfo_t` structure.
95+
///
96+
/// This function is provided because `si_pid` is a complex macro on some
97+
/// platforms and cannot be imported directly into Swift. It is renamed back to
98+
/// `siginfo_t.si_pid` in Swift.
99+
SWT_SWIFT_NAME(getter:siginfo_t.si_pid(self:))
100+
static pid_t swt_siginfo_t_si_pid(const siginfo_t *siginfo) {
101+
return siginfo->si_pid;
104102
}
103+
#endif
105104

106-
static int swt_WEXITSTATUS(int exitCode) {
107-
return WEXITSTATUS(exitCode);
105+
#if __has_include(<signal.h>) && defined(si_status)
106+
/// Get the value of the `si_status` field of a `siginfo_t` structure.
107+
///
108+
/// This function is provided because `si_status` is a complex macro on some
109+
/// platforms and cannot be imported directly into Swift. It is renamed back to
110+
/// `siginfo_t.si_status` in Swift.
111+
SWT_SWIFT_NAME(getter:siginfo_t.si_status(self:))
112+
static pid_t swt_siginfo_t_si_status(const siginfo_t *siginfo) {
113+
return siginfo->si_status;
108114
}
109-
110115
#endif
111116

112117
SWT_ASSUME_NONNULL_END

0 commit comments

Comments
 (0)