Repository navigation
expose pthread_sigmask(), pthread_kill(), sigpending() and sigwait() in the signal module #52654
Description
| BPO | 8407 |
|---|---|
| Nosy | @loewis, @gpshead, @jcea, @pitrou, @vstinner, @giampaolo, @benjaminp, @skrah, @smarnach |
| Files |
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 = <Date 2011-06-10.10:59:18.072>
created_at = <Date 2010-04-15.05:00:36.517>
labels = ['extension-modules', 'type-feature', 'library']
title = 'expose pthread_sigmask(), pthread_kill(), sigpending() and sigwait() in the signal module'
updated_at = <Date 2012-03-30.23:24:33.524>
user = 'https://bugs.python.org/exarkun'bugs.python.org fields:
activity = <Date 2012-03-30.23:24:33.524>
actor = 'smarnach'
assignee = 'exarkun'
closed = True
closed_date = <Date 2011-06-10.10:59:18.072>
closer = 'vstinner'
components = ['Extension Modules', 'Library (Lib)']
creation = <Date 2010-04-15.05:00:36.517>
creator = 'exarkun'
dependencies = []
files = ['21736', '21857', '21921', '21922', '21930', '22093', '22300', '22312']
hgrepos = []
issue_num = 8407
keywords = ['patch', 'needs review']
message_count = 68.0
messages = ['103182', '103183', '103184', '103210', '103326', '104725', '105099', '105100', '105123', '105134', '105162', '128979', '133977', '134111', '134113', '134861', '134865', '134973', '134978', '134981', '134982', '135011', '135029', '135032', '135038', '135041', '135087', '135112', '135118', '135122', '135220', '135252', '135266', '135436', '135437', '135438', '135439', '135440', '135442', '135499', '135509', '135511', '135512', '135514', '135539', '135580', '135581', '136761', '136816', '136817', '137071', '137382', '137387', '137407', '137945', '138017', '138018', '138036', '138039', '138042', '138057', '138061', '138066', '138072', '138073', '138076', '138077', '157157']
nosy_count = 15.0
nosy_names = ['loewis', 'gregory.p.smith', 'jcea', 'spiv', 'exarkun', 'pitrou', 'vstinner', 'giampaolo.rodola', 'benjamin.peterson', 'marcin.bachry', 'schmichael', 'skrah', 'neologix', 'python-dev', 'smarnach']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8407'
versions = ['Python 3.3']
Activity
exarkun commentedon Apr 15, 2010
Linux offers the signalfd syscall since 2.6.22 (with minor changes afterwards). This call allows signals to be handled as bytes read out of a file descriptor, rather than as interruptions to the flow of a program. Quite usefully, this file descriptor can be select()'d on (or poll()'d, epoll()'d, etc) alongside other "normal" file descriptors.
In order to effectively use signalfd(), the signals in question must be blocked, though. So it makes sense to expose sigprocmask(2) at the same time, in order to allow this blocking to be set up.
exarkun commentedon Apr 15, 2010
I've started on an implementation of this in /branches/signalfd-issue8407.
loewis commentedon Apr 15, 2010
Notice that 2.7 has seen its first beta release, so no new features are allowed for it. I think it's better to target this feature for 3.2.
pitrou commentedon Apr 15, 2010
I'm with Martin, better to target 3.2 IMO.
Does signalfd really bring something compared to set_wakeup_fd()?
The one big difference I can see is that set_wakeup_fd() doesn't transmit the signal number, but this could be fixed if desired (instead of sending '\0', send a byte containing the signal number).
exarkun commentedon Apr 16, 2010
There's a lot more information than the signal number available as well. The signalfd_siginfo struct has 16 fields in it now and may have more in the future.
Another advantage is that this approach allows all asynchronous preemption to be disabled. This side-steps the possibility of hitting any signal bugs in CPython itself. Of course, any such bugs that are found should be fixed, but fixing them is often quite challenging and involves lots of complex domain-specific knowledge. In comparison, the signalfd and sigprocmask extensions are quite straight-forward and self-contained.
It's also possible to have several signalfds, each with a different signal mask. set_wakeup_fd is limited to a single fd per-process.
sigprocmask has other uses all by itself, of course, like delaying the delivery of signals while some sensitive code runs.
exarkun commentedon May 1, 2010
One open question regarding interaction with threading. sigprocmask's behavior in a multithreaded program is unspecified. pthread_sigmask should be used instead. I could either expose both of these and let the caller choose, or I could make signal.sigprocmask use pthread_sigmask if it's available, and fall back to sigprocmask. I don't see any disadvantages of doing the latter, and the former seems like it would create an attractive nuisance. However, I don't have much experience with sigprocmask; I'm only interested in it because of its use in combination with signalfd.
pitrou commentedon May 5, 2010
Or perhaps you could disable the feature if pthread_sigmask isn't available. Apparently, FreeBSD and Mac OS X have it, as well as Linux.
tseaver commentedon May 5, 2010
Trying pthread_sigmask first, and falling back, seems like the right strategy to me.
exarkun commentedon May 6, 2010
I think this is ready for a first review. See <http://codereview.appspot.com/1132041\>. If everyone agrees this is inappropriate for 2.7, then I'll port the changes to 3.x. I don't expect there to be much difference in the 3.x version.
pitrou commentedon May 6, 2010
I think the decision is up to Benjamin.
benjaminp commentedon May 6, 2010
Let's leave it for 3.2.
46 remaining items
giampaolo commentedon Jun 9, 2011
This whole thread is becoming quite confusing.
It would be better to open a separate issue for any bug or feature request which is not related to "exposing signalfd(2) and pthread_sigmask".
vstinner commentedon Jun 9, 2011
You are complelty right, sorry to pollute this issue. Changes related to this issue:
I created issue bpo-12303 for sigwaitinfo() and sigtimedwait(), and issue bpo-12304 for signalfd.
[-]expose signalfd(2) and pthread_sigmask in the signal module[/-][+]expose pthread_sigmask(), pthread_kill(), sigpending() and sigwait() in the signal module[/+]python-dev commentedon Jun 9, 2011
New changeset a5c8b6ebe895 by Victor Stinner in branch 'default':
Issue bpo-8407: signal.sigwait() releases the GIL
http://hg.python.org/cpython/rev/a5c8b6ebe895
vstinner commentedon Jun 9, 2011
sigwait() is not useless: the signal is not necessary send by another thread. The signal can be send by alarm(), from a child process, by an user, by the kernel.
Releasing the GIL is a new feature. Because it is cheap and pause() does also release the GIL, I commited your patch.
test_sigwait() test pass on all 3.x buildbots (some don't have sigwait(), the test is skipped). Sometimes, test_sigwait() is executed with 2 threads, the main thread and Tkinter event loop thread, and the signal is not blocked in any thread.
On Linux, it works well with more than one thread. I added a test using a thread, we will see if it works on buildbots. The signal is raised by a thread (whereas the signal is not blocked in any thread).
I wrote a can_test_blocked_signals() function in test_signal which has hardcoded tests to check for some known C threads: the faulthandler timeout thread and for the Tkinter event loop thread. can_test_blocked_signals() returns True if pthread_kill() is available.
I don't know how it works if a thread uses pthread_kill() to raise a signal into the main thread (waiting in sigwait()), whereas the signal is not blocked in any thread.
If it is not possible to get the list of all C/Python threads and/or block a signal in all threads, we can use a subprocess without threads (with only the main thread).
Would you like to work on a patch to avoid the undefined behaviour?
vstinner commentedon Jun 10, 2011
The test hangs on FreeBSD 8.2:
[235/356] test_signal
Timeout (1:00:00)!
Thread 0x0000000800e041c0:
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/test_signal.py", line 625 in test_sigwait_thread
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/case.py", line 407 in _executeTestPart
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/case.py", line 462 in run
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/case.py", line 514 in __call__
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/suite.py", line 105 in run
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/suite.py", line 67 in __call__
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/suite.py", line 105 in run
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/unittest/suite.py", line 67 in __call__
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/support.py", line 1166 in run
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/support.py", line 1254 in _run_suite
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/support.py", line 1280 in run_unittest
File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/test_signal.py", line 687 in test_main
File "./Lib/test/regrtest.py", line 1043 in runtest_inner
File "./Lib/test/regrtest.py", line 841 in runtest
File "./Lib/test/regrtest.py", line 668 in main
File "./Lib/test/regrtest.py", line 1618 in <module>
*** Error code 1
http://www.python.org/dev/buildbot/all/builders/AMD64%20FreeBSD%208.2%203.x/builds/519/steps/test/logs/stdio
(The test has not run yet on FreeBSD 6.4 buildbot)
vstinner commentedon Jun 10, 2011
See also bpo-12310 which may be related.
neologix commentedon Jun 10, 2011
If it doesn't release the GIL, it is useless.
The common usage pattern for sigwait() is to create a dedicated thread
for signals management. If this thread calls sigwait without releasing
the GIL, the whole process is suspended, which is definitely not what
you want.
It's mere luck:
Comment out the first two lines that change the signal handler, and
the test will be killed by SIGALRM's default handler ("Alarm clock").
I tested on Linux, and if a the signal isn't blocked before calling sigwait:
with SIGALRM the process gets killed)
This is a typical example of what POSIX calls "undefined behavior".
If pthread_sigmask is called before sigwait, then it works as expected.
If we really wanted to test this properly, the way to go would be to
fork() a child process (that way we're sure there's only one thread),
have it block and sigwait() SIGUSR1 without touching the handler, send
it SIGUSR1, and check its return code.
Patch attached.
python-dev commentedon Jun 10, 2011
New changeset a17710e27ea2 by Victor Stinner in branch 'default':
Issue bpo-8407: Make signal.sigwait() tests more reliable
http://hg.python.org/cpython/rev/a17710e27ea2
vstinner commentedon Jun 10, 2011
neologix> Patch attached.
I wrote a different patch based on your patch. The main change is that I chose to keep signal.alarm(1) instead of sending the signal from the parent process to the child process, because I don't think that time.sleep(0.5) is a reliable synchronization code to wait until the child process is waiting in sigwait().
test_sigwait_thread() uses time.sleep(1) to wait until the main thread is waiting in sigwait(), but it is not mandatory because the signal is already blocked in the thread. I wrote test_sigwait_thread() to ensure that sigwait() releases the GIL, not to check that if a thread raises a signal while sigwait() is waiting, sigwait() does correctly receive it.
We may use time.sleep() in test_sigwait() if the signal is blocked in the parent process before the fork() (and unblocked in the parent process after the fork, but before sending the signal to the child process), but I think that signal.alarm() is more reliable and simple.
--
test_sigwait(_thread) was the last known bug of this issue, so let's close it. Reopen it if you see other bugs in pthread_sigmask(), pthread_kill(), sigpending() and sigwait() functions, or maybe better: open new issues because this issue has a too long history! See issue bpo-12303 for sigwaitinfo() and sigtimedwait(), and issue bpo-12304 for signalfd.
python-dev commentedon Jun 10, 2011
New changeset 37a87b709403 by Victor Stinner in branch 'default':
Issue bpo-8407: write error message on sigwait test failure
http://hg.python.org/cpython/rev/37a87b709403
python-dev commentedon Jun 10, 2011
New changeset 60b1ab4d0cd4 by Victor Stinner in branch 'default':
Issue bpo-8407: skip sigwait() tests if pthread_sigmask() is missing
http://hg.python.org/cpython/rev/60b1ab4d0cd4
smarnach commentedon Mar 30, 2012
The documentation has been left in a confusing state by this patch -- at least confusing to me. I've created bpo-14456 with further details.