Signal handler callables take int, not Signals
#5622
Conversation
The documentation for [`signal.signal`][1] points out that the current
annotation for signal handlers might be wrong (emphasis my own):
> The handler is called with two arguments: the signal **number** and the
> current stack frame (**`None` or** a frame object; for a description of frame
> objects, see the description in the type hierarchy or see the attribute
> descriptions in the `inspect` module).
And when we use them, we can see that the signal number is passed as an `int`,
not a `signal.Signals` member:
import signal
def handler(signal_number, frame):
print("In signal handler!")
print("Signal number:", signal_number, type(signal_number))
print("Stack frame: ", frame, type(frame))
# Set signal handler:
signal.signal(signal.SIGHUP, handler)
# Use it:
signal.raise_signal(signal.SIGHUP)
Which prints:
In signal handler!
Signal number: 1 <class 'int'>
Stack frame: <frame at 0x7f804402abe0, file '<stdin>', line 12, code <module>> <class 'frame'>
[1]: https://docs.python.org/3/library/signal.html#signal.signal
|
Diff from mypy_primer, showing the effect of this PR on open source code: pip (https://github.com/pypa/pip.git)
+ src/pip/_internal/cli/progress_bars.py:86: error: Incompatible types in assignment (expression has type "Callable[[int, FrameType], None]", variable has type "Union[Callable[[int, Optional[FrameType]], Any], int, Handlers, None]")
|
|
I can't figure out when the frame would actually be None. I tried catching a few different signals (SIGHUP, SIGCONT, SIGINT). The docs don't mention when it happens, besides mentioning that it can happen. Can we just use |
|
The CPython On a higher level, isn't the whole point of a type system to describe everything that can happen so that programmers can be proactively defensive against all possible states? I'm very wary of more convenient but less accurate type annotations, especially in code as low-level as this. |
|
I looked a bit and it happens if there is no frame in the current PyThreadState. I'm not sure when that would happen, perhaps during very early setup. |
|
The docs for inspect.currentframe say that on non-CPython, frames might not exist at all, so maybe |
| @@ -80,7 +80,7 @@ if sys.platform != "win32": | |||
| SIG_SETMASK = Sigmasks.SIG_SETMASK | |||
|
|
|||
| _SIGNUM = Union[int, Signals] | |||
| _HANDLER = Union[Callable[[Signals, FrameType], Any], int, Handlers, None] | |||
| _HANDLER = Union[Callable[[int, Optional[FrameType]], Any], int, Handlers, None] | |||
There was a problem hiding this comment.
Small nit for future reference: we now prefer the new Foo | None syntax instead of Optional[Foo].
Following the changes in python#5622 handlers are typed as taking an Optional frame but the default_int_handler does not take None. This seems inconsistent and pervents you from calling the default handler from a custom handler without checking that the frame is not None first.
Closes #5621.
In the
Callablevariant ofsignal._HANDLER, changeSignalstointandFrameTypetoOptional[FrameType].