From 39a42aa28a076c146976adfaa00e229950138116 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Thu, 11 Jun 2026 12:49:33 -0700 Subject: [PATCH] Reject writes after StandardInputWriter finishes with a clear error Retaining the StandardInputWriter beyond the run body closure no longer hangs, but using the writer after that point failed with a cryptic 'Bad file descriptor', and nothing flagged the misuse. Track whether the writer has finished and throw a descriptive SubprocessError on a subsequent write, instead of writing to a closed and possibly recycled file descriptor. finish() is now explicitly idempotent. Document the writer's lifetime contract on StandardInputWriter and Execution.standardInputWriter: it is valid only within the body closure, run calls finish() automatically when the body returns, and awaiting the child's full output without finishing first can deadlock. --- Sources/Subprocess/Error.swift | 32 ++++++++++++- Sources/Subprocess/Execution.swift | 7 ++- Sources/Subprocess/IO/Input.swift | 49 ++++++++++++++++++-- Tests/SubprocessTests/IntegrationTests.swift | 37 +++++++++++++++ 4 files changed, 118 insertions(+), 7 deletions(-) diff --git a/Sources/Subprocess/Error.swift b/Sources/Subprocess/Error.swift index cd1d8e4d..31a3a9ed 100644 --- a/Sources/Subprocess/Error.swift +++ b/Sources/Subprocess/Error.swift @@ -172,6 +172,14 @@ extension SubprocessError: CustomStringConvertible, CustomDebugStringConvertible return "Failed to read bytes from the child process." } case .failedToWriteToSubprocess: + if let context = self.context[self.code], + case .string(let reason) = context + { + if let underlying = self.underlyingError { + return "Failed to write bytes to the child process: \(reason) Underlying error: \(underlying)" + } + return "Failed to write bytes to the child process: \(reason)" + } if let underlying = self.underlyingError { return "Failed to write bytes to the child process with underlying error: \(underlying)" } else { @@ -353,12 +361,32 @@ extension SubprocessError { } internal static func failedToWriteToProcess( - withUnderlyingError underlyingError: UnderlyingError? + withUnderlyingError underlyingError: UnderlyingError?, + reason: String? = nil ) -> Self { + var context: [SubprocessError.Code: SubprocessError.Context] = [:] + if let reason = reason { + context[.failedToWriteToSubprocess] = .string(reason) + } return SubprocessError( code: .failedToWriteToSubprocess, underlyingError: underlyingError, - context: [:] + context: context + ) + } + + /// The standard input writer was used after it finished. The writer is only + /// valid inside the `run(_:)` body closure; `run()` closes standard input + /// automatically when the body returns, so it must not be stored or used + /// afterward. + internal static var standardInputWriterFinished: Self { + return .failedToWriteToProcess( + withUnderlyingError: nil, + reason: """ + the standard input writer has already finished. The writer is only valid + inside the run(_:) body closure, which closes standard input automatically + when it returns; don't store the writer or use it after the closure returns. + """ ) } diff --git a/Sources/Subprocess/Execution.swift b/Sources/Subprocess/Execution.swift index e6682424..c12306ba 100644 --- a/Sources/Subprocess/Execution.swift +++ b/Sources/Subprocess/Execution.swift @@ -66,7 +66,12 @@ extension Execution where Input == CustomWriteInput { /// A writer that sends data to the subprocess's standard input. /// /// Call ``StandardInputWriter/finish()`` after the last write so the - /// subprocess sees end-of-file on its standard input. + /// subprocess sees end-of-file on its standard input. If you don't, `run` + /// finishes the writer for you when the body closure returns. + /// + /// Like the ``Execution`` value itself, the writer is valid only for the + /// duration of the body closure. Don't store it or use it after the closure + /// returns. See ``StandardInputWriter`` for details. public var standardInputWriter: StandardInputWriter { return self.inputWriter! } diff --git a/Sources/Subprocess/IO/Input.swift b/Sources/Subprocess/IO/Input.swift index 9d5f0e3b..d6d79381 100644 --- a/Sources/Subprocess/IO/Input.swift +++ b/Sources/Subprocess/IO/Input.swift @@ -251,11 +251,31 @@ extension InputProtocol { // MARK: - StandardInputWriter /// A writer that sends data to the standard input of a subprocess. +/// +/// You obtain a `StandardInputWriter` from ``Execution/standardInputWriter`` +/// inside the body closure of a `run` function, when the input is +/// ``InputProtocol/inputWriter``. Each `write` call sends its bytes to the +/// subprocess right away — there's no intermediate buffer and no separate flush +/// step. Call ``finish()`` after your final write so the subprocess sees +/// end-of-file on its standard input. +/// +/// - Important: The writer is valid only for the duration of the body closure. +/// When the closure returns, `run` automatically calls ``finish()`` to close +/// standard input. Don't store the writer or use it after the closure +/// returns: a write at that point throws a ``SubprocessError`` whose code is +/// ``SubprocessError/Code/failedToWriteToSubprocess``. +/// +/// - Important: If you await the subprocess's entire standard output from within +/// the body closure, call ``finish()`` *before* you await it. Many programs +/// don't finish producing output until they see end-of-file on standard +/// input, so waiting for all output without finishing first can deadlock. public final actor StandardInputWriter: Sendable { internal var diskIO: IODescriptor internal let processIdentifier: ProcessIdentifier + private var didFinish: Bool = false + init(diskIO: consuming IODescriptor, processIdentifier: ProcessIdentifier) { self.diskIO = diskIO self.processIdentifier = processIdentifier @@ -264,11 +284,15 @@ public final actor StandardInputWriter: Sendable { /// Writes an array of bytes to the subprocess's standard input. /// - Parameter array: The bytes to write. /// - Throws: `SubprocessError` with error code `.failedToWriteToSubprocess`. - /// See ``underlyingError`` for more details. + /// See ``underlyingError`` for more details. Also throws this error if + /// the writer has already finished (see ``finish()``). /// - Returns: The number of bytes written. public func write( _ array: [UInt8] ) async throws(SubprocessError) -> Int { + guard !self.didFinish else { + throw SubprocessError.standardInputWriterFinished + } return try await AsyncIO.shared.write(array, to: self.diskIO, for: self.processIdentifier) } @@ -276,9 +300,13 @@ public final actor StandardInputWriter: Sendable { /// /// - Parameter span: The span to write. /// - Throws: `SubprocessError` with error code `.failedToWriteToSubprocess`. - /// See ``underlyingError`` for more details. + /// See ``underlyingError`` for more details. Also throws this error if + /// the writer has already finished (see ``finish()``). /// - Returns: The number of bytes written. public func write(_ span: borrowing RawSpan) async throws(SubprocessError) -> Int { + guard !self.didFinish else { + throw SubprocessError.standardInputWriterFinished + } return try await AsyncIO.shared.write(span, to: self.diskIO, for: self.processIdentifier) } @@ -287,7 +315,8 @@ public final actor StandardInputWriter: Sendable { /// - string: The string to write. /// - encoding: The encoding to use when converting the string to bytes. /// - Throws: `SubprocessError` with error code `.failedToWriteToSubprocess`. - /// See ``underlyingError`` for more details. + /// See ``underlyingError`` for more details. Also throws this error if + /// the writer has already finished (see ``finish()``). /// - Returns: The number of bytes written. public func write( _ string: some StringProtocol, @@ -299,10 +328,22 @@ public final actor StandardInputWriter: Sendable { return 0 } - /// Signals that all writes are finished. + /// Signals that all writes are finished, closing standard input so the + /// subprocess sees end-of-file. + /// + /// `run` calls this automatically when the body closure returns, so you only + /// need to call it yourself when you want the subprocess to observe + /// end-of-file earlier. Calling `finish()` more than once is safe; subsequent calls do nothing. + /// After finishing, further writes throw a ``SubprocessError`` with the code + /// ``SubprocessError/Code/failedToWriteToSubprocess``. + /// /// - Throws: `SubprocessError` with error code `.asyncIOFailed`. /// See ``underlyingError`` for more details. public func finish() async throws(SubprocessError) { + guard !self.didFinish else { + return + } + self.didFinish = true try self.diskIO.safelyClose() } } diff --git a/Tests/SubprocessTests/IntegrationTests.swift b/Tests/SubprocessTests/IntegrationTests.swift index 0b180ffe..410e97e5 100644 --- a/Tests/SubprocessTests/IntegrationTests.swift +++ b/Tests/SubprocessTests/IntegrationTests.swift @@ -1067,6 +1067,43 @@ extension SubprocessIntegrationTests { } } +// MARK: - Standard Input Writer Lifetime Tests + +extension SubprocessIntegrationTests { + @Test func testStandardInputWriterRejectsWritesAfterFinish() async throws { + #if os(Windows) + let setup = TestSetup( + executable: .name("cmd.exe"), + arguments: ["/c", "findstr x*"] + ) + #else + let setup = TestSetup( + executable: .path("/bin/cat"), + arguments: [] + ) + #endif + let result = try await _run( + setup, + input: .inputWriter, + output: .string(limit: 1024), + error: .discarded + ) { execution in + let writer = execution.standardInputWriter + _ = try await writer.write("Hello, world!\n") + try await writer.finish() + // Writing after finish() must throw, not silently no-op or crash. + let afterFinishError = await #expect(throws: SubprocessError.self) { + _ = try await writer.write("after finish") + } + #expect(afterFinishError?.code == .failedToWriteToSubprocess) + // finish() is idempotent. + try await writer.finish() + } + #expect(result.terminationStatus.isSuccess) + #expect((result.standardOutput ?? "").contains("Hello, world!")) + } +} + // MARK: - Output Tests extension SubprocessIntegrationTests { @Test func testDiscardedOutput() async throws {