-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
std: Don't spawn threads in
wait_with_output
Semantically there's actually no reason for us to spawn threads as part of the call to `wait_with_output`, and that's generally an incredibly heavyweight operation for just reading a few bytes (especially when stderr probably rarely has bytes!). An equivalent operation in terms of what's implemented today would be to just drain both pipes of all contents and then call `wait` on the child process itself. On Unix we can implement this through some convenient use of the `select` function, whereas on Windows we can make use of overlapped I/O. Note that on Windows this requires us to use named pipes instead of anonymous pipes, but they're semantically the same under the hood.
- Loading branch information
1 parent
6afa32a
commit 7c3038f
Showing
10 changed files
with
459 additions
and
34 deletions.
There are no files selected for viewing
Submodule liblibc
updated
16 files
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
7c3038f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: Sorry, I misunderstood... I thought the streams were being combined.
This is an incorrect implementation on Unix. You're supposed to create a single pipe pair and
dup
(A kernel syscall) the write end. Typically, you'll be callingdup2
to setup FDs 1 and 2, and that's howbash
does this for2>&1
, just callingdup2(1,2)
during the redirection phase.On Windows I'm unsure if this is correct or nice, do you do any handling of line buffering? Appending lines is supposed to be an atomic operation, so a partially read line from stdout *should not intermingle with data from stderr... and it looks like there would be a single buffer to hold two partial lines, not good. Though I haven't run Windows in over two decades so I don't really care if you fix that.