Skip to content

Commit 471a3ab

Browse files
iCharlesHuiCharlesHu
committed
Unify tests on all supported platforms to ensure consistent behavior and add more tests.
1 parent 7fb7ee8 commit 471a3ab

20 files changed

+3401
-1726
lines changed

‎Sources/Subprocess/Configuration.swift

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -79,33 +79,39 @@ public struct Configuration: Sendable {
7979

8080
let execution = _spawnResult.execution
8181

82-
let result: Swift.Result<Result, Error>
83-
do {
84-
result = try await .success(withAsyncTaskCleanupHandler {
85-
let inputIO = _spawnResult.inputWriteEnd()
86-
let outputIO = _spawnResult.outputReadEnd()
87-
let errorIO = _spawnResult.errorReadEnd()
82+
return try await withAsyncTaskCleanupHandler {
83+
let inputIO = _spawnResult.inputWriteEnd()
84+
let outputIO = _spawnResult.outputReadEnd()
85+
let errorIO = _spawnResult.errorReadEnd()
8886

87+
let result: Swift.Result<Result, Error>
88+
do {
8989
// Body runs in the same isolation
90-
return try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
91-
} onCleanup: {
92-
// Attempt to terminate the child process
93-
await execution.runTeardownSequence(
94-
self.platformOptions.teardownSequence
95-
)
96-
})
97-
} catch {
98-
result = .failure(error)
99-
}
90+
let bodyResult = try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
91+
result = .success(bodyResult)
92+
} catch {
93+
result = .failure(error)
94+
}
10095

101-
// Ensure that we begin monitoring process termination after `body` runs
102-
// and regardless of whether `body` throws, so that the pid gets reaped
103-
// even if `body` throws, and we are not leaving zombie processes in the
104-
// process table which will cause the process termination monitoring thread
105-
// to effectively hang due to the pid never being awaited
106-
let terminationStatus = try await Subprocess.monitorProcessTermination(forExecution: _spawnResult.execution)
96+
// Ensure that we begin monitoring process termination after `body` runs
97+
// and regardless of whether `body` throws, so that the pid gets reaped
98+
// even if `body` throws, and we are not leaving zombie processes in the
99+
// process table which will cause the process termination monitoring thread
100+
// to effectively hang due to the pid never being awaited
101+
let terminationStatus = try await monitorProcessTermination(
102+
forExecution: _spawnResult.execution
103+
)
107104

108-
return try ExecutionResult(terminationStatus: terminationStatus, value: result.get())
105+
return ExecutionResult(
106+
terminationStatus: terminationStatus,
107+
value: try result.get()
108+
)
109+
} onCleanup: {
110+
// Attempt to terminate the child process
111+
await execution.runTeardownSequence(
112+
self.platformOptions.teardownSequence
113+
)
114+
}
109115
}
110116
}
111117

@@ -329,12 +335,12 @@ public struct Arguments: Sendable, ExpressibleByArrayLiteral, Hashable {
329335
self.executablePathOverride = nil
330336
}
331337
}
338+
#endif
332339

333340
public init(_ array: [[UInt8]]) {
334341
self.storage = array.map { .rawBytes($0) }
335342
self.executablePathOverride = nil
336343
}
337-
#endif
338344
}
339345

340346
extension Arguments: CustomStringConvertible, CustomDebugStringConvertible {
@@ -864,7 +870,7 @@ internal struct CreatedPipe: ~Copyable {
864870
DWORD(readBufferSize),
865871
DWORD(readBufferSize),
866872
0,
867-
&saAttributes
873+
nil
868874
)
869875
}
870876
guard let parentEnd, parentEnd != INVALID_HANDLE_VALUE else {

‎Sources/Subprocess/IO/AsyncIO+Darwin.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ internal import Dispatch
2525
final class AsyncIO: Sendable {
2626
static let shared: AsyncIO = AsyncIO()
2727

28-
private init() {}
28+
internal init() {}
29+
30+
internal func shutdown() { /* noop on Darwin */ }
2931

3032
internal func read(
3133
from diskIO: borrowing IOChannel,

‎Sources/Subprocess/IO/AsyncIO+Linux.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ final class AsyncIO: Sendable {
6767
static let shared: AsyncIO = AsyncIO()
6868

6969
private let state: Result<State, SubprocessError>
70+
private let shutdownFlag: Atomic<UInt8> = Atomic(0)
7071

71-
private init() {
72+
internal init() {
7273
// Create main epoll fd
7374
let epollFileDescriptor = epoll_create1(CInt(EPOLL_CLOEXEC))
7475
guard epollFileDescriptor >= 0 else {
@@ -204,11 +205,15 @@ final class AsyncIO: Sendable {
204205
}
205206
}
206207

207-
private func shutdown() {
208+
internal func shutdown() {
208209
guard case .success(let currentState) = self.state else {
209210
return
210211
}
211212

213+
guard self.shutdownFlag.add(1, ordering: .sequentiallyConsistent).newValue == 1 else {
214+
// We already closed this AsyncIO
215+
return
216+
}
212217
var one: UInt64 = 1
213218
// Wake up the thread for shutdown
214219
_ = _SubprocessCShims.write(currentState.shutdownFileDescriptor, &one, MemoryLayout<UInt64>.stride)
@@ -226,7 +231,6 @@ final class AsyncIO: Sendable {
226231
}
227232
}
228233

229-
230234
private func registerFileDescriptor(
231235
_ fileDescriptor: FileDescriptor,
232236
for event: Event
@@ -277,11 +281,12 @@ final class AsyncIO: Sendable {
277281
&event
278282
)
279283
if rc != 0 {
284+
let capturedError = errno
280285
let error = SubprocessError(
281286
code: .init(.asyncIOFailed(
282287
"failed to add \(fileDescriptor.rawValue) to epoll list")
283288
),
284-
underlyingError: .init(rawValue: errno)
289+
underlyingError: .init(rawValue: capturedError)
285290
)
286291
continuation.finish(throwing: error)
287292
return
@@ -344,6 +349,9 @@ extension AsyncIO {
344349
from fileDescriptor: FileDescriptor,
345350
upTo maxLength: Int
346351
) async throws -> [UInt8]? {
352+
guard maxLength > 0 else {
353+
return nil
354+
}
347355
// If we are reading until EOF, start with readBufferSize
348356
// and gradually increase buffer size
349357
let bufferLength = maxLength == .max ? readBufferSize : maxLength
@@ -407,6 +415,7 @@ extension AsyncIO {
407415
}
408416
}
409417
}
418+
resultBuffer.removeLast(resultBuffer.count - readLength)
410419
return resultBuffer
411420
}
412421

@@ -421,6 +430,9 @@ extension AsyncIO {
421430
_ bytes: Bytes,
422431
to diskIO: borrowing IOChannel
423432
) async throws -> Int {
433+
guard bytes.count > 0 else {
434+
return 0
435+
}
424436
let fileDescriptor = diskIO.channel
425437
let signalStream = self.registerFileDescriptor(fileDescriptor, for: .write)
426438
var writtenLength: Int = 0
@@ -464,6 +476,9 @@ extension AsyncIO {
464476
_ span: borrowing RawSpan,
465477
to diskIO: borrowing IOChannel
466478
) async throws -> Int {
479+
guard span.byteCount > 0 else {
480+
return 0
481+
}
467482
let fileDescriptor = diskIO.channel
468483
let signalStream = self.registerFileDescriptor(fileDescriptor, for: .write)
469484
var writtenLength: Int = 0

‎Sources/Subprocess/IO/AsyncIO+Windows.swift

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ final class AsyncIO: @unchecked Sendable {
5151
static let shared = AsyncIO()
5252

5353
private let ioCompletionPort: Result<HANDLE, SubprocessError>
54-
5554
private let monitorThread: Result<HANDLE, SubprocessError>
55+
private let shutdownFlag: Atomic<UInt8> = Atomic(0)
5656

57-
private init() {
57+
internal init() {
5858
var maybeSetupError: SubprocessError? = nil
5959
// Create the the completion port
6060
guard let port = CreateIoCompletionPort(
@@ -78,10 +78,11 @@ final class AsyncIO: @unchecked Sendable {
7878
/// > thread management rather than CreateThread and ExitThread
7979
let threadHandleValue = _beginthreadex(nil, 0, { args in
8080
func reportError(_ error: SubprocessError) {
81-
_registration.withLock { store in
82-
for continuation in store.values {
83-
continuation.finish(throwing: error)
84-
}
81+
let continuations = _registration.withLock { store in
82+
return store.values
83+
}
84+
for continuation in continuations {
85+
continuation.finish(throwing: error)
8586
}
8687
}
8788

@@ -110,11 +111,13 @@ final class AsyncIO: @unchecked Sendable {
110111
// in the store. Windows does not offer an API to remove a
111112
// HANDLE from an IOCP port, therefore we leave the registration
112113
// to signify the HANDLE has already been resisted.
113-
_registration.withLock { store in
114+
let continuation = _registration.withLock { store -> SignalStream.Continuation? in
114115
if let continuation = store[targetFileDescriptor] {
115-
continuation.finish()
116+
return continuation
116117
}
118+
return nil
117119
}
120+
continuation?.finish()
118121
continue
119122
} else {
120123
let error = SubprocessError(
@@ -159,12 +162,17 @@ final class AsyncIO: @unchecked Sendable {
159162
}
160163
}
161164

162-
private func shutdown() {
163-
// Post status to shutdown HANDLE
165+
internal func shutdown() {
164166
guard case .success(let ioPort) = ioCompletionPort,
165167
case .success(let monitorThreadHandle) = monitorThread else {
166168
return
167169
}
170+
// Make sure we don't shutdown the same instance twice
171+
guard self.shutdownFlag.add(1, ordering: .relaxed).newValue == 1 else {
172+
// We already closed this AsyncIO
173+
return
174+
}
175+
// Post status to shutdown HANDLE
168176
PostQueuedCompletionStatus(
169177
ioPort, // CompletionPort
170178
0, // Number of bytes transferred.
@@ -245,6 +253,9 @@ final class AsyncIO: @unchecked Sendable {
245253
from handle: HANDLE,
246254
upTo maxLength: Int
247255
) async throws -> [UInt8]? {
256+
guard maxLength > 0 else {
257+
return nil
258+
}
248259
// If we are reading until EOF, start with readBufferSize
249260
// and gradually increase buffer size
250261
let bufferLength = maxLength == .max ? readBufferSize : maxLength
@@ -284,8 +295,12 @@ final class AsyncIO: @unchecked Sendable {
284295
// Make sure we only get `ERROR_IO_PENDING` or `ERROR_BROKEN_PIPE`
285296
let lastError = GetLastError()
286297
if lastError == ERROR_BROKEN_PIPE {
287-
// We reached EOF
288-
return nil
298+
// We reached EOF. Return whatever's left
299+
guard readLength > 0 else {
300+
return nil
301+
}
302+
resultBuffer.removeLast(resultBuffer.count - readLength)
303+
return resultBuffer
289304
}
290305
guard lastError == ERROR_IO_PENDING else {
291306
let error = SubprocessError(
@@ -337,6 +352,9 @@ final class AsyncIO: @unchecked Sendable {
337352
_ span: borrowing RawSpan,
338353
to diskIO: borrowing IOChannel
339354
) async throws -> Int {
355+
guard span.byteCount > 0 else {
356+
return 0
357+
}
340358
let handle = diskIO.channel
341359
var signalStream = self.registerHandle(diskIO.channel).makeAsyncIterator()
342360
var writtenLength: Int = 0
@@ -389,6 +407,9 @@ final class AsyncIO: @unchecked Sendable {
389407
_ bytes: Bytes,
390408
to diskIO: borrowing IOChannel
391409
) async throws -> Int {
410+
guard bytes.count > 0 else {
411+
return 0
412+
}
392413
let handle = diskIO.channel
393414
var signalStream = self.registerHandle(diskIO.channel).makeAsyncIterator()
394415
var writtenLength: Int = 0

‎Sources/Subprocess/IO/Input.swift

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,15 @@ public protocol InputProtocol: Sendable, ~Copyable {
4949
public struct NoInput: InputProtocol {
5050
internal func createPipe() throws -> CreatedPipe {
5151
#if os(Windows)
52-
// On Windows, instead of binding to dev null,
53-
// we don't set the input handle in the `STARTUPINFOW`
54-
// to signal no input
55-
return CreatedPipe(
56-
readFileDescriptor: nil,
57-
writeFileDescriptor: nil
58-
)
52+
let devnullFd: FileDescriptor = try .openDevNull(withAccessMode: .writeOnly)
53+
let devnull = HANDLE(bitPattern: _get_osfhandle(devnullFd.rawValue))!
5954
#else
6055
let devnull: FileDescriptor = try .openDevNull(withAccessMode: .readOnly)
56+
#endif
6157
return CreatedPipe(
6258
readFileDescriptor: .init(devnull, closeWhenDone: true),
6359
writeFileDescriptor: nil
6460
)
65-
#endif
6661
}
6762

6863
public func write(with writer: StandardInputWriter) async throws {

‎Sources/Subprocess/IO/Output.swift

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,17 @@ public struct DiscardedOutput: OutputProtocol {
5858
public typealias OutputType = Void
5959

6060
internal func createPipe() throws -> CreatedPipe {
61+
6162
#if os(Windows)
62-
// On Windows, instead of binding to dev null,
63-
// we don't set the input handle in the `STARTUPINFOW`
64-
// to signal no output
65-
return CreatedPipe(
66-
readFileDescriptor: nil,
67-
writeFileDescriptor: nil
68-
)
63+
let devnullFd: FileDescriptor = try .openDevNull(withAccessMode: .writeOnly)
64+
let devnull = HANDLE(bitPattern: _get_osfhandle(devnullFd.rawValue))!
6965
#else
70-
let devnull: FileDescriptor = try .openDevNull(withAccessMode: .readOnly)
66+
let devnull: FileDescriptor = try .openDevNull(withAccessMode: .writeOnly)
67+
#endif
7168
return CreatedPipe(
7269
readFileDescriptor: nil,
7370
writeFileDescriptor: .init(devnull, closeWhenDone: true)
7471
)
75-
#endif
7672
}
7773

7874
internal init() {}
@@ -289,6 +285,7 @@ extension OutputProtocol {
289285
from diskIO: consuming IOChannel?
290286
) async throws -> OutputType {
291287
if OutputType.self == Void.self {
288+
try diskIO?.safelyClose()
292289
return () as! OutputType
293290
}
294291
// `diskIO` is only `nil` for any types that conform to `OutputProtocol`
@@ -330,6 +327,7 @@ extension OutputProtocol {
330327
underlyingError: nil
331328
)
332329
}
330+
333331
#if canImport(Darwin)
334332
return try self.output(from: result ?? .empty)
335333
#else
@@ -400,3 +398,16 @@ extension DispatchData {
400398
return result ?? []
401399
}
402400
}
401+
402+
extension FileDescriptor {
403+
internal static func openDevNull(
404+
withAccessMode mode: FileDescriptor.AccessMode
405+
) throws -> FileDescriptor {
406+
#if os(Windows)
407+
let devnull: FileDescriptor = try .open("NUL", mode)
408+
#else
409+
let devnull: FileDescriptor = try .open("/dev/null", mode)
410+
#endif
411+
return devnull
412+
}
413+
}

0 commit comments

Comments
 (0)