Skip to content

Commit c2d5278

Browse files
iCharlesHuiCharlesHu
committed
Make CreatedPipe ~Copyable
1 parent 26abe91 commit c2d5278

File tree

5 files changed

+140
-82
lines changed

5 files changed

+140
-82
lines changed

‎Sources/Subprocess/API.swift

+8-8
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public func runDetached(
593593
withInput: try processInput.createPipe(),
594594
outputPipe: try processOutput.createPipe(),
595595
errorPipe: try processError.createPipe()
596-
).processIdentifier
596+
).execution.processIdentifier
597597
case (.none, .none, .some(let errorFd)):
598598
let processInput = NoInput()
599599
let processOutput = DiscardedOutput()
@@ -605,7 +605,7 @@ public func runDetached(
605605
withInput: try processInput.createPipe(),
606606
outputPipe: try processOutput.createPipe(),
607607
errorPipe: try processError.createPipe()
608-
).processIdentifier
608+
).execution.processIdentifier
609609
case (.none, .some(let outputFd), .none):
610610
let processInput = NoInput()
611611
let processOutput = FileDescriptorOutput(
@@ -616,7 +616,7 @@ public func runDetached(
616616
withInput: try processInput.createPipe(),
617617
outputPipe: try processOutput.createPipe(),
618618
errorPipe: try processError.createPipe()
619-
).processIdentifier
619+
).execution.processIdentifier
620620
case (.none, .some(let outputFd), .some(let errorFd)):
621621
let processInput = NoInput()
622622
let processOutput = FileDescriptorOutput(
@@ -631,7 +631,7 @@ public func runDetached(
631631
withInput: try processInput.createPipe(),
632632
outputPipe: try processOutput.createPipe(),
633633
errorPipe: try processError.createPipe()
634-
).processIdentifier
634+
).execution.processIdentifier
635635
case (.some(let inputFd), .none, .none):
636636
let processInput = FileDescriptorInput(
637637
fileDescriptor: inputFd,
@@ -643,7 +643,7 @@ public func runDetached(
643643
withInput: try processInput.createPipe(),
644644
outputPipe: try processOutput.createPipe(),
645645
errorPipe: try processError.createPipe()
646-
).processIdentifier
646+
).execution.processIdentifier
647647
case (.some(let inputFd), .none, .some(let errorFd)):
648648
let processInput = FileDescriptorInput(
649649
fileDescriptor: inputFd, closeAfterSpawningProcess: false
@@ -657,7 +657,7 @@ public func runDetached(
657657
withInput: try processInput.createPipe(),
658658
outputPipe: try processOutput.createPipe(),
659659
errorPipe: try processError.createPipe()
660-
).processIdentifier
660+
).execution.processIdentifier
661661
case (.some(let inputFd), .some(let outputFd), .none):
662662
let processInput = FileDescriptorInput(
663663
fileDescriptor: inputFd,
@@ -672,7 +672,7 @@ public func runDetached(
672672
withInput: try processInput.createPipe(),
673673
outputPipe: try processOutput.createPipe(),
674674
errorPipe: try processError.createPipe()
675-
).processIdentifier
675+
).execution.processIdentifier
676676
case (.some(let inputFd), .some(let outputFd), .some(let errorFd)):
677677
let processInput = FileDescriptorInput(
678678
fileDescriptor: inputFd,
@@ -690,7 +690,7 @@ public func runDetached(
690690
withInput: try processInput.createPipe(),
691691
outputPipe: try processOutput.createPipe(),
692692
errorPipe: try processError.createPipe()
693-
).processIdentifier
693+
).execution.processIdentifier
694694
}
695695
}
696696

‎Sources/Subprocess/Configuration.swift

+32-14
Original file line numberDiff line numberDiff line change
@@ -62,35 +62,41 @@ public struct Configuration: Sendable {
6262
@available(SubprocessSpan, *)
6363
#endif
6464
internal func run<Result>(
65-
input: CreatedPipe,
66-
output: CreatedPipe,
67-
error: CreatedPipe,
65+
input: consuming CreatedPipe,
66+
output: consuming CreatedPipe,
67+
error: consuming CreatedPipe,
6868
isolation: isolated (any Actor)? = #isolation,
6969
_ body: ((Execution, TrackedPlatformDiskIO?, TrackedPlatformDiskIO?, TrackedPlatformDiskIO?) async throws -> Result)
7070
) async throws -> ExecutionResult<Result> {
71-
let execution = try self.spawn(
71+
let spawnResults = try self.spawn(
7272
withInput: input,
7373
outputPipe: output,
7474
errorPipe: error
7575
)
76+
let pid = spawnResults.execution.processIdentifier
77+
78+
var spawnResultBox: SpawnResult?? = consume spawnResults
7679

7780
return try await withAsyncTaskCleanupHandler {
81+
let _spawnResult = spawnResultBox!.take()!
82+
7883
async let terminationStatus = try monitorProcessTermination(
79-
forProcessWithIdentifier: execution.processIdentifier
84+
forProcessWithIdentifier: _spawnResult.execution.processIdentifier
8085
)
8186
// Body runs in the same isolation
82-
let inputIO = input.createInputPlatformDiskIO()
83-
let outputIO = output.createOutputPlatformDiskIO()
84-
let errorIO = error.createOutputPlatformDiskIO()
85-
let result = try await body(execution, inputIO, outputIO, errorIO)
87+
let inputIO = _spawnResult.inputPipe.createInputPlatformDiskIO()
88+
let outputIO = _spawnResult.outputPipe.createOutputPlatformDiskIO()
89+
let errorIO = _spawnResult.errorPipe.createOutputPlatformDiskIO()
90+
let result = try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
8691
return ExecutionResult(
8792
terminationStatus: try await terminationStatus,
8893
value: result
8994
)
9095
} onCleanup: {
9196
// Attempt to terminate the child process
9297
await Execution.runTeardownSequence(
93-
self.platformOptions.teardownSequence, on: execution.processIdentifier
98+
self.platformOptions.teardownSequence,
99+
on: pid
94100
)
95101
}
96102
}
@@ -128,9 +134,9 @@ extension Configuration {
128134
/// Close each input individually, and throw the first error if there's multiple errors thrown
129135
@Sendable
130136
internal func cleanupPreSpawn(
131-
input: CreatedPipe,
132-
output: CreatedPipe,
133-
error: CreatedPipe
137+
input: borrowing CreatedPipe,
138+
output: borrowing CreatedPipe,
139+
error: borrowing CreatedPipe
134140
) throws {
135141
var inputError: Swift.Error?
136142
var outputError: Swift.Error?
@@ -472,6 +478,18 @@ extension TerminationStatus: CustomStringConvertible, CustomDebugStringConvertib
472478

473479
// MARK: - Internal
474480

481+
extension Configuration {
482+
#if SubprocessSpan
483+
@available(SubprocessSpan, *)
484+
#endif
485+
internal struct SpawnResult: ~Copyable {
486+
let execution: Execution
487+
let inputPipe: CreatedPipe
488+
let outputPipe: CreatedPipe
489+
let errorPipe: CreatedPipe
490+
}
491+
}
492+
475493
internal enum StringOrRawBytes: Sendable, Hashable {
476494
case string(String)
477495
case rawBytes([UInt8])
@@ -593,7 +611,7 @@ internal struct TrackedDispatchIO {
593611
}
594612
#endif
595613

596-
internal struct CreatedPipe {
614+
internal struct CreatedPipe: ~Copyable {
597615
internal let readFileDescriptor: TrackedFileDescriptor?
598616
internal let writeFileDescriptor: TrackedFileDescriptor?
599617

‎Sources/Subprocess/Platforms/Subprocess+Darwin.swift

+54-37
Original file line numberDiff line numberDiff line change
@@ -157,17 +157,25 @@ extension Configuration {
157157
@available(SubprocessSpan, *)
158158
#endif
159159
internal func spawn(
160-
withInput inputPipe: CreatedPipe,
161-
outputPipe: CreatedPipe,
162-
errorPipe: CreatedPipe
163-
) throws -> Execution {
160+
withInput inputPipe: consuming CreatedPipe,
161+
outputPipe: consuming CreatedPipe,
162+
errorPipe: consuming CreatedPipe
163+
) throws -> SpawnResult {
164164
// Instead of checking if every possible executable path
165165
// is valid, spawn each directly and catch ENOENT
166166
let possiblePaths = self.executable.possibleExecutablePaths(
167167
withPathValue: self.environment.pathValue()
168168
)
169-
return try self.preSpawn { args throws -> Execution in
169+
var inputPipeBox: CreatedPipe?? = consume inputPipe
170+
var outputPipeBox: CreatedPipe?? = consume outputPipe
171+
var errorPipeBox: CreatedPipe?? = consume errorPipe
172+
173+
return try self.preSpawn { args throws -> SpawnResult in
170174
let (env, uidPtr, gidPtr, supplementaryGroups) = args
175+
let _inputPipe = inputPipeBox!.take()!
176+
let _outputPipe = outputPipeBox!.take()!
177+
let _errorPipe = errorPipeBox!.take()!
178+
171179
for possibleExecutablePath in possiblePaths {
172180
var pid: pid_t = 0
173181

@@ -187,67 +195,68 @@ extension Configuration {
187195
defer {
188196
posix_spawn_file_actions_destroy(&fileActions)
189197
}
198+
190199
// Input
191200
var result: Int32 = -1
192-
if let inputRead = inputPipe.readFileDescriptor {
201+
if let inputRead = _inputPipe.readFileDescriptor {
193202
result = posix_spawn_file_actions_adddup2(&fileActions, inputRead.platformDescriptor, 0)
194203
guard result == 0 else {
195-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
204+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
196205
throw SubprocessError(
197206
code: .init(.spawnFailed),
198207
underlyingError: .init(rawValue: result)
199208
)
200209
}
201210
}
202-
if let inputWrite = inputPipe.writeFileDescriptor {
211+
if let inputWrite = _inputPipe.writeFileDescriptor {
203212
// Close parent side
204213
result = posix_spawn_file_actions_addclose(&fileActions, inputWrite.platformDescriptor)
205214
guard result == 0 else {
206-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
215+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
207216
throw SubprocessError(
208217
code: .init(.spawnFailed),
209218
underlyingError: .init(rawValue: result)
210219
)
211220
}
212221
}
213222
// Output
214-
if let outputWrite = outputPipe.writeFileDescriptor {
223+
if let outputWrite = _outputPipe.writeFileDescriptor {
215224
result = posix_spawn_file_actions_adddup2(&fileActions, outputWrite.platformDescriptor, 1)
216225
guard result == 0 else {
217-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
226+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
218227
throw SubprocessError(
219228
code: .init(.spawnFailed),
220229
underlyingError: .init(rawValue: result)
221230
)
222231
}
223232
}
224-
if let outputRead = outputPipe.readFileDescriptor {
233+
if let outputRead = _outputPipe.readFileDescriptor {
225234
// Close parent side
226235
result = posix_spawn_file_actions_addclose(&fileActions, outputRead.platformDescriptor)
227236
guard result == 0 else {
228-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
237+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
229238
throw SubprocessError(
230239
code: .init(.spawnFailed),
231240
underlyingError: .init(rawValue: result)
232241
)
233242
}
234243
}
235244
// Error
236-
if let errorWrite = errorPipe.writeFileDescriptor {
245+
if let errorWrite = _errorPipe.writeFileDescriptor {
237246
result = posix_spawn_file_actions_adddup2(&fileActions, errorWrite.platformDescriptor, 2)
238247
guard result == 0 else {
239-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
248+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
240249
throw SubprocessError(
241250
code: .init(.spawnFailed),
242251
underlyingError: .init(rawValue: result)
243252
)
244253
}
245254
}
246-
if let errorRead = errorPipe.readFileDescriptor {
255+
if let errorRead = _errorPipe.readFileDescriptor {
247256
// Close parent side
248257
result = posix_spawn_file_actions_addclose(&fileActions, errorRead.platformDescriptor)
249258
guard result == 0 else {
250-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
259+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
251260
throw SubprocessError(
252261
code: .init(.spawnFailed),
253262
underlyingError: .init(rawValue: result)
@@ -290,7 +299,7 @@ extension Configuration {
290299

291300
// Error handling
292301
if chdirError != 0 || spawnAttributeError != 0 {
293-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
302+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
294303
if spawnAttributeError != 0 {
295304
throw SubprocessError(
296305
code: .init(.spawnFailed),
@@ -336,34 +345,36 @@ extension Configuration {
336345
}
337346
// Throw all other errors
338347
try self.cleanupPreSpawn(
339-
input: inputPipe,
340-
output: outputPipe,
341-
error: errorPipe
348+
input: _inputPipe,
349+
output: _outputPipe,
350+
error: _errorPipe
342351
)
343352
throw SubprocessError(
344353
code: .init(.spawnFailed),
345354
underlyingError: .init(rawValue: spawnError)
346355
)
347356
}
348357

349-
func captureError(_ work: () throws -> Void) -> (any Swift.Error)? {
350-
do {
351-
try work()
352-
return nil
353-
} catch {
354-
return error
355-
}
356-
}
357358
// After spawn finishes, close all child side fds
358-
let inputCloseError = captureError {
359-
try inputPipe.readFileDescriptor?.safelyClose()
359+
var inputCloseError: (any Swift.Error)? = nil
360+
do {
361+
try _inputPipe.readFileDescriptor?.safelyClose()
362+
} catch {
363+
inputCloseError = error
360364
}
361-
let outputCloseError = captureError {
362-
try outputPipe.writeFileDescriptor?.safelyClose()
365+
var outputCloseError: (any Swift.Error)? = nil
366+
do {
367+
try _outputPipe.writeFileDescriptor?.safelyClose()
368+
} catch {
369+
outputCloseError = error
363370
}
364-
let errorCloseError = captureError {
365-
try errorPipe.writeFileDescriptor?.safelyClose()
371+
var errorCloseError: (any Swift.Error)? = nil
372+
do {
373+
try _errorPipe.writeFileDescriptor?.safelyClose()
374+
} catch {
375+
errorCloseError = error
366376
}
377+
367378
if let inputCloseError = inputCloseError {
368379
throw inputCloseError
369380
}
@@ -374,17 +385,23 @@ extension Configuration {
374385
throw errorCloseError
375386
}
376387

377-
return Execution(
388+
let execution = Execution(
378389
processIdentifier: .init(value: pid)
379390
)
391+
return SpawnResult(
392+
execution: execution,
393+
inputPipe: _inputPipe,
394+
outputPipe: _outputPipe,
395+
errorPipe: _errorPipe
396+
)
380397
}
381398

382399
// If we reach this point, it means either the executable path
383400
// or working directory is not valid. Since posix_spawn does not
384401
// provide which one is not valid, here we make a best effort guess
385402
// by checking whether the working directory is valid. This technically
386403
// still causes TOUTOC issue, but it's the best we can do for error recovery.
387-
try self.cleanupPreSpawn(input: inputPipe, output: outputPipe, error: errorPipe)
404+
try self.cleanupPreSpawn(input: _inputPipe, output: _outputPipe, error: _errorPipe)
388405
let workingDirectory = self.workingDirectory.string
389406
guard Configuration.pathAccessible(workingDirectory, mode: F_OK) else {
390407
throw SubprocessError(

0 commit comments

Comments
 (0)