Skip to content

Commit b66271c

Browse files
committed
Ensure monitorProcessTermination is ALWAYS called after body runs
This fixes a regression introduced by #88. We need to ensure monitorProcessTermination is ALWAYS called after body runs, regardless of whether it threw an error or not. Closes #89
1 parent 1038151 commit b66271c

File tree

1 file changed

+26
-17
lines changed

1 file changed

+26
-17
lines changed

Sources/Subprocess/Configuration.swift

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -78,25 +78,34 @@ public struct Configuration: Sendable {
7878

7979
let processIdentifier = _spawnResult.execution.processIdentifier
8080

81-
let result = try await withAsyncTaskCleanupHandler {
82-
let inputIO = _spawnResult.inputWriteEnd()
83-
let outputIO = _spawnResult.outputReadEnd()
84-
let errorIO = _spawnResult.errorReadEnd()
85-
86-
// Body runs in the same isolation
87-
return try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
88-
} onCleanup: {
89-
// Attempt to terminate the child process
90-
await Execution.runTeardownSequence(
91-
self.platformOptions.teardownSequence,
92-
on: pid
93-
)
81+
let result: Swift.Result<Result, Error>
82+
do {
83+
result = try await .success(withAsyncTaskCleanupHandler {
84+
let inputIO = _spawnResult.inputWriteEnd()
85+
let outputIO = _spawnResult.outputReadEnd()
86+
let errorIO = _spawnResult.errorReadEnd()
87+
88+
// Body runs in the same isolation
89+
return try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
90+
} onCleanup: {
91+
// Attempt to terminate the child process
92+
await Execution.runTeardownSequence(
93+
self.platformOptions.teardownSequence,
94+
on: pid
95+
)
96+
})
97+
} catch {
98+
result = .failure(error)
9499
}
95100

96-
return ExecutionResult(
97-
terminationStatus: try await monitorProcessTermination(forProcessWithIdentifier: processIdentifier),
98-
value: result
99-
)
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(forProcessWithIdentifier: processIdentifier)
107+
108+
return try ExecutionResult(terminationStatus: terminationStatus, value: result.get())
100109
}
101110
}
102111

0 commit comments

Comments
 (0)