Skip to content

Delete stale note about mp.Lock.acquire/SIGINT #120929

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 1 commit into from
Jul 21, 2024
Merged

Delete stale note about mp.Lock.acquire/SIGINT #120929

merged 1 commit into from
Jul 21, 2024

Conversation

andmis
Copy link
Contributor

@andmis andmis commented Jun 24, 2024

The note I've deleted in this PR from the multiprocessing docs does not seem to be true anymore. (Or I'm misunderstanding something.)

~/tmp cat main.py
import threading
L = threading.Lock()
L.acquire()
print('got it')
L.acquire()
~/tmp python3.12 main.py
got it
^CTraceback (most recent call last):
  File "/Users/andrey/tmp/main.py", line 5, in <module>
    L.acquire()
KeyboardInterrupt
~/tmp:130

@andmis andmis requested a review from gpshead as a code owner June 24, 2024 01:07
@ghost
Copy link

ghost commented Jun 24, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Jun 24, 2024
Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looking at the tests / history, I don't think it's guaranteed that SIGINT will be handled in threading:

@unittest.skipIf(USING_PTHREAD_COND,
'POSIX condition variables cannot be interrupted')
@unittest.skipIf(sys.platform.startswith('linux') and
not sys.thread_info.version,
'Issue 34004: musl does not allow interruption of locks '
'by signals.')
# Issue #20564: sem_timedwait() cannot be interrupted on OpenBSD
@unittest.skipIf(sys.platform.startswith('openbsd'),
'lock cannot be interrupted on OpenBSD')

(I'm not sure how much that's up to date though, e.g. I couldn't reproduce on Alpine)

So it feels like this is still a useful additional guarantee to document. That said, Python did used to always ignore SIGINT in threading, and no longer does, so maybe we should say something like:

This differs from the behaviour of :mod:threading where on certain platforms SIGINT is
not guaranteed to be able to interrupt while the equivalent blocking calls are in progress.

@gpshead
Copy link
Member

gpshead commented Jul 21, 2024

That text was from the original when multiprocessing was added. The behavior of threading locks in CPython changed in the 3.2 or 3.3 time-frame as we fixed our underlying implementation to use the right POSIX APIs. I agree, we don't need this text anymore. It is describing normal behavior of the module here and attempting to mention something about another module which isn't generally true anymore and even if it were, doesn't belong here.

@gpshead gpshead added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Jul 21, 2024
@gpshead gpshead enabled auto-merge (squash) July 21, 2024 06:16
@gpshead gpshead merged commit 0dcbc83 into python:main Jul 21, 2024
34 checks passed
@miss-islington-app
Copy link

Thanks @andmis for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 21, 2024
(cherry picked from commit 0dcbc83)

Co-authored-by: Andrey Mishchenko <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 21, 2024

GH-122078 is a backport of this pull request to the 3.13 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 21, 2024
(cherry picked from commit 0dcbc83)

Co-authored-by: Andrey Mishchenko <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 21, 2024
@bedevere-app
Copy link

bedevere-app bot commented Jul 21, 2024

GH-122079 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Jul 21, 2024
gpshead pushed a commit that referenced this pull request Jul 21, 2024
…-122079)

Delete stale note about mp.Lock.acquire/SIGINT (GH-120929)
(cherry picked from commit 0dcbc83)

Co-authored-by: Andrey Mishchenko <[email protected]>
gpshead pushed a commit that referenced this pull request Jul 21, 2024
…-122078)

Delete stale note about mp.Lock.acquire/SIGINT (GH-120929)
(cherry picked from commit 0dcbc83)

Co-authored-by: Andrey Mishchenko <[email protected]>
@andmis andmis deleted the andrey/delete-acquire-sigint-note branch August 14, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants