Skip to content

Fix a crash due to improper error handling on Windows #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 30, 2025
Merged

Fix a crash due to improper error handling on Windows #103

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 97 additions & 54 deletions Sources/Subprocess/Platforms/Subprocess+Windows.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,40 @@ extension Configuration {
outputPipe: consuming CreatedPipe,
errorPipe: consuming CreatedPipe
) throws -> SpawnResult {
var inputPipeBox: CreatedPipe? = consume inputPipe
var outputPipeBox: CreatedPipe? = consume outputPipe
var errorPipeBox: CreatedPipe? = consume errorPipe

var _inputPipe = inputPipeBox.take()!
var _outputPipe = outputPipeBox.take()!
var _errorPipe = errorPipeBox.take()!

let inputReadFileDescriptor: TrackedFileDescriptor? = _inputPipe.readFileDescriptor()
let inputWriteFileDescriptor: TrackedFileDescriptor? = _inputPipe.writeFileDescriptor()
let outputReadFileDescriptor: TrackedFileDescriptor? = _outputPipe.readFileDescriptor()
let outputWriteFileDescriptor: TrackedFileDescriptor? = _outputPipe.writeFileDescriptor()
let errorReadFileDescriptor: TrackedFileDescriptor? = _errorPipe.readFileDescriptor()
let errorWriteFileDescriptor: TrackedFileDescriptor? = _errorPipe.writeFileDescriptor()

let (
applicationName,
commandAndArgs,
environment,
intendedWorkingDir
) = try self.preSpawn()

var inputReadFileDescriptor: TrackedFileDescriptor? = inputPipe.readFileDescriptor()
var inputWriteFileDescriptor: TrackedFileDescriptor? = inputPipe.writeFileDescriptor()
var outputReadFileDescriptor: TrackedFileDescriptor? = outputPipe.readFileDescriptor()
var outputWriteFileDescriptor: TrackedFileDescriptor? = outputPipe.writeFileDescriptor()
var errorReadFileDescriptor: TrackedFileDescriptor? = errorPipe.readFileDescriptor()
var errorWriteFileDescriptor: TrackedFileDescriptor? = errorPipe.writeFileDescriptor()
): (String?, String, String, String?)
do {
(applicationName, commandAndArgs, environment, intendedWorkingDir) = try self.preSpawn()
} catch {
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor,
inputWrite: inputWriteFileDescriptor,
outputRead: outputReadFileDescriptor,
outputWrite: outputWriteFileDescriptor,
errorRead: errorReadFileDescriptor,
errorWrite: errorWriteFileDescriptor
)
throw error
}

var startupInfo = try self.generateStartupInfo(
withInputRead: inputReadFileDescriptor,
Expand All @@ -77,15 +98,15 @@ extension Configuration {
try configurator(&createProcessFlags, &startupInfo)
}
// Spawn!
try applicationName.withOptionalNTPathRepresentation { applicationNameW in
let created = try applicationName.withOptionalNTPathRepresentation { applicationNameW in
try commandAndArgs.withCString(
encodedAs: UTF16.self
) { commandAndArgsW in
try environment.withCString(
encodedAs: UTF16.self
) { environmentW in
try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW in
let created = CreateProcessW(
CreateProcessW(
applicationNameW,
UnsafeMutablePointer<WCHAR>(mutating: commandAndArgsW),
nil, // lpProcessAttributes
Expand All @@ -97,26 +118,27 @@ extension Configuration {
&startupInfo,
&processInfo
)
guard created else {
let windowsError = GetLastError()
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor.take(),
inputWrite: inputWriteFileDescriptor.take(),
outputRead: outputReadFileDescriptor.take(),
outputWrite: outputWriteFileDescriptor.take(),
errorRead: errorReadFileDescriptor.take(),
errorWrite: errorWriteFileDescriptor.take()
)
throw SubprocessError(
code: .init(.spawnFailed),
underlyingError: .init(rawValue: windowsError)
)
}
}
}
}
}

guard created else {
let windowsError = GetLastError()
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor,
inputWrite: inputWriteFileDescriptor,
outputRead: outputReadFileDescriptor,
outputWrite: outputWriteFileDescriptor,
errorRead: errorReadFileDescriptor,
errorWrite: errorWriteFileDescriptor
)
throw SubprocessError(
code: .init(.spawnFailed),
underlyingError: .init(rawValue: windowsError)
)
}

let pid = ProcessIdentifier(
value: processInfo.dwProcessId
)
Expand Down Expand Up @@ -157,19 +179,40 @@ extension Configuration {
errorPipe: consuming CreatedPipe,
userCredentials: PlatformOptions.UserCredentials
) throws -> SpawnResult {
var inputPipeBox: CreatedPipe? = consume inputPipe
var outputPipeBox: CreatedPipe? = consume outputPipe
var errorPipeBox: CreatedPipe? = consume errorPipe

var _inputPipe = inputPipeBox.take()!
var _outputPipe = outputPipeBox.take()!
var _errorPipe = errorPipeBox.take()!

let inputReadFileDescriptor: TrackedFileDescriptor? = _inputPipe.readFileDescriptor()
let inputWriteFileDescriptor: TrackedFileDescriptor? = _inputPipe.writeFileDescriptor()
let outputReadFileDescriptor: TrackedFileDescriptor? = _outputPipe.readFileDescriptor()
let outputWriteFileDescriptor: TrackedFileDescriptor? = _outputPipe.writeFileDescriptor()
let errorReadFileDescriptor: TrackedFileDescriptor? = _errorPipe.readFileDescriptor()
let errorWriteFileDescriptor: TrackedFileDescriptor? = _errorPipe.writeFileDescriptor()

let (
applicationName,
commandAndArgs,
environment,
intendedWorkingDir
) = try self.preSpawn()

var inputReadFileDescriptor: TrackedFileDescriptor? = inputPipe.readFileDescriptor()
var inputWriteFileDescriptor: TrackedFileDescriptor? = inputPipe.writeFileDescriptor()
var outputReadFileDescriptor: TrackedFileDescriptor? = outputPipe.readFileDescriptor()
var outputWriteFileDescriptor: TrackedFileDescriptor? = outputPipe.writeFileDescriptor()
var errorReadFileDescriptor: TrackedFileDescriptor? = errorPipe.readFileDescriptor()
var errorWriteFileDescriptor: TrackedFileDescriptor? = errorPipe.writeFileDescriptor()
): (String?, String, String, String?)
do {
(applicationName, commandAndArgs, environment, intendedWorkingDir) = try self.preSpawn()
} catch {
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor,
inputWrite: inputWriteFileDescriptor,
outputRead: outputReadFileDescriptor,
outputWrite: outputWriteFileDescriptor,
errorRead: errorReadFileDescriptor,
errorWrite: errorWriteFileDescriptor
)
throw error
}

var startupInfo = try self.generateStartupInfo(
withInputRead: inputReadFileDescriptor,
Expand All @@ -186,7 +229,7 @@ extension Configuration {
try configurator(&createProcessFlags, &startupInfo)
}
// Spawn (featuring pyramid!)
try userCredentials.username.withCString(
let created = try userCredentials.username.withCString(
encodedAs: UTF16.self
) { usernameW in
try userCredentials.password.withCString(
Expand All @@ -203,7 +246,7 @@ extension Configuration {
encodedAs: UTF16.self
) { environmentW in
try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW in
let created = CreateProcessWithLogonW(
CreateProcessWithLogonW(
usernameW,
domainW,
passwordW,
Expand All @@ -216,21 +259,6 @@ extension Configuration {
&startupInfo,
&processInfo
)
guard created else {
let windowsError = GetLastError()
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor.take(),
inputWrite: inputWriteFileDescriptor.take(),
outputRead: outputReadFileDescriptor.take(),
outputWrite: outputWriteFileDescriptor.take(),
errorRead: errorReadFileDescriptor.take(),
errorWrite: errorWriteFileDescriptor.take()
)
throw SubprocessError(
code: .init(.spawnFailed),
underlyingError: .init(rawValue: windowsError)
)
}
}
}
}
Expand All @@ -239,6 +267,22 @@ extension Configuration {
}
}

guard created else {
let windowsError = GetLastError()
try self.safelyCloseMultiple(
inputRead: inputReadFileDescriptor,
inputWrite: inputWriteFileDescriptor,
outputRead: outputReadFileDescriptor,
outputWrite: outputWriteFileDescriptor,
errorRead: errorReadFileDescriptor,
errorWrite: errorWriteFileDescriptor
)
throw SubprocessError(
code: .init(.spawnFailed),
underlyingError: .init(rawValue: windowsError)
)
}

let pid = ProcessIdentifier(
value: processInfo.dwProcessId
)
Expand Down Expand Up @@ -1038,10 +1082,9 @@ extension FileDescriptor {

extension TrackedFileDescriptor {
internal consuming func createPlatformDiskIO() -> TrackedPlatformDiskIO {
return .init(
self.fileDescriptor,
closeWhenDone: self.closeWhenDone
)
// TrackedPlatformDiskIO is a typealias of TrackedFileDescriptor on Windows (they're the same type)
// Just return the same object so we don't create a copy and try to double-close the fd.
return self
}

internal func readUntilEOF(
Expand Down
Loading