Repository navigation
signal.signal/signal.alarm not working as expected #53713
Description
| BPO | 9504 |
|---|---|
| Nosy | @birkenfeld, @ronaldoussoren, @pitrou, @vstinner, @vadmium, @jdemeyer, @iritkatriel |
| 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 2021-12-11.19:08:53.459>
created_at = <Date 2010-08-04.08:33:55.189>
labels = ['type-bug']
title = 'signal.signal/signal.alarm not working as expected'
updated_at = <Date 2021-12-11.19:08:53.458>
user = 'https://bugs.python.org/alanwilter'bugs.python.org fields:
activity = <Date 2021-12-11.19:08:53.458>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2021-12-11.19:08:53.459>
closer = 'iritkatriel'
components = []
creation = <Date 2010-08-04.08:33:55.189>
creator = 'alanwilter'
dependencies = []
files = ['20327']
hgrepos = []
issue_num = 9504
keywords = ['patch']
message_count = 16.0
messages = ['112771', '125815', '125830', '125845', '125851', '125863', '127960', '127964', '254946', '254948', '255484', '255499', '255520', '255536', '408259', '408280']
nosy_count = 10.0
nosy_names = ['georg.brandl', 'ronaldoussoren', 'pitrou', 'vstinner', 'nirai', 'neologix', 'alanwilter', 'martin.panter', 'jdemeyer', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9504'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']
Activity
alanwilter commentedon Aug 4, 2010
I have this example code to illustrate a problem I am having with python3. It works fine with python 2.6 and 2.7 but does not with python 3.1.
Executing it:
#### It prints essentially the same thing with a *very* *big* difference it takes 22 seconds and actually the alarm only works when the whole task ('find /') is finished.
neologix commentedon Jan 9, 2011
It's due to the way the python interpreter handles signals: when the signal is received, python runs a stub signal handler that just sets a flag indicating that the signal has been received: the actual handler is executed later, synchronously, mainly from the main evaluation loop.
The problem here is that since the main process is performing a read until EOF is reached (i.e. until the subprocess exits), the C code loops around the read(2) call without giving a chance for the handler to run.
A patch for is similar issue has been applied, see http://bugs.python.org/issue9617#
But it only fixed writes, not reads.
I think _bufferedreader_read_all and _bufferedreader_read_generic should be fixed too to call PyErr_CheckSignals().
birkenfeld commentedon Jan 9, 2011
Antoine?
pitrou commentedon Jan 9, 2011
Charles-François' analysis seems to be right. Note that the actual issue here is that read() always succeeds, returning a partial result (because you're executing a command, 'find /', which outputs a lot of data). If read() were interrupted before anything could be read, it would return EINTR and the handler would get executed immediately.
Anyone wants to propose a patch + tests?
neologix commentedon Jan 9, 2011
Attached is a tentative patch: it checks for pending signals inside
_bufferedreader_read_generic, fileio_readall, and rawiobase_readall.
I tested quickly on regular files and on pipes, and handlers are now
called on time. As a an obvious bonus, it's also easier to interrupt a
script doing I/O with a KeyboardInterrupt (SIGINT).
If nothing seems blatantly wrong about this patch, I will go ahead and
try to write some tests for this.
Charles
pitrou commentedon Jan 9, 2011
A potential downside is that the bytes which have been read simply get
lost. But since a signal can happen at any moment I guess nothing can be
done to protect from it in the general case, anyway.
As for the tests, the SignalsTest class in test_io.py is a natural
recipient.
pitrou commentedon Feb 5, 2011
It seems that my patch for bpo-10956 also fixes this issue (although it doesn't try to fix FileIO). It also has tests and fixes read() as well as write().
I'm sorry for not noticing that the two issues were so similar. Suggest closing this issue as duplicate.
pitrou commentedon Feb 5, 2011
Sorry, bad analysis. The patch in bpo-10956 only seemed to work because my system is too fast and "find /" in the script returned in less than 5 seconds. Replacing it with "yes" shows that the two issued are actually independent.
vadmium commentedon Nov 20, 2015
With Python’s current technique for signal handling, I think there is always going to be a window where a signal can arrive at the low level after checking the Python flag and before starting a blocking call. If you need to robustly handle a signal maybe it is better to use set_signal_fd() and select() or something.
I haven’t looked at the patch closely, but it sounds like it could reduce the size of the window. This would make interactive signals like SIGINT more reliable, so it may be worthwhile. It probably needs properly reviewing in light of the recent EINTR (PEP-457) changes though.
vadmium commentedon Nov 20, 2015
Sorry, EINTR changes were PEP-475.
jdemeyer commentedon Nov 27, 2015
Just a comment: if you need really robust signal handling, you just cannot do it with pure Python. I would recommend using Cython, where one has complete control over when signals are checked.
vstinner commentedon Nov 27, 2015
I don't understand this issue.
I would expect that a system call fails with EINTR if it is interrupted by a signal. Python explicitly configures the C library to get EINTR errors: automatic retry is disabled when Python sets its signal handlers.
If a system call doesn't fail, what shoud we do with the result, especially when reading data (read/recv)?
Yes, we _can_ checks to check regulary if we got a C signal by adding checks, but it's unclear to me if we must raise the exception immediatly, or later. Immedialty if I understand correctly the initial message. So should we check *all* system calls, or only system calls well known to block on I/O, like read/write, send/recv, select, etc.?
Do we try to workaround design choices by the Linux kernel and the GNU libc?
Right now, my favorite choice would be to do nothing. Yes, sometimes signals are delayed. Sometimes, a signal doesn't interrupt immediatly a blocked program. It's not perfect. But it's already very complex, I'm not sure that I want to make the code even more complex to check for signals in more places.
vadmium commentedon Nov 28, 2015
Okay, I tend to agree, and don’t have a problem with rejecting this.
ronaldoussoren commentedon Nov 28, 2015
Victor: read(2) and recv(2) are defined as returning all available data when interrupted by a signal and might therefore not return an error. The same is true for write(2): it returns the number of bytes written when there are any bytes written (instead of setting errno to SIGINT and returning an error).
I'd expect the C implementation of readall to behave the same as the Python implementation: a signal handler that raises an exception can cause data loss due to discarding the buffer that's used to collect the data. The data loss in the script isn't worse than it currently is, AFAIK it would currently lose data due to raising the exception just after the call to readall in the next loop through the eval loop.
If I read the issue correctly the signal handling issue mentioned here is basically similar to the one in this scriptlet:
When you run this with a large file on stdin (I've used /dev/zero) the script will continue to run indefinitely and cannot be cleanly interrupted at all (on my system its eventually killed by the system due to running out of its rlimit limitations, otherwise it can only be killed by sending it an uncatchable signal like SIGKILL or SIGABRT).
iritkatriel commentedon Dec 10, 2021
I'm unable to reproduce this on 3.11 on a Mac. Has it been fixed?
vstinner commentedon Dec 11, 2021
https://www.python.org/dev/peps/pep-0475/ changed deeply how Python handles signals. Python now tries to restart syscalls interrupted by signals (EINTR). A Python function only raises an exception if the Python signal handler raises an exception.