Skip to content

Commit 7fb7ee8

Browse files
jakepetroulesjakepetroules
authored
Remove runDetached API (#95)
Currently runDetached returns a ProcessIdentifier, which contains a pidfd on Linux and HANDLE on Windows. These handles are closed as soon as runDetached returns, making the values effectively useless to the caller. Similar to #92, there is a Windows-specific race condition here. On Unix the pid will be valid until someone waitid's it, so a caller _could_ use pidfd_open to get a new pidfd. But on Windows the ProcessIdentifier may already be invalid by the time runDetached returns and therefore OpenProcess can't be used to recover a handle. On FreeBSD one can't get a process descriptor from a PID at all (PDs are only for child processes). In order to avoid these race conditions, runDetached would need to either return a type which manages the lifetime of the various platform-specific handles, or leak the handles and leave them to be managed by the user. Due to questionable utility of these APIs in the first place (the closure based API can be used in conjunction with a Task to do pretty much everything runDetached can, without the downsides), we simply remove them. Closes #94
1 parent f3f9512 commit 7fb7ee8

File tree

5 files changed

+2
-247
lines changed

5 files changed

+2
-247
lines changed

‎README.md‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,6 @@ async let monitorResult = run(
7878
}
7979
```
8080

81-
### Running Unmonitored Processes
82-
83-
While `Subprocess` is designed with Swift’s structural concurrency in mind, it also provides a lower level, synchronous method for launching child processes. However, since `Subprocess` can’t synchronously monitor child process’s state or handle cleanup, you’ll need to attach a FileDescriptor to each I/O directly. Remember to close the `FileDescriptor` once you’re finished.
84-
85-
```swift
86-
import Subprocess
87-
88-
let input: FileDescriptor = ...
89-
90-
input.closeAfter {
91-
let pid = try runDetached(.path("/bin/daemon"), input: input)
92-
// ... other opeartions
93-
}
94-
```
95-
9681
### Customizable Execution
9782

9883
You can set various parameters when running the child process, such as `Arguments`, `Environment`, and working directory:

‎Sources/Subprocess/API.swift‎

Lines changed: 0 additions & 172 deletions
Original file line numberDiff line numberDiff line change
@@ -585,175 +585,3 @@ public func run<Result>(
585585
return try await body(execution, writer, outputSequence, errorSequence)
586586
}
587587
}
588-
589-
// MARK: - Detached
590-
591-
/// Run an executable with given parameters and return its process
592-
/// identifier immediately without monitoring the state of the
593-
/// subprocess nor waiting until it exits.
594-
///
595-
/// This method is useful for launching subprocesses that outlive their
596-
/// parents (for example, daemons and trampolines).
597-
///
598-
/// - Parameters:
599-
/// - executable: The executable to run.
600-
/// - arguments: The arguments to pass to the executable.
601-
/// - environment: The environment to use for the process.
602-
/// - workingDirectory: The working directory for the process.
603-
/// - platformOptions: The platform specific options to use for the process.
604-
/// - input: A file descriptor to bind to the subprocess' standard input.
605-
/// - output: A file descriptor to bind to the subprocess' standard output.
606-
/// - error: A file descriptor to bind to the subprocess' standard error.
607-
/// - Returns: the process identifier for the subprocess.
608-
public func runDetached(
609-
_ executable: Executable,
610-
arguments: Arguments = [],
611-
environment: Environment = .inherit,
612-
workingDirectory: FilePath? = nil,
613-
platformOptions: PlatformOptions = PlatformOptions(),
614-
input: FileDescriptor? = nil,
615-
output: FileDescriptor? = nil,
616-
error: FileDescriptor? = nil
617-
) throws -> ProcessIdentifier {
618-
let config: Configuration = Configuration(
619-
executable: executable,
620-
arguments: arguments,
621-
environment: environment,
622-
workingDirectory: workingDirectory,
623-
platformOptions: platformOptions
624-
)
625-
return try runDetached(config, input: input, output: output, error: error)
626-
}
627-
628-
/// Run an executable with given configuration and return its process
629-
/// identifier immediately without monitoring the state of the
630-
/// subprocess nor waiting until it exits.
631-
///
632-
/// This method is useful for launching subprocesses that outlive their
633-
/// parents (for example, daemons and trampolines).
634-
///
635-
/// - Parameters:
636-
/// - configuration: The `Subprocess` configuration to run.
637-
/// - input: A file descriptor to bind to the subprocess' standard input.
638-
/// - output: A file descriptor to bind to the subprocess' standard output.
639-
/// - error: A file descriptor to bind to the subprocess' standard error.
640-
/// - Returns: the process identifier for the subprocess.
641-
public func runDetached(
642-
_ configuration: Configuration,
643-
input: FileDescriptor? = nil,
644-
output: FileDescriptor? = nil,
645-
error: FileDescriptor? = nil
646-
) throws -> ProcessIdentifier {
647-
let execution: Execution
648-
switch (input, output, error) {
649-
case (.none, .none, .none):
650-
let processInput = NoInput()
651-
let processOutput = DiscardedOutput()
652-
let processError = DiscardedOutput()
653-
execution = try configuration.spawn(
654-
withInput: try processInput.createPipe(),
655-
outputPipe: try processOutput.createPipe(),
656-
errorPipe: try processError.createPipe()
657-
).execution
658-
case (.none, .none, .some(let errorFd)):
659-
let processInput = NoInput()
660-
let processOutput = DiscardedOutput()
661-
let processError = FileDescriptorOutput(
662-
fileDescriptor: errorFd,
663-
closeAfterSpawningProcess: false
664-
)
665-
execution = try configuration.spawn(
666-
withInput: try processInput.createPipe(),
667-
outputPipe: try processOutput.createPipe(),
668-
errorPipe: try processError.createPipe()
669-
).execution
670-
case (.none, .some(let outputFd), .none):
671-
let processInput = NoInput()
672-
let processOutput = FileDescriptorOutput(
673-
fileDescriptor: outputFd, closeAfterSpawningProcess: false
674-
)
675-
let processError = DiscardedOutput()
676-
execution = try configuration.spawn(
677-
withInput: try processInput.createPipe(),
678-
outputPipe: try processOutput.createPipe(),
679-
errorPipe: try processError.createPipe()
680-
).execution
681-
case (.none, .some(let outputFd), .some(let errorFd)):
682-
let processInput = NoInput()
683-
let processOutput = FileDescriptorOutput(
684-
fileDescriptor: outputFd,
685-
closeAfterSpawningProcess: false
686-
)
687-
let processError = FileDescriptorOutput(
688-
fileDescriptor: errorFd,
689-
closeAfterSpawningProcess: false
690-
)
691-
execution = try configuration.spawn(
692-
withInput: try processInput.createPipe(),
693-
outputPipe: try processOutput.createPipe(),
694-
errorPipe: try processError.createPipe()
695-
).execution
696-
case (.some(let inputFd), .none, .none):
697-
let processInput = FileDescriptorInput(
698-
fileDescriptor: inputFd,
699-
closeAfterSpawningProcess: false
700-
)
701-
let processOutput = DiscardedOutput()
702-
let processError = DiscardedOutput()
703-
execution = try configuration.spawn(
704-
withInput: try processInput.createPipe(),
705-
outputPipe: try processOutput.createPipe(),
706-
errorPipe: try processError.createPipe()
707-
).execution
708-
case (.some(let inputFd), .none, .some(let errorFd)):
709-
let processInput = FileDescriptorInput(
710-
fileDescriptor: inputFd, closeAfterSpawningProcess: false
711-
)
712-
let processOutput = DiscardedOutput()
713-
let processError = FileDescriptorOutput(
714-
fileDescriptor: errorFd,
715-
closeAfterSpawningProcess: false
716-
)
717-
execution = try configuration.spawn(
718-
withInput: try processInput.createPipe(),
719-
outputPipe: try processOutput.createPipe(),
720-
errorPipe: try processError.createPipe()
721-
).execution
722-
case (.some(let inputFd), .some(let outputFd), .none):
723-
let processInput = FileDescriptorInput(
724-
fileDescriptor: inputFd,
725-
closeAfterSpawningProcess: false
726-
)
727-
let processOutput = FileDescriptorOutput(
728-
fileDescriptor: outputFd,
729-
closeAfterSpawningProcess: false
730-
)
731-
let processError = DiscardedOutput()
732-
execution = try configuration.spawn(
733-
withInput: try processInput.createPipe(),
734-
outputPipe: try processOutput.createPipe(),
735-
errorPipe: try processError.createPipe()
736-
).execution
737-
case (.some(let inputFd), .some(let outputFd), .some(let errorFd)):
738-
let processInput = FileDescriptorInput(
739-
fileDescriptor: inputFd,
740-
closeAfterSpawningProcess: false
741-
)
742-
let processOutput = FileDescriptorOutput(
743-
fileDescriptor: outputFd,
744-
closeAfterSpawningProcess: false
745-
)
746-
let processError = FileDescriptorOutput(
747-
fileDescriptor: errorFd,
748-
closeAfterSpawningProcess: false
749-
)
750-
execution = try configuration.spawn(
751-
withInput: try processInput.createPipe(),
752-
outputPipe: try processOutput.createPipe(),
753-
errorPipe: try processError.createPipe()
754-
).execution
755-
}
756-
execution.release()
757-
return execution.processIdentifier
758-
}
759-

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ extension Configuration {
154154
errorWrite: errorWriteFileDescriptor
155155
)
156156
} catch {
157-
// If spawn() throws, monitorProcessTermination or runDetached
157+
// If spawn() throws, monitorProcessTermination
158158
// won't have an opportunity to call release, so do it here to avoid leaking the handles.
159159
execution.release()
160160
throw error
@@ -298,7 +298,7 @@ extension Configuration {
298298
errorWrite: errorWriteFileDescriptor
299299
)
300300
} catch {
301-
// If spawn() throws, monitorProcessTermination or runDetached
301+
// If spawn() throws, monitorProcessTermination
302302
// won't have an opportunity to call release, so do it here to avoid leaking the handles.
303303
execution.release()
304304
throw error

‎Tests/SubprocessTests/SubprocessTests+Unix.swift‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -776,25 +776,6 @@ extension SubprocessUnixTests {
776776

777777
// MARK: - Misc
778778
extension SubprocessUnixTests {
779-
@Test func testRunDetached() async throws {
780-
let (readFd, writeFd) = try FileDescriptor.pipe()
781-
let pid = try runDetached(
782-
.path("/bin/sh"),
783-
arguments: ["-c", "echo $$"],
784-
output: writeFd
785-
)
786-
var status: Int32 = 0
787-
waitpid(pid.value, &status, 0)
788-
#expect(_was_process_exited(status) > 0)
789-
try writeFd.close()
790-
let data = try await readFd.readUntilEOF(upToLength: 10)
791-
let resultPID = try #require(
792-
String(data: Data(data), encoding: .utf8)
793-
).trimmingCharacters(in: .whitespacesAndNewlines)
794-
#expect("\(pid.value)" == resultPID)
795-
try readFd.close()
796-
}
797-
798779
@Test func testTerminateProcess() async throws {
799780
let stuckResult = try await Subprocess.run(
800781
// This will intentionally hang

‎Tests/SubprocessTests/SubprocessTests+Windows.swift‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -688,45 +688,6 @@ extension SubprocessWindowsTests {
688688
}
689689
#expect(stuckProcess.terminationStatus.isSuccess)
690690
}
691-
692-
@Test func testRunDetached() async throws {
693-
let (readFd, writeFd) = try FileDescriptor.ssp_pipe()
694-
SetHandleInformation(
695-
readFd.platformDescriptor,
696-
DWORD(HANDLE_FLAG_INHERIT),
697-
0
698-
)
699-
let pid = try Subprocess.runDetached(
700-
.name("powershell.exe"),
701-
arguments: [
702-
"-Command", "Write-Host $PID",
703-
],
704-
output: writeFd
705-
)
706-
try writeFd.close()
707-
// Wait for process to finish
708-
guard
709-
let processHandle = OpenProcess(
710-
DWORD(PROCESS_QUERY_INFORMATION | SYNCHRONIZE),
711-
false,
712-
pid.value
713-
)
714-
else {
715-
Issue.record("Failed to get process handle")
716-
return
717-
}
718-
719-
// Wait for the process to finish
720-
WaitForSingleObject(processHandle, INFINITE)
721-
722-
// Up to 10 characters because Windows process IDs are DWORDs (UInt32), whose max value is 10 digits.
723-
let data = try await readFd.readUntilEOF(upToLength: 10)
724-
let resultPID = try #require(
725-
String(data: data, encoding: .utf8)
726-
).trimmingCharacters(in: .whitespacesAndNewlines)
727-
#expect("\(pid.value)" == resultPID)
728-
try readFd.close()
729-
}
730691
}
731692

732693
// MARK: - User Utils

0 commit comments

Comments
 (0)