Recognize ERROR_INVALID_HANDLE as indicating the IOCP was closed #86
Conversation
There's a race condition when we signal the IOCP thread to exit by closing the IOCP: the thread might be blocked in GetQueuedCompletionStatusEx, or it might (rarely) be in the rest of the loop. The effect of this is that there are two possible error messages we might get that both indicate that the IOCP was closed. Before we were only detecting one of them; now we detect both. This has the side-effect of making it so that during the test suite we deterministically only take the regular-exit path, rather than sometimes taking the blow-up-on-unrecognized-error path. Which is good because codecov was getting cranky about that non-deterministic coverage and yelling at people for no reason (see python-triogh-81).
|
@brettcannon: hopefully this should fix the codecov thing - any interest in reviewing? |
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
- Coverage 98.29% 98.24% -0.06%
==========================================
Files 50 50
Lines 5876 5878 +2
Branches 464 464
==========================================
- Hits 5776 5775 -1
- Misses 86 88 +2
- Partials 14 15 +1
Continue to review full report at Codecov.
|
| # signal that it should shut down, the main thread just closes the | ||
| # IOCP, which causes GetQueuedCompletionStatusEx to return with an | ||
| # error: | ||
| IOCP_CLOSED_ERRORS = { |
There was a problem hiding this comment.
Any reason this constant is a local and not a global?
There was a problem hiding this comment.
Eh, could go either way, but I feel like the category being defined here is more specific to the semantics of this particular function than something that's generally meaningful.
Then you're in the same boat as me :-) |
|
@brettcannon: btw, want to be added as a collaborator? (Can you tell I'm eager to get more folks involved 😄) |
|
Sure, but I make no promises I will ever use such powers. 😁 |
|
That's fair :-). You should have an invite. |
There's a race condition when we signal the IOCP thread to exit by
closing the IOCP: the thread might be blocked in
GetQueuedCompletionStatusEx, or it might (rarely) be in the rest of
the loop. The effect of this is that there are two possible error
messages we might get that both indicate that the IOCP was
closed. Before we were only detecting one of them; now we detect
both.
This has the side-effect of making it so that during the test suite we
deterministically only take the regular-exit path, rather than
sometimes taking the blow-up-on-unrecognized-error path. Which is
good because codecov was getting cranky about that non-deterministic
coverage and yelling at people for no reason (see gh-81).