Skip to content

Commit 69b4973

Browse files
committed
Block signal masks pre-fork and restore post-fork.
On Darwin and Linux, rearchitect `TrackedFileDescriptor` (now named `DiskIO`) to either hold a `FileDescriptor` or a `DispatchIO`. We create `DispatchIO` as the last step of `spawn()` and use it to perform read/write, as well as cleanup instead of using a raw file descriptor.
1 parent 1612896 commit 69b4973

13 files changed

+524
-232
lines changed

Sources/Subprocess/AsyncBufferSequence.swift

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,39 +26,39 @@ public struct AsyncBufferSequence: AsyncSequence, Sendable {
2626
public struct Iterator: AsyncIteratorProtocol {
2727
public typealias Element = SequenceOutput.Buffer
2828

29-
private let fileDescriptor: TrackedFileDescriptor
29+
private let diskIO: DiskIO
3030
private var buffer: [UInt8]
3131
private var currentPosition: Int
3232
private var finished: Bool
3333

34-
internal init(fileDescriptor: TrackedFileDescriptor) {
35-
self.fileDescriptor = fileDescriptor
34+
internal init(diskIO: DiskIO) {
35+
self.diskIO = diskIO
3636
self.buffer = []
3737
self.currentPosition = 0
3838
self.finished = false
3939
}
4040

41-
public mutating func next() async throws -> SequenceOutput.Buffer? {
42-
let data = try await self.fileDescriptor.wrapped.readChunk(
41+
public func next() async throws -> SequenceOutput.Buffer? {
42+
let data = try await self.diskIO.readChunk(
4343
upToLength: readBufferSize
4444
)
4545
if data == nil {
4646
// We finished reading. Close the file descriptor now
47-
try self.fileDescriptor.safelyClose()
47+
try self.diskIO.safelyClose()
4848
return nil
4949
}
5050
return data
5151
}
5252
}
5353

54-
private let fileDescriptor: TrackedFileDescriptor
54+
private let diskIO: DiskIO
5555

56-
init(fileDescriptor: TrackedFileDescriptor) {
57-
self.fileDescriptor = fileDescriptor
56+
internal init(diskIO: DiskIO) {
57+
self.diskIO = diskIO
5858
}
5959

6060
public func makeAsyncIterator() -> Iterator {
61-
return Iterator(fileDescriptor: self.fileDescriptor)
61+
return Iterator(diskIO: self.diskIO)
6262
}
6363
}
6464

Sources/Subprocess/Configuration.swift

Lines changed: 63 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,6 @@ public struct Configuration: Sendable {
9090
// After spawn, cleanup child side fds
9191
try await self.cleanup(
9292
execution: execution,
93-
inputPipe: inputPipe,
94-
outputPipe: outputPipe,
95-
errorPipe: errorPipe,
9693
childSide: true,
9794
parentSide: false,
9895
attemptToTerminateSubProcess: false
@@ -104,7 +101,7 @@ public struct Configuration: Sendable {
104101
// Body runs in the same isolation
105102
let result = try await body(
106103
execution,
107-
.init(fileDescriptor: inputPipe.writeFileDescriptor!)
104+
.init(fileDescriptor: execution.inputPipe.writeFileDescriptor!)
108105
)
109106
return ExecutionResult(
110107
terminationStatus: try await waitingStatus,
@@ -116,9 +113,6 @@ public struct Configuration: Sendable {
116113
// this is the best we can do
117114
try? await self.cleanup(
118115
execution: execution,
119-
inputPipe: inputPipe,
120-
outputPipe: outputPipe,
121-
errorPipe: errorPipe,
122116
childSide: false,
123117
parentSide: true,
124118
attemptToTerminateSubProcess: true
@@ -154,9 +148,6 @@ public struct Configuration: Sendable {
154148
// After spawn, clean up child side
155149
try await self.cleanup(
156150
execution: execution,
157-
inputPipe: inputPipe,
158-
outputPipe: outputPipe,
159-
errorPipe: errorPipe,
160151
childSide: true,
161152
parentSide: false,
162153
attemptToTerminateSubProcess: false
@@ -174,7 +165,7 @@ public struct Configuration: Sendable {
174165
standardError
175166
) = try await execution.captureIOs()
176167
// Write input in the same scope
177-
guard let writeFd = inputPipe.writeFileDescriptor else {
168+
guard let writeFd = execution.inputPipe.writeFileDescriptor else {
178169
fatalError("Trying to write to an input that has been closed")
179170
}
180171
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, any Swift.Error>) in
@@ -193,7 +184,7 @@ public struct Configuration: Sendable {
193184
)
194185
#endif
195186

196-
writeFd.wrapped.write(bytes) { _, error in
187+
writeFd.write(bytes) { _, error in
197188
if let error = error {
198189
continuation.resume(throwing: error)
199190
} else {
@@ -216,9 +207,6 @@ public struct Configuration: Sendable {
216207
// this is the best we can do
217208
try? await self.cleanup(
218209
execution: execution,
219-
inputPipe: inputPipe,
220-
outputPipe: outputPipe,
221-
errorPipe: errorPipe,
222210
childSide: false,
223211
parentSide: true,
224212
attemptToTerminateSubProcess: true
@@ -257,9 +245,6 @@ public struct Configuration: Sendable {
257245
// After spawn, clean up child side
258246
try await self.cleanup(
259247
execution: execution,
260-
inputPipe: inputPipe,
261-
outputPipe: outputPipe,
262-
errorPipe: errorPipe,
263248
childSide: true,
264249
parentSide: false,
265250
attemptToTerminateSubProcess: false
@@ -271,7 +256,7 @@ public struct Configuration: Sendable {
271256
returning: ExecutionResult.self
272257
) { group in
273258
group.addTask {
274-
if let writeFd = inputPipe.writeFileDescriptor {
259+
if let writeFd = execution.inputPipe.writeFileDescriptor {
275260
let writer = StandardInputWriter(fileDescriptor: writeFd)
276261
try await input.write(with: writer)
277262
try await writer.finish()
@@ -300,9 +285,6 @@ public struct Configuration: Sendable {
300285
// this is the best we can do
301286
try? await self.cleanup(
302287
execution: execution,
303-
inputPipe: inputPipe,
304-
outputPipe: outputPipe,
305-
errorPipe: errorPipe,
306288
childSide: false,
307289
parentSide: true,
308290
attemptToTerminateSubProcess: true
@@ -350,9 +332,6 @@ extension Configuration {
350332
Error: OutputProtocol
351333
>(
352334
execution: Execution<Output, Error>,
353-
inputPipe: CreatedPipe,
354-
outputPipe: CreatedPipe,
355-
errorPipe: CreatedPipe,
356335
childSide: Bool,
357336
parentSide: Bool,
358337
attemptToTerminateSubProcess: Bool
@@ -384,25 +363,25 @@ extension Configuration {
384363

385364
if childSide {
386365
inputError = captureError {
387-
try inputPipe.readFileDescriptor?.safelyClose()
366+
try execution.inputPipe.readFileDescriptor?.safelyClose()
388367
}
389368
outputError = captureError {
390-
try outputPipe.writeFileDescriptor?.safelyClose()
369+
try execution.outputPipe.writeFileDescriptor?.safelyClose()
391370
}
392371
errorError = captureError {
393-
try errorPipe.writeFileDescriptor?.safelyClose()
372+
try execution.errorPipe.writeFileDescriptor?.safelyClose()
394373
}
395374
}
396375

397376
if parentSide {
398377
inputError = captureError {
399-
try inputPipe.writeFileDescriptor?.safelyClose()
378+
try execution.inputPipe.writeFileDescriptor?.safelyClose()
400379
}
401380
outputError = captureError {
402-
try outputPipe.readFileDescriptor?.safelyClose()
381+
try execution.outputPipe.readFileDescriptor?.safelyClose()
403382
}
404383
errorError = captureError {
405-
try errorPipe.readFileDescriptor?.safelyClose()
384+
try execution.errorPipe.readFileDescriptor?.safelyClose()
406385
}
407386
}
408387

@@ -822,27 +801,51 @@ internal enum StringOrRawBytes: Sendable, Hashable {
822801
}
823802
}
824803

825-
/// A simple wrapper on `FileDescriptor` plus a flag indicating
826-
/// whether it should be closed automactially when done.
827-
internal struct TrackedFileDescriptor: Hashable {
804+
/// A wrapped `FileDescriptor` or `DispatchIO` and
805+
/// whether it should beeddsw closed automactially when done.
806+
internal struct DiskIO {
807+
internal enum Storage {
808+
case fileDescriptor(FileDescriptor)
809+
#if !os(Windows) // Darwin and Linux
810+
case dispatchIO(DispatchIO)
811+
#endif
812+
}
813+
828814
internal let closeWhenDone: Bool
829-
internal let wrapped: FileDescriptor
815+
internal let storage: Storage
830816

831817
internal init(
832-
_ wrapped: FileDescriptor,
818+
_ fileDescriptor: FileDescriptor,
833819
closeWhenDone: Bool
834820
) {
835-
self.wrapped = wrapped
821+
self.storage = .fileDescriptor(fileDescriptor)
836822
self.closeWhenDone = closeWhenDone
837823
}
838824

825+
#if !os(Windows)
826+
internal init(
827+
_ dispatchIO: DispatchIO,
828+
closeWhenDone: Bool
829+
) {
830+
self.storage = .dispatchIO(dispatchIO)
831+
self.closeWhenDone = closeWhenDone
832+
}
833+
#endif
834+
839835
internal func safelyClose() throws {
840836
guard self.closeWhenDone else {
841837
return
842838
}
843839

844840
do {
845-
try self.wrapped.close()
841+
switch self.storage {
842+
case .fileDescriptor(let fileDescriptor):
843+
try fileDescriptor.close()
844+
#if !os(Windows)
845+
case .dispatchIO(let dispatchIO):
846+
dispatchIO.close()
847+
#endif
848+
}
846849
} catch {
847850
guard let errno: Errno = error as? Errno else {
848851
throw error
@@ -854,25 +857,39 @@ internal struct TrackedFileDescriptor: Hashable {
854857
}
855858

856859
internal var platformDescriptor: PlatformFileDescriptor {
857-
return self.wrapped.platformDescriptor
860+
switch self.storage {
861+
case .fileDescriptor(let fileDescriptor):
862+
return fileDescriptor.platformDescriptor
863+
#if !os(Windows)
864+
case .dispatchIO(let dispatchIO):
865+
return dispatchIO.fileDescriptor
866+
#endif // !os(Windows)
867+
}
858868
}
859869
}
860870

861871
internal struct CreatedPipe {
862-
internal let readFileDescriptor: TrackedFileDescriptor?
863-
internal let writeFileDescriptor: TrackedFileDescriptor?
872+
internal enum PipeEnd {
873+
case readEnd
874+
case writeEnd
875+
}
876+
877+
internal let readFileDescriptor: DiskIO?
878+
internal let writeFileDescriptor: DiskIO?
879+
internal let parentEnd: PipeEnd
864880

865881
internal init(
866-
readFileDescriptor: TrackedFileDescriptor?,
867-
writeFileDescriptor: TrackedFileDescriptor?
882+
readFileDescriptor: DiskIO?,
883+
writeFileDescriptor: DiskIO?,
884+
parentEnd: PipeEnd
868885
) {
869886
self.readFileDescriptor = readFileDescriptor
870887
self.writeFileDescriptor = writeFileDescriptor
888+
self.parentEnd = parentEnd
871889
}
872890

873-
internal init(closeWhenDone: Bool) throws {
891+
internal init(closeWhenDone: Bool, parentEnd: PipeEnd) throws {
874892
let pipe = try FileDescriptor.ssp_pipe()
875-
876893
self.readFileDescriptor = .init(
877894
pipe.readEnd,
878895
closeWhenDone: closeWhenDone
@@ -881,6 +898,7 @@ internal struct CreatedPipe {
881898
pipe.writeEnd,
882899
closeWhenDone: closeWhenDone
883900
)
901+
self.parentEnd = parentEnd
884902
}
885903
}
886904

Sources/Subprocess/Error.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ extension SubprocessError: CustomStringConvertible, CustomDebugStringConvertible
9292
public var description: String {
9393
switch self.code.storage {
9494
case .spawnFailed:
95-
return "Failed to spawn the new process."
95+
return "Failed to spawn the new process with underlying error: \(self.underlyingError!)"
9696
case .executableNotFound(let executableName):
9797
return "Executable \"\(executableName)\" is not found or cannot be executed."
9898
case .failedToChangeWorkingDirectory(let workingDirectory):

Sources/Subprocess/Execution.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public final class Execution<
4242

4343
internal let output: Output
4444
internal let error: Error
45+
internal let inputPipe: CreatedPipe
4546
internal let outputPipe: CreatedPipe
4647
internal let errorPipe: CreatedPipe
4748
internal let outputConsumptionState: AtomicBox
@@ -53,13 +54,15 @@ public final class Execution<
5354
processIdentifier: ProcessIdentifier,
5455
output: Output,
5556
error: Error,
57+
inputPipe: CreatedPipe,
5658
outputPipe: CreatedPipe,
5759
errorPipe: CreatedPipe,
5860
consoleBehavior: PlatformOptions.ConsoleBehavior
5961
) {
6062
self.processIdentifier = processIdentifier
6163
self.output = output
6264
self.error = error
65+
self.inputPipe = inputPipe
6366
self.outputPipe = outputPipe
6467
self.errorPipe = errorPipe
6568
self.outputConsumptionState = AtomicBox()
@@ -70,12 +73,14 @@ public final class Execution<
7073
processIdentifier: ProcessIdentifier,
7174
output: Output,
7275
error: Error,
76+
inputPipe: CreatedPipe,
7377
outputPipe: CreatedPipe,
7478
errorPipe: CreatedPipe
7579
) {
7680
self.processIdentifier = processIdentifier
7781
self.output = output
7882
self.error = error
83+
self.inputPipe = inputPipe
7984
self.outputPipe = outputPipe
8085
self.errorPipe = errorPipe
8186
self.outputConsumptionState = AtomicBox()
@@ -98,11 +103,11 @@ extension Execution where Output == SequenceOutput {
98103
)
99104

100105
guard consumptionState.contains(.standardOutputConsumed),
101-
let fd = self.outputPipe.readFileDescriptor
106+
let readFd = self.outputPipe.readFileDescriptor
102107
else {
103108
fatalError("The standard output has already been consumed")
104109
}
105-
return AsyncBufferSequence(fileDescriptor: fd)
110+
return AsyncBufferSequence(diskIO: readFd)
106111
}
107112
}
108113

@@ -121,11 +126,11 @@ extension Execution where Error == SequenceOutput {
121126
)
122127

123128
guard consumptionState.contains(.standardErrorConsumed),
124-
let fd = self.errorPipe.readFileDescriptor
129+
let readFd = self.errorPipe.readFileDescriptor
125130
else {
126131
fatalError("The standard output has already been consumed")
127132
}
128-
return AsyncBufferSequence(fileDescriptor: fd)
133+
return AsyncBufferSequence(diskIO: readFd)
129134
}
130135
}
131136

0 commit comments

Comments
 (0)