Avoid deadlocking on log scanning with lots of output on stderr #5738
Conversation
This file contains hidden or 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
In some cases, we call `BufferedExec` and only want standard output; that is, we ignore standard error. In such a case, we can cause the data to be buffered forever and we'll block if there's enough of it, especially on Windows. Let's add a `StdoutBufferedExec` that redirects standard error to `/dev/null`, since that's effectively what we're doing already, except that our new function will be more robust because it won't cause indefinite blocking.
We already have some small wrappers for `BufferedExec`; let's add some for `StdoutBufferedExec` which we'll use in a future commit.
These are functions for which we don't care about standard error and which we don't want to block indefinitely.
When we process log output, we can sometimes get a large amount of data on standard error, which is buffered. If we wait until the end to process it, then the child process can block because it's trying to write to standard error but we're not reading from it, and in turn we can block because we're waiting for more standard output. The solution is simple: process standard error in another goroutine and send the result over a channel to be read at the end. This guarantees that we'll always process the data as expected and prevent deadlock.
chrisd8088
approved these changes
May 7, 2024
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
jochenhz
pushed a commit
to Anchorpoint-Software/git-lfs
that referenced
this pull request
May 17, 2024
Avoid deadlocking on log scanning with lots of output on stderr
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we're running our log scanning code, we can end up with a deadlock if there is lots of output on standard error because we either don't read standard error at all or we only read it when we're done reading standard output. To fix this, let's add and use some functions that either discard standard error if we're not using it, as well as making sure we read standard error in a separate goroutine so the child process doesn't block.
Fixes #5720