Skip to content

Conversation

owenv
Copy link
Contributor

@owenv owenv commented Sep 26, 2025

The AsyncProcess implementation on Windows was effectively buffering all output until process exit. Vendor over some utilities from Swift Build to stream the output, and attempt to avoid the bug addressed in #8047 (review)

@owenv
Copy link
Contributor Author

owenv commented Sep 26, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Sep 26, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Sep 26, 2025

@swift-ci test Windows

Copy link
Member

@dschaefer2 dschaefer2 left a comment

Choose a reason for hiding this comment

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

Yay! Yes, the only proper way to read stdout/stderr on Windows is multi-threaded. I do wonder if creating dedicated threads for this instead of using the Task thread pool would be safer.

@jakepetroules
Copy link
Contributor

This is fine, it's all non-blocking through Dispatch.

@owenv
Copy link
Contributor Author

owenv commented Sep 30, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Sep 30, 2025

@swift-ci test

@bripeticca
Copy link
Contributor

@swift-ci please test windows

@owenv owenv force-pushed the windows-streaming branch from c2dd0c0 to 3483f59 Compare October 1, 2025 04:57
@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test Windows

while !Task.isCancelled {
let chunk = try await readChunk(upToLength: 4096)
if chunk.isEmpty {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a continuation (e.g. cont.finish() here)? I'm not sure what the implications are of returning nil are for clients of this stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

returning nil here is intended to terminate the stream, this initializer doesn't use a continuation

@owenv owenv force-pushed the windows-streaming branch from 3483f59 to f153b91 Compare October 1, 2025 16:53
@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test Windows

@owenv owenv force-pushed the windows-streaming branch from f153b91 to 7c818d8 Compare October 1, 2025 20:39
@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Oct 1, 2025

@swift-ci test Windows

@ahoppen
Copy link
Member

ahoppen commented Oct 19, 2025

Just so you are aware, because we are using DispatchIO.read(offset:length:queue:), we are seeing one thread spinning on Windows in SourceKit-LSP (lengthy discussion here and here and here).

You are using read(fromFileDescriptor:maxLength:runningHandlerOn:handler:), which seems to take a different code path at first but I wouldn’t be surprised if it ends up with a very similar issue in the end. If you don’t, all the better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants