[windows][test] Ensure the ordering of stdout and stderr messages. #26317
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
|
@swift-ci please test Windows platform |
|
@swift-ci please smoke test |
|
Sigh. It was expected. The other test that's flaky in Windows. @swift-ci please test Windows platform |
|
@swift-ci please test Windows platform |
|
Funny thing. Seems that windows-2019-2 machine in CI didn't refreshed the environment variables and was never going to pass that test. @swift-ci please test Windows platform |
jrose-apple
reviewed
Jul 30, 2019
It is possible that the rest of the platforms are relying in some not clearly documented behaviour that the stdout is flushed before stderr can be used, or something similar. Windows didn't seem to like that, and was sometimes outputting the stderr messages interlaced with the stdout messages (specially in the Azure testing for VS2017). The initial solution was adding -DAG to some CHECK lines, but that doesn't cover all the possibilities, and wasn't enabled for all the checks. I tried to fix it in several ways, but none of them were perfect, and many of them were deadlocking. There's a fundamental difference between others and Windows and that is that for others stdout seems to have a little bit of an edge in being treated first. I tried to get the Windows code closer to that idea, but I have no luck. Some of the approaches were using the main thread to read from the threads reading stdout and stderr; using only the main thread and IOCP on the pipes reading from the child process; and flushing after every output to stdout. None of them were perfect. The final solution is a hack, but it seems to not fail when I run the test repeatedly in my machine, while other approaches were failing at least once before I discarded them. The solution is including a small sleep (1 millisecond) in the Windows code. This should yield the execution time slice to other thread/process, which seems to do the trick and keep the stdout before the stderr. Hopefully this fixes the errors in Azure, and doesn't affect the rest of the testing machines. Being a really small sleep also should not affect the duration of the test itself.
drodriguez
force-pushed
the
windows-hack-for-crashing-tests
branch
from
ce3cc81 to
8bea502
Compare
|
@swift-ci please test Windows platform |
|
@swift-ci please smoke test |
|
@compnerd: do we still want this? It will solve one of the problems in https://dev.azure.com/compnerd/windows-swift/_build/results?buildId=5744&view=results |
|
This is horrible, but, if it makes the 2017 tests reliable, lets do it. |
compnerd
approved these changes
Aug 23, 2019
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.
It is possible that the rest of the platforms are relying in some not
clearly documented behaviour that the stdout is flushed before stderr
can be used, or something similar. Windows didn't seem to like that, and
was sometimes outputting the stderr messages interlaced with the stdout
messages (specially in the Azure testing for VS2017). The initial
solution was adding -DAG to some CHECK lines, but that doesn't cover all
the possibilities, and wasn't enabled for all the checks.
I tried to fix it in several ways, but none of them were perfect, and
many of them were deadlocking. There's a fundamental difference between
others and Windows and that is that for others stdout seems to have a
little bit of an edge in being treated first. I tried to get the Windows
code closer to that idea, but I have no luck. Some of the approaches
were using the main thread to read from the threads reading stdout and
stderr; using only the main thread and IOCP on the pipes reading from
the child process; and flushing after every output to stdout. None of
them were perfect.
The final solution is a hack, but it seems to not fail when I run the
test repeatedly in my machine, while other approaches were failing at
least once before I discarded them. The solution is including a small
sleep (1 millisecond) in the Windows code. This should yield the
execution time slice to other thread/process, which seems to do the
trick and keep the stdout before the stderr.
Hopefully this fixes the errors in Azure, and doesn't affect the rest of
the testing machines. Being a really small sleep also should not affect
the duration of the test itself.