bpo-43406: Fix possible race condition where PyErr_CheckSignals tries to execute a non-Python signal handler
#24756
Conversation
…ries to execute a non-Python signal handler.
pitrou
added
needs backport to 3.8
| * raising cryptic exceptions asynchronously | ||
| * such as "TypeError: 'int' object is not callable". | ||
| */ | ||
| continue; |
There was a problem hiding this comment.
This should probably require some consensus, but I would personally raise, as the situation is tricky enough that I think it should not pass silently.
There was a problem hiding this comment.
@vstinner What do you think?
There was a problem hiding this comment.
One problem is that re-raising will break the assumption that _thread.interrupt_main only simulates SIGINT.
There was a problem hiding this comment.
One problem is that re-raising will break the assumption that
_thread.interrupt_mainonly simulates SIGINT.
That's an excellent point actually. Maybe we should set an unraisable exception (PyErr_WriteUnraisable)?
There was a problem hiding this comment.
That would not be much better than the asynchronous TypeError, would it?
There was a problem hiding this comment.
It does not set the error indicator IIRC and can be handled separately if needed by a hook. By default is like printing to stderr. The advantage would be that keeps the assumption that _thread.interrupt_main only simulates SIGINT but doesn't pass silently.
There was a problem hiding this comment.
Feel free to take a look at the updated patch @pablogsal .
Misc/NEWS.d/next/Core and Builtins/2021-03-04-22-53-10.bpo-43406.Na_VpA.rst
Outdated
Show resolved
Hide resolved
…06.Na_VpA.rst Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
There was a problem hiding this comment.
LGTM
This is great work. Thanks a lot for working on this!
I think we can refine the error message (so we don't say "race condition" as users won't be able to act on that) but I propose to merge this as is and I will propose another PR with an improvement in the error message.
|
Thanks @pitrou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
|
Thanks for the quick review @pablogsal ! |
…ries to execute a non-Python signal handler (pythonGH-24756) We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception. (cherry picked from commit 68245b7) Co-authored-by: Antoine Pitrou <antoine@python.org>
|
GH-24761 is a backport of this pull request to the 3.9 branch. |
|
Sorry, @pitrou, I could not cleanly backport this to |
…ls`` tries to execute a non-Python signal handler (pythonGH-24756) We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception.. (cherry picked from commit 68245b7) Co-authored-by: Antoine Pitrou <antoine@python.org>
|
GH-24762 is a backport of this pull request to the 3.8 branch. |
…ls`` tries to execute a non-Python signal handler (GH-24756) (GH-24761) We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception. (cherry picked from commit 68245b7) Co-authored-by: Antoine Pitrou <antoine@python.org>
…ls`` tries to execute a non-Python signal handler (GH-24756) (GH-24762) We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception.. (cherry picked from commit 68245b7) Co-authored-by: Antoine Pitrou <antoine@python.org>
…ries to execute a non-Python signal handler (pythonGH-24756) We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler. Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception.
https://bugs.python.org/issue43406