Skip to content

Fix Windows IPC race condition when using parallel checking#21228

Merged
JukkaL merged 5 commits intomasterfrom
windows-ipc-fix
Apr 14, 2026
Merged

Fix Windows IPC race condition when using parallel checking#21228
JukkaL merged 5 commits intomasterfrom
windows-ipc-fix

Conversation

@JukkaL
Copy link
Copy Markdown
Collaborator

@JukkaL JukkaL commented Apr 14, 2026

Cancellation is asynchronous, so some data could be read after calling cancel(). Rework ready_to_read to 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.

@github-actions
Copy link
Copy Markdown
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Copy Markdown
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@JukkaL JukkaL merged commit 19cf4d9 into master Apr 14, 2026
24 checks passed
@JukkaL JukkaL deleted the windows-ipc-fix branch April 14, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants