Skip to content

gh-135099: Only wait on _PyOS_SigintEvent() in main thread #135100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Jun 3, 2025

On Windows, the _PyOS_SigintEvent() event handle is used to interrupt the main thread when Ctrl-C is pressed. Previously, we also waited on the event from other threads, but ignored the result. However, this can race with interpreter shutdown because the main thread closes the handle in _PySignal_Fini and threads may still be running and using mutexes during interpreter shtudown.

Only use _PyOS_SigintEvent() in the main thread in parking_lot.c, like we do in other places in the CPython codebase.

On Windows, the `_PyOS_SigintEvent()` event handle is used to interrupt
the main thread when Ctrl-C is pressed. Previously, we also waited on
the event from other threads, but ignored the result. However, this can
race with interpreter shutdown because the main thread closes the handle
in `_PySignal_Fini` and threads may still be running and using mutexes
during interpreter shtudown.

Only use `_PyOS_SigintEvent()` in the main thread in parking_lot.c, like
we do in other places in the CPython codebase.
@colesbury
Copy link
Contributor Author

Manually verified that Ctrl-C still interrupts lock acquisitions on Windows.

@colesbury colesbury marked this pull request as ready for review June 3, 2025 19:08
@colesbury
Copy link
Contributor Author

!buildbot Windows

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 7c7d96e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135100%2Fmerge

The command will test the builders whose names match following regular expression: Windows

The builders matched are:

  • AMD64 Windows11 Bigmem PR
  • AMD64 Windows PGO PR
  • ARM64 Windows Non-Debug PR
  • ARM64 Windows PR
  • AMD64 Windows11 Refleaks PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Windows11 Non-Debug PR
  • AMD64 Windows PGO NoGIL PR
  • AMD64 Windows10 PR

@encukou
Copy link
Member

encukou commented Jun 4, 2025

Thank you! The Windows buildbots look very happy with this change.

@encukou encukou merged commit cc581f3 into python:main Jun 4, 2025
52 checks passed
@miss-islington-app
Copy link

Thanks @colesbury for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 4, 2025
…ythonGH-135100)

On Windows, the `_PyOS_SigintEvent()` event handle is used to interrupt
the main thread when Ctrl-C is pressed. Previously, we also waited on
the event from other threads, but ignored the result. However, this can
race with interpreter shutdown because the main thread closes the handle
in `_PySignal_Fini` and threads may still be running and using mutexes
during interpreter shtudown.

Only use `_PyOS_SigintEvent()` in the main thread in parking_lot.c, like
we do in other places in the CPython codebase.
(cherry picked from commit cc581f3)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jun 4, 2025

GH-135116 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 4, 2025
@colesbury colesbury deleted the gh-135099-parking-lot-crash branch June 4, 2025 12:44
colesbury added a commit that referenced this pull request Jun 4, 2025
…H-135100) (GH-135116)

On Windows, the `_PyOS_SigintEvent()` event handle is used to interrupt
the main thread when Ctrl-C is pressed. Previously, we also waited on
the event from other threads, but ignored the result. However, this can
race with interpreter shutdown because the main thread closes the handle
in `_PySignal_Fini` and threads may still be running and using mutexes
during interpreter shtudown.

Only use `_PyOS_SigintEvent()` in the main thread in parking_lot.c, like
we do in other places in the CPython codebase.
(cherry picked from commit cc581f3)

Co-authored-by: Sam Gross <colesbury@gmail.com>
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.

4 participants