Skip to content
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

_thread.interrupt_main() no longer interrupts Lock.wait #80719

Open
gpshead opened this issue Apr 5, 2019 · 2 comments
Open

_thread.interrupt_main() no longer interrupts Lock.wait #80719

gpshead opened this issue Apr 5, 2019 · 2 comments
Labels
3.8 only security fixes docs Documentation in the Doc dir extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@gpshead
Copy link
Member

gpshead commented Apr 5, 2019

BPO 36538
Nosy @gpshead, @eryksun

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-04-05.20:07:07.889>
labels = ['extension-modules', '3.8', 'type-bug', 'library', 'docs']
title = '_thread.interrupt_main() no longer interrupts Lock.wait'
updated_at = <Date 2019-04-06.07:28:06.177>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2019-04-06.07:28:06.177>
actor = 'eryksun'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'Extension Modules', 'Library (Lib)']
creation = <Date 2019-04-05.20:07:07.889>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 36538
keywords = []
message_count = 2.0
messages = ['339518', '339528']
nosy_count = 3.0
nosy_names = ['gregory.p.smith', 'docs@python', 'eryksun']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36538'
versions = ['Python 3.8']

@gpshead
Copy link
Member Author

gpshead commented Apr 5, 2019

In Python 2.7 our threading implementation was so poor that a thread join ultimately called our lock wait implementation that busy looped polling and sleeping to check for a lock acquisition success. calling thread.interrupt_main() which is just PyErr_SetInterrupt() C API in disguise successfully broke out of that lock wait loop.

In Python 3 with our drastically improved threading implementation, a lock wait is a pthreads sem_timedwait() or sem_trywait() API call, blocking within the pthreads library or OS kernel. PyErr_SetInterrupt() obviously has no effect on that. Only an actual signal arriving can interrupt that.

Thus instead of code using _thread.interrupt_main() - in 2and3 compatible applications, six.moves._thread.interrupt_main() - they should instead write:

os.kill(os.getpid(), signal.SIGINT)

Given that _thread is a private module making _thread.interrupt_main() a private API, do we need to keep it? If we do, we should at least document this behavior and recommend actually sending the signal. It is less capable of actually interrupting the main thread from some common blocking operations today. Sending the signal seems like it would always be better.

@gpshead gpshead added the 3.8 only security fixes label Apr 5, 2019
@gpshead gpshead added docs Documentation in the Doc dir extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Apr 5, 2019
@eryksun
Copy link
Contributor

eryksun commented Apr 6, 2019

Windows doesn't implement POSIX signals in the kernel, so _thread.interrupt_main (i.e. PyErr_SetInterrupt) can be useful as an abstraction.

Currently PyErr_SetInterrupt (Modules/signalmodule.c) doesn't set our SIGINT event in Windows. It was suggested in bpo-29926 to replace the trip_signal call with raise(SIGINT). That works for Windows -- as long as we're not concerned about emulating Ctrl+C exactly as if the user typed it in the console (if we're attached to a console). In POSIX, raise(SIGINT) won't interrupt the main thread if it's in a blocking system call, since it targets pthread_self(), so PyErr_SetInterrupt can instead call kill(getpid(), SIGINT), as you suggest, or pthread_kill(main_thread, SIGINT).

3.8 adds signal.raise_signal (see bpo-35568), so it's possible to implement this in pure Python. Either way, it would be nice to keep it as a cross-platform function, and hopefully make it public and documented.

Another option would be to enhance our Windows implementation of os.kill to more closely match POSIX kill(), such that os.kill(os.getpid(), signal.SIGINT) is implemented as raise(SIGINT). That would be a breaking change since currently it calls TerminateProcess (an abrupt termination like POSIX SIGKILL).

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 only security fixes docs Documentation in the Doc dir extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Status: No status
Development

No branches or pull requests

2 participants