Skip to content

Commit c03685b

Browse files
committed
Fatal error if TrackedFileDescriptor and TrackedDispatchIO have not been closed in deinit.
We cannot depend on calling .close() in deinit since it’s non-failable. As a result, Subprocess needs to manually call .close() at the conclusion of the read/write operation. fatalError in deinit to ensures the FileDescriptor/DispatchIO will not leak.
1 parent 67e2f7d commit c03685b

File tree

2 files changed

+8
-23
lines changed

2 files changed

+8
-23
lines changed

Sources/Subprocess/Configuration.swift

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -632,26 +632,7 @@ internal struct TrackedFileDescriptor: ~Copyable {
632632
return
633633
}
634634

635-
do {
636-
try fileDescriptor.close()
637-
} catch {
638-
guard let errno: Errno = error as? Errno else {
639-
return
640-
}
641-
// Getting `.badFileDescriptor` suggests that the file descriptor
642-
// might have been closed unexpectedly. This can pose security risks
643-
// if another part of the code inadvertently reuses the same file descriptor
644-
// number. This problem is especially concerning on Unix systems due to POSIX’s
645-
// guarantee of using the lowest available file descriptor number, making reuse
646-
// more probable. We use `fatalError` upon receiving `.badFileDescriptor`
647-
// to prevent accidentally closing a different file descriptor.
648-
guard errno != .badFileDescriptor else {
649-
fatalError(
650-
"FileDescriptor \(fileDescriptor.rawValue) is already closed"
651-
)
652-
}
653-
// Otherwise ignore the error
654-
}
635+
fatalError("FileDescriptor \(self.fileDescriptor.rawValue) was not closed")
655636
}
656637

657638
internal func platformDescriptor() -> PlatformFileDescriptor {
@@ -695,7 +676,7 @@ internal struct TrackedDispatchIO: ~Copyable {
695676
return
696677
}
697678

698-
dispatchIO.close()
679+
fatalError("DispatchIO \(self.dispatchIO) was not closed")
699680
}
700681
}
701682
#endif

Sources/Subprocess/IO/Output.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,12 @@ public struct BytesOutput: OutputProtocol {
143143
from diskIO: consuming TrackedPlatformDiskIO?
144144
) async throws -> [UInt8] {
145145
#if os(Windows)
146-
return try await diskIO?.fileDescriptor.read(upToLength: self.maxSize) ?? []
146+
let result = try await diskIO?.fileDescriptor.read(upToLength: self.maxSize) ?? []
147+
try diskIO?.safelyClose()
148+
return result
147149
#else
148150
let result = try await diskIO!.dispatchIO.read(upToLength: self.maxSize)
151+
try diskIO?.safelyClose()
149152
return result?.array() ?? []
150153
#endif
151154
}
@@ -261,12 +264,13 @@ extension OutputProtocol {
261264
if OutputType.self == Void.self {
262265
return () as! OutputType
263266
}
264-
265267
#if os(Windows)
266268
let result = try await diskIO?.fileDescriptor.read(upToLength: self.maxSize)
269+
try diskIO?.safelyClose()
267270
return try self.output(from: result ?? [])
268271
#else
269272
let result = try await diskIO!.dispatchIO.read(upToLength: self.maxSize)
273+
try diskIO?.safelyClose()
270274
return try self.output(from: result ?? .empty)
271275
#endif
272276
}

0 commit comments

Comments
 (0)