log: add a mutex for stderr redirection #49936
Merged
Conversation
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 change is |
The `TestLogScope` infrastructure is meant to force-configure logging to go to a temporary directory. It was designed with the assumption it would be used "outside" of server initialization, i.e. non-concurrently with `log` API calls. However, at least one test (`TestHBAAuthenticationRules`) violates the assumption and switches the output directory while a server is running. After checking it appears that the behavior is sound, but there was one racy access remaining: the write to `os.Stderr` during the redirect. This had been racy ever since TestLogScope was introduced; however it was only visible on windows because the unix builds were missing the assignment to `os.Stderr`. A recent fix to make the builds consistent revealed the race to our CI checker. This patch fixes it by adding the missing mutex. Release note: None
tbg
approved these changes
Jun 6, 2020
|
thx bors r=tbg |
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #49930
The
TestLogScopeinfrastructure is meant to force-configure loggingto go to a temporary directory. It was designed with the assumption it
would be used "outside" of server initialization, i.e.
non-concurrently with
logAPI calls.However, at least one test (
TestHBAAuthenticationRules) violates theassumption and switches the output directory while a server is running.
After checking it appears that the behavior is sound, but there was
one racy access remaining: the write to
os.Stderrduring theredirect.
This had been racy ever since TestLogScope was introduced; however
it was only visible on windows because the unix builds were missing
the assignment to
os.Stderr. A recent fix to make the buildsconsistent revealed the race to our CI checker.
This patch fixes it by adding the missing mutex.
Release note: None