Fix Windows IPC race condition when using parallel checking#21228
Fix Windows IPC race condition when using parallel checking#21228
Conversation
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks! Yeah, let's try this.
cc @emmatyping (just FYI, feel free to ignore if you don't have time)
| # simply skipped. This avoids a race between checking if an operation | ||
| # is signaled and cancelling it. | ||
| for _, ov in pending: | ||
| ov.cancel() |
There was a problem hiding this comment.
TBH I don't know if this can raise itself. Also maybe we can combine two loops, i.e.:
for i, ov in pending:
try:
ov.cancel()
_, err = ov.GetOverlappedResult(True)
except OSError as e:
...
...(but to be clear I know very little about Windows)
There was a problem hiding this comment.
Claude Code was confident that this won't raise, but Codex warned about this potentially raising. I could look into the Python stdlib to see if they expect exceptions in similar calls.
There was a problem hiding this comment.
CPython has two loops for a similar use case, and it also appears to assume that cancel() doesn't raise in a similar use case.
Cancellation is asynchronous, so some data could be read after calling
cancel(). Reworkready_to_readto first cancel all operations and then check for each if data is available.See discussion in #21221 for more context.
I heavily relied on coding agent assist for this, but I did multiple review iterations and refinements.