Repository navigation
Provide a way to defer SIGINT handling in the current thread #75569
Description
| BPO | 31388 |
|---|---|
| Nosy | @gpshead, @ncoghlan, @pitrou, @njsmith, @jdemeyer, @1st1 |
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 2017-09-07.20:03:25.096>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'Provide a way to defer SIGINT handling in the current thread'
updated_at = <Date 2019-08-07.08:13:45.053>
user = 'https://github.com/ncoghlan'bugs.python.org fields:
activity = <Date 2019-08-07.08:13:45.053>
actor = 'jdemeyer'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2017-09-07.20:03:25.096>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 31388
keywords = []
message_count = 12.0
messages = ['301622', '301625', '301627', '301665', '301690', '301692', '301693', '301703', '301705', '301726', '306224', '349154']
nosy_count = 6.0
nosy_names = ['gregory.p.smith', 'ncoghlan', 'pitrou', 'njs', 'jdemeyer', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31388'
versions = ['Python 3.7']
Activity
ncoghlan commentedon Sep 7, 2017
As discussed in bpo-29988, it's currently difficult to write completely robust cleanup code in Python, as the default SIGINT handler may lead to KeyboardInterrupt being raised as soon as *any* Python code starts executing in the main thread, even when that Python code is part of:
bpo-29988 proposes a way to adjust with statements to ensure that __exit__ at least starts executing if __enter__ returned successfully, but that's only sufficient to ensure robust resource cleanup if the __exit__ method is implemented in C and never calls back in to any operation that starts executing Python code.
Currently, the "best" option for ensuring cleanup code isn't interrupted is to outright ignore SIGINT while the cleanup code is running:
Fortunately, most folks aren't willing to do this, but it does suggest a potential pattern for temporarily *deferring* SIGINT handling: adding a "deferred" attribute to the entries in the array that tracks incoming signals, and providing some C level decorators and context managers for manipulating that state.
pitrou commentedon Sep 7, 2017
I think SIGINT handling is the wrong level to do this. Instead, it should be done at the ceval level, at the point where the "eval breaker" flag is examined for any interruption request to the normal sequential flow of execution.
ncoghlan commentedon Sep 7, 2017
Yes, it could also be done as a temporary global block on all signal and pending call processing, not just on SIGINT specifically.
Either way, I'll also note that this can be a no-op in any thread other than the main thread, as those already delegate signal handling to the main thread.
njsmith commentedon Sep 8, 2017
Note also that adding a check to ceval is not sufficient -- there are also lots of calls to PyErr_CheckSignals scattered around the tree, e.g. next to every syscall that can return EINTR.
I've considered proposing something like this in the past, since it feels weird that currently "control-C actually works" is some proprietary advantage of trio instead of something that the interpreter supports. I haven't because I couldn't figure out how to do it in a clean way... there are some subtleties. But then I started to write a comment here saying why it's impossible, and realized maybe it's not so bad after all :-).
The main issue I was worried about was that in an event loop, there's a tricky potential race condition where you want the core loop to have protection enabled in general, but you need to receive signals when the loop blocks waiting for I/O, BUT you can't actually run the normal Python handlers here because if they raise an exception you'll lose the I/O state.
The solution is to use a mixture of set_wakeup_fd to handle the wakeup, and then an explicit run-signal-handlers primitive to collect the exception in a controlled manner:
So we should have a primitive like this.
The other issue is where how to store the "are we deferring signals?" state. The main feature we would like is for it to automatically vary depending on the execution context -- in particular, naively sticking it into a thread-local won't work, because if you use send/throw/next to switch contexts you want that to switch the deferred state as well. I started writing some complicated thing here involving new attributes on frames and functions and rules for initializing one from the other or inheriting it from a parent frame, and then I realized there were some complications around generators... and actually I was just reinventing the same machinery we need for exc_info and PEP-550. So probably the is just "use a PEP-550 context var". But, it will need a bit of special magic: we need to make it possible to atomically set this state to a particular value when invoking a function, and then restore it when exiting that function.
I'm imagining something like, you write:
and it sets some magic flag on the function object that makes it act as if you wrote:
except that the assignment happens atomically WRT interrupts.
So yeah, I think the necessary and sufficient features are:
pitrou commentedon Sep 8, 2017
Le 08/09/2017 à 03:54, Nathaniel Smith a écrit :
Perhaps you are gold-plating this too much. This is primitive for a
couple low-level routine writers, not for the average developer.
njsmith commentedon Sep 8, 2017
Yes... low-level routines like coroutine schedulers where you need to defer interrupts inside the scheduler but not inside the coroutines being scheduled. It's not gold plating, it's actually the original motivation :-)
https://vorpus.org/blog/control-c-handling-in-python-and-trio/#how-do-we-know-which-code-should-be-protected
pitrou commentedon Sep 8, 2017
You just wrote an example where you are protecting __exit__ from interrupts. Do you schedule coroutines inside __exit__?
njsmith commentedon Sep 8, 2017
... it's true I did write such an example. And a few lines up in the same message I wrote an example where I was protecting an event loop from interrupts. In both cases the tricky question is how to manage the transitions between protected and unprotected without race conditions. Context-local state is a great solution for part of this problem.
I did realize this morning that technically it is *possible* to make a plain thread local work: you have to move all the state management into task context. So each time you start a task, wrap it in a helper that enables interrupts, and in early lowest level function that yields, reenable interrupts. This seems a bit pointless if we have context local state available though, because using context local state adds zero overhead to yields, and we yield approximately 10,000,000x more frequently than we receive SIGINT.
pitrou commentedon Sep 8, 2017
Le 08/09/2017 à 19:27, Nathaniel Smith a écrit :
But what overhead does context local state add to ceval.c?
Of course, we don't have the answer until Yuri actually submits an
implementation, but I'm not sure it can be as light-weight as currently.
njsmith commentedon Sep 8, 2017
Sure, but whatever overhead it has, it has. Once we're paying for it, new keys are free (at yield/resume time).
Compared to a bare thread-local it probably has somewhat higher overhead when we have to check it, but (a) that's why PEP-550 has a clever caching mechanism, and (b) you don't have to check it until you have a pending signal, and you almost never have a pending signal.
Anyway, we can wait until PEP-550 settles down before making any commitment here...
jdemeyer commentedon Nov 14, 2017
It's not only about ensuring that cleanup code gets run, but also about ensuring that the interrupt is actually seen. Currently, if you press CTRL-C at the "wrong" moment (during __del__), it will just get ignored.
jdemeyer commentedon Aug 7, 2019
Another idea I had is to somehow deal with this in PyErr_WriteUnraisable: whenever PyErr_WriteUnraisable is called for a KeyboardInterrupt, defer that exception to a later time, for example when _PyEval_EvalFrameDefault() is called.
ncoghlan commentedon Oct 25, 2024
Just noting that while 4958f5d resolved the resource cleanup failures reported in #74174 for context managers implemented in C (by only checking for external interrupts when returning from function calls and on backwards jumps in the opcode evaluation sequence), there's not yet a way to obtain similar guarantees in regular Python code (hence this issue still being open).
Context variables were added back in Python 3.7, so tracking this "signals are deferred" state in a coroutine-friendly way could potentially rely on that, but doing so wouldn't necessarily manage that state properly for generator frames (since running those in a separate context from the calling function remains a deferred proposal).