Skip to content

Fatal error if TrackedFileDescriptor and TrackedDispatchIO have not been closed in deinit #72

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 12, 2025
Merged

Fatal error if TrackedFileDescriptor and TrackedDispatchIO have not been closed in deinit #72

Show file tree
Hide file tree
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
23 changes: 2 additions & 21 deletions Sources/Subprocess/Configuration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,26 +632,7 @@ internal struct TrackedFileDescriptor: ~Copyable {
return
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's still this guard self.closeWhenDone here which is probably not intended as we're never closing here (which is great)

do {
try fileDescriptor.close()
} catch {
guard let errno: Errno = error as? Errno else {
return
}
// Getting `.badFileDescriptor` suggests that the file descriptor
// might have been closed unexpectedly. This can pose security risks
// if another part of the code inadvertently reuses the same file descriptor
// number. This problem is especially concerning on Unix systems due to POSIX’s
// guarantee of using the lowest available file descriptor number, making reuse
// more probable. We use `fatalError` upon receiving `.badFileDescriptor`
// to prevent accidentally closing a different file descriptor.
guard errno != .badFileDescriptor else {
fatalError(
"FileDescriptor \(fileDescriptor.rawValue) is already closed"
)
}
// Otherwise ignore the error
}
fatalError("FileDescriptor \(self.fileDescriptor.rawValue) was not closed")
}

internal func platformDescriptor() -> PlatformFileDescriptor {
Expand Down Expand Up @@ -695,7 +676,7 @@ internal struct TrackedDispatchIO: ~Copyable {
return
}

dispatchIO.close()
fatalError("DispatchIO \(self.dispatchIO) was not closed")
}
}
#endif
Expand Down
8 changes: 6 additions & 2 deletions Sources/Subprocess/IO/Output.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,12 @@ public struct BytesOutput: OutputProtocol {
from diskIO: consuming TrackedPlatformDiskIO?
) async throws -> [UInt8] {
#if os(Windows)
return try await diskIO?.fileDescriptor.read(upToLength: self.maxSize) ?? []
let result = try await diskIO?.fileDescriptor.read(upToLength: self.maxSize) ?? []
try diskIO?.safelyClose()
return result
#else
let result = try await diskIO!.dispatchIO.read(upToLength: self.maxSize)
try diskIO?.safelyClose()
return result?.array() ?? []
#endif
}
Expand Down Expand Up @@ -261,12 +264,13 @@ extension OutputProtocol {
if OutputType.self == Void.self {
return () as! OutputType
}

#if os(Windows)
let result = try await diskIO?.fileDescriptor.read(upToLength: self.maxSize)
try diskIO?.safelyClose()
return try self.output(from: result ?? [])
#else
let result = try await diskIO!.dispatchIO.read(upToLength: self.maxSize)
try diskIO?.safelyClose()
return try self.output(from: result ?? .empty)
#endif
}
Expand Down
Loading