Ensure test library issues json string line-by-line#109729
Merged
bors merged 1 commit intorust-lang:masterfrom May 3, 2023
Merged
Ensure test library issues json string line-by-line#109729bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
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.
#108659 introduces a custom test display implementation. It does so by using libtest to output json. The stdout is read line by line and parsed. The code trims the line read and checks whether it starts with a
{and ends with a}.Unfortunately, there is a race condition in how json data is written to stdout. The
write_messagefunction callsself.out.write_allrepeatedly to write a buffer that contains (partial) json data, or a new line. There is no lock around theself.out.write_allfunctions. Similarly, thewrite_messagefunction itself is called with only partial json data. As these functions are called from concurrent threads, this may result in json data ending up on the same stdout line. This PR avoids this by buffering the complete json data before issuing a singleself.out.write_all.(#109484 implemented a partial fix for this issue; it only avoids that failed json parsing would result in a panic.)
cc: @jethrogb, @pietroalbini