Improvements to robustness of piped process support on Windows #6675
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
To reduce the likelihood of exceeding its capacity.
To give the sub process more time to read from the pipe.
thomasspriggs
force-pushed
the
tas/windows_pipe_robustness
branch
from
364068d to
5aed74f
Compare
7 tasks
tautschnig
approved these changes
Feb 16, 2022
So that the changes in the following commit do not get mixed in with clang-format related include re-ordering.
Codecov Report
@@ Coverage Diff @@
## develop #6675 +/- ##
========================================
Coverage 76.81% 76.81%
========================================
Files 1582 1582
Lines 182774 182764 -10
========================================
- Hits 140403 140399 -4
+ Misses 42371 42365 -6
Continue to review full report at Codecov.
|
thomasspriggs
force-pushed
the
tas/windows_pipe_robustness
branch
from
5aed74f to
b6f9b52
Compare
thomasspriggs
requested review from
peterschrammel,
NlightNFotis,
TGWDB,
chris-ryder and
kroening
as code owners
thomasspriggs
force-pushed
the
tas/windows_pipe_robustness
branch
from
b6f9b52 to
aff1f8c
Compare
peterschrammel
approved these changes
Feb 16, 2022
src/util/piped_process.cpp
Outdated
| @@ -372,6 +372,7 @@ piped_processt::send_responset piped_processt::send(const std::string &message) | |||
| break; | |||
| // Give the sub-process chance to read the waiting message(s). | |||
| FlushFileBuffers(child_std_IN_Wr); | |||
| Sleep(1 << send_attempts); | |||
There was a problem hiding this comment.
What's the unit of that?
There was a problem hiding this comment.
Milliseconds.
NlightNFotis
approved these changes
Feb 16, 2022
TGWDB
approved these changes
Feb 16, 2022
So that if it fails again we have more information in the CI log.
thomasspriggs
force-pushed
the
tas/windows_pipe_robustness
branch
from
aff1f8c to
52d5788
Compare
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.
Intermittent failures have been observed for the
bitvector-arithmetic-operators/simple_equation.desctest when running in CI for the Windows job. See https://github.com/diffblue/cbmc/runs/5202299979?check_suite_focus=true or https://github.com/diffblue/cbmc/runs/5201922925?check_suite_focus=true for examples.This PR contains a collection of potential improvements to the robustness of piped process support on Windows. These should collectively reduce the likelihood of the issue occurring again and help with debugging if it does. I am happy to discuss the benefits of spitting these out to try them individually and work out which are required or just merging all of them in order to get the issue resolved ASAP.