Skip to content

Commit 4f71108

Browse files
macmadeclaude
andcommitted
fix: Read child-process output to EOF to prevent truncation and hangs
The previous implementation wrote the entire stdin payload synchronously before any stdout was drained, then relied on NSFileHandleDataAvailable notifications to collect output. For inputs larger than the 64 KB pipe buffer the child blocked writing stdout, stopped reading stdin, and the parent's blocking write deadlocked before waitUntilExit() was reached. Drain stdout and stderr to EOF on background queues started before the stdin write, join them after the process exits, then publish the buffers. This collects output in full and removes the deadlock. Dropping the notification observers also removes the unbalanced-observer teardown, so no deinit is required. Add a testable run(executableURL:) entry point and TaskTests covering a >64 KB round-trip, stderr capture, and non-zero exit status. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 66a528e commit 4f71108

3 files changed

Lines changed: 104 additions & 36 deletions

File tree

Shared/Task.swift

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,18 @@ public class Task
4242
return nil
4343
}
4444

45-
guard FileManager.default.fileExists( atPath: executable.path )
45+
return self.run( executableURL: executable, arguments: arguments, input: input )
46+
}
47+
48+
public class func run( executableURL: URL, arguments: [ String ], input: Data? ) -> Task?
49+
{
50+
guard FileManager.default.fileExists( atPath: executableURL.path )
4651
else
4752
{
4853
return nil
4954
}
5055

51-
let task = Task( executable: executable, arguments: arguments )
56+
let task = Task( executable: executableURL, arguments: arguments )
5257

5358
task.run( input: input )
5459

@@ -68,12 +73,6 @@ public class Task
6873

6974
self.standardOutput = Data()
7075
self.standardError = Data()
71-
72-
NotificationCenter.default.addObserver( self, selector: #selector( self.dataAvailableForStandardOutput( _: ) ), name: NSNotification.Name.NSFileHandleDataAvailable, object: self.pipeOut.fileHandleForReading )
73-
NotificationCenter.default.addObserver( self, selector: #selector( self.dataAvailableForStandardError( _: ) ), name: NSNotification.Name.NSFileHandleDataAvailable, object: self.pipeErr.fileHandleForReading )
74-
75-
self.pipeOut.fileHandleForReading.waitForDataInBackgroundAndNotify()
76-
self.pipeErr.fileHandleForReading.waitForDataInBackgroundAndNotify()
7776
}
7877

7978
public func run( input: Data? )
@@ -85,6 +84,24 @@ public class Task
8584

8685
self.task.launch()
8786

87+
// Drain both output pipes to EOF on background queues, started before we
88+
// write stdin and before we block in waitUntilExit(). This guarantees the
89+
// child can never deadlock by filling a pipe buffer we aren't reading, and
90+
// that the full output is collected rather than truncated.
91+
let group = DispatchGroup()
92+
var outData = Data()
93+
var errData = Data()
94+
95+
DispatchQueue.global( qos: .userInitiated ).async( group: group )
96+
{
97+
outData = self.pipeOut.fileHandleForReading.readDataToEndOfFile()
98+
}
99+
100+
DispatchQueue.global( qos: .userInitiated ).async( group: group )
101+
{
102+
errData = self.pipeErr.fileHandleForReading.readDataToEndOfFile()
103+
}
104+
88105
if let input = input, let pipe = self.task.standardInput as? Pipe
89106
{
90107
let handle = pipe.fileHandleForWriting
@@ -95,6 +112,12 @@ public class Task
95112

96113
self.task.waitUntilExit()
97114

115+
// The child has exited and closed its pipe ends, so both reads have hit
116+
// (or are about to hit) EOF. Join them before publishing the results.
117+
group.wait()
118+
119+
self.standardOutput = outData
120+
self.standardError = errData
98121
self.terminationStatus = self.task.terminationStatus
99122

100123
#if DEBUG
@@ -104,32 +127,4 @@ public class Task
104127
}
105128
#endif
106129
}
107-
108-
@objc
109-
private func dataAvailableForStandardOutput( _ notification: Notification )
110-
{
111-
guard let handle = notification.object as? FileHandle?,
112-
let data = handle?.availableData
113-
else
114-
{
115-
return
116-
}
117-
118-
self.standardOutput.append( data )
119-
handle?.waitForDataInBackgroundAndNotify()
120-
}
121-
122-
@objc
123-
private func dataAvailableForStandardError( _ notification: Notification )
124-
{
125-
guard let handle = notification.object as? FileHandle?,
126-
let data = handle?.availableData
127-
else
128-
{
129-
return
130-
}
131-
132-
self.standardError.append( data )
133-
handle?.waitForDataInBackgroundAndNotify()
134-
}
135130
}

SharedTests/TaskTests.swift

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*******************************************************************************
2+
* The MIT License (MIT)
3+
*
4+
* Copyright (c) 2022, Jean-David Gadina - www.xs-labs.com
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the Software), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
******************************************************************************/
24+
25+
import Foundation
26+
import Testing
27+
28+
@Suite( "Task — child-process I/O" )
29+
struct TaskTests
30+
{
31+
// A payload several times larger than the 64 KB kernel pipe buffer. If the
32+
// output is not drained while the child writes, the child blocks on write
33+
// and the parent deadlocks in waitUntilExit(); if reads are torn down too
34+
// early, the tail is truncated. Either way this round-trip fails.
35+
private static let largePayload = Data( ( 0 ..< ( 256 * 1024 ) ).map { UInt8( $0 & 0xFF ) } )
36+
37+
@Test( "Collects complete stdout for input larger than the pipe buffer", .timeLimit( .minutes( 1 ) ) )
38+
func largeInputRoundTrip() throws
39+
{
40+
let task = try #require( Task.run( executableURL: URL( fileURLWithPath: "/bin/cat" ), arguments: [], input: Self.largePayload ) )
41+
42+
#expect( task.terminationStatus == 0 )
43+
#expect( task.standardOutput.count == Self.largePayload.count )
44+
#expect( task.standardOutput == Self.largePayload )
45+
}
46+
47+
@Test( "Captures standard error", .timeLimit( .minutes( 1 ) ) )
48+
func capturesStandardError() throws
49+
{
50+
let task = try #require( Task.run( executableURL: URL( fileURLWithPath: "/bin/sh" ), arguments: [ "-c", "printf 'boom' 1>&2" ], input: nil ) )
51+
52+
#expect( task.terminationStatus == 0 )
53+
#expect( String( data: task.standardError, encoding: .utf8 ) == "boom" )
54+
}
55+
56+
@Test( "Reports a non-zero termination status", .timeLimit( .minutes( 1 ) ) )
57+
func nonZeroTerminationStatus() throws
58+
{
59+
let task = try #require( Task.run( executableURL: URL( fileURLWithPath: "/bin/sh" ), arguments: [ "-c", "exit 3" ], input: nil ) )
60+
61+
#expect( task.terminationStatus == 3 )
62+
}
63+
64+
@Test( "Returns nil for a non-existent executable" )
65+
func missingExecutable()
66+
{
67+
#expect( Task.run( executableURL: URL( fileURLWithPath: "/nonexistent/executable" ), arguments: [], input: nil ) == nil )
68+
}
69+
}

XcodeFormat.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
415B69E897EF098F1D497C6B /* String.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05744ED528EC2EDB00A88503 /* String.swift */; };
6464
422C428BA7CBC2B94FD892A0 /* Task.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05BED21928EC4A2A008039F6 /* Task.swift */; };
6565
52C07AA4ACA79DEA083917ED /* URL.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05744ECD28EC2E5000A88503 /* URL.swift */; };
66+
54AF1A10772285FD73793431 /* TaskTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 913F32B63DB056684754D787 /* TaskTests.swift */; };
6667
6AF1DB0943F9AA065EA80860 /* URLTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B907F7DC445EA71845A03EE9 /* URLTests.swift */; };
6768
7CA215582B396CDC75D0E7A5 /* Data.swift in Sources */ = {isa = PBXBuildFile; fileRef = 05744ED228EC2ED400A88503 /* Data.swift */; };
6869
EF9CF118DCDD7951D6C90043 /* Configuration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 050E0F6128EB66390080C562 /* Configuration.swift */; };
@@ -241,6 +242,7 @@
241242
2B7221495008B0E46F3003F1 /* SharedTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = SharedTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
242243
754426165B1A6F7B9A8AA57D /* Cocoa.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Cocoa.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.0.sdk/System/Library/Frameworks/Cocoa.framework; sourceTree = DEVELOPER_DIR; };
243244
78DA3F1F404A081C0DE85CA4 /* ConfigurationTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ConfigurationTests.swift; sourceTree = "<group>"; };
245+
913F32B63DB056684754D787 /* TaskTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TaskTests.swift; sourceTree = "<group>"; };
244246
B907F7DC445EA71845A03EE9 /* URLTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = URLTests.swift; sourceTree = "<group>"; };
245247
/* End PBXFileReference section */
246248

@@ -571,6 +573,7 @@
571573
children = (
572574
B907F7DC445EA71845A03EE9 /* URLTests.swift */,
573575
78DA3F1F404A081C0DE85CA4 /* ConfigurationTests.swift */,
576+
913F32B63DB056684754D787 /* TaskTests.swift */,
574577
);
575578
name = SharedTests;
576579
path = SharedTests;
@@ -812,6 +815,7 @@
812815
415B69E897EF098F1D497C6B /* String.swift in Sources */,
813816
52C07AA4ACA79DEA083917ED /* URL.swift in Sources */,
814817
422C428BA7CBC2B94FD892A0 /* Task.swift in Sources */,
818+
54AF1A10772285FD73793431 /* TaskTests.swift in Sources */,
815819
);
816820
runOnlyForDeploymentPostprocessing = 0;
817821
};

0 commit comments

Comments
 (0)