This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Turn SIG_DFL and SIG_IGN into functions
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: barry, brett.cannon, christian.heimes, eli.bendersky, giampaolo.rodola, miss-islington, serhiy.storchaka, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2015-01-26 20:38 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
signal_std_handlers.patch serhiy.storchaka, 2015-01-26 20:38 review
Pull Requests
URL Status Linked Edit
PR 8920 open serhiy.storchaka, 2018-08-25 10:41
PR 31759 merged christian.heimes, 2022-03-08 10:35
PR 31768 merged miss-islington, 2022-03-08 18:22
Messages (14)
msg234775 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-26 20:38
In C the SIG_DFL and SIG_IGN macros expand into integral expressions that are not equal to an address of any function. In Python they are int objects with platform depended values. Second argument of the signal() function should be SIG_DFL, SIG_IGN, or a function. The getsignal() function returns SIG_DFL, SIG_IGN, None, or a function. They are tested for identity in signal() and getsignal() returns identical values. So actually SIG_DFL and SIG_IGN are just singletons.

I propose to turn SIG_DFL and SIG_IGN into functions. Benefits:

1. They will have names and self-descriptive representation.
2. They will have docstrings.
3. The signature of signal() will be simpler. The type of second argument would be function.
4. Their pickling will be portable.

This patch depends on the backout of turning these constants to enums (issue21076).
msg238327 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-03-17 18:13
A private method is being added to Enum to better support Enum replacement of constants, part of which includes changing __reduce_ex__ to return the string of the name.

These changes answer points 1 and 4.

Point 2 would be nice, but seems somewhat less important if Enums are being used.

Point 3 -- I don't see how 'Signal(xxx, yyy)' is more complicated than 'Signal(xxx, zzz)'?
msg238495 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-19 07:17
Currently the action parameter of signal.signal() can be SIG_DFL, SIG_IGN, or a callable Python object. Would be just a callable Python object. This is simpler.

The main benefit is that they will be better look in pydoc output.
msg238538 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-03-19 15:51
Thanks for the explanation, point taken.

However, it seems to me that changing the type of the constants from 'int' to 'function' is backwards incompatible, will break code, and is probably not worth it (and would require a deprecation period if it was worth it).

I still think the better answer is to delve into Modules/signalmodule.c and fix the comparisons.
msg324075 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2018-08-25 16:26
I'm afraid this could break old codes so not sure it's worth or not. I've seen code like `SIGTERM = 15; signal.signal(SIGTERM, handler)`. I won't be surprised to see a handcraft SIG_DFL/IGN.
msg324084 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-08-25 17:42
Numerical values of signal numbers are standardized. signal.signal(15, handler) is valid. SIGTERM is just a handy name for the constant 15.

But SIG_DFL and SIG_IGN are not integers in C (they are special pointers), and it is not declared anywhere that they should be integers in Python. The code that converts them to Python integer is implementation depending (PyLong_FromVoidPtr). Handcrafted SIG_DFL and SIG_IGN are broken by design, because the signal module uses identity comparison for SIG_DFL and SIG_IGN.
msg324096 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2018-08-25 19:36
I miss that. So this change seems to be user transparent and make more sense then.
msg411037 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-20 17:00
Serhiy, could you please rebase your PR to tip of main branch? I'd like to try it out.
msg411045 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-01-20 20:43
It may take a time, because the module initialization code has been completely rewritten.
msg411104 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-01-21 09:54
Understood, thanks!

A trivial fix for the integer comparison bug would solve my issue:

--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -527,21 +527,20 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler)
                          "signal number out of range");
         return NULL;
     }
-    if (handler == modstate->ignore_handler) {
+    if (PyCallable_Check(handler)) {
+        func = signal_handler;
+    }
+    else if (PyObject_RichCompareBool(handler, modstate->ignore_handler, Py_EQ) == 1) {
         func = SIG_IGN;
     }
-    else if (handler == modstate->default_handler) {
+    else if (PyObject_RichCompareBool(handler, modstate->default_handler, Py_EQ) == 1) {
         func = SIG_DFL;
-    }
-    else if (!PyCallable_Check(handler)) {
+    } else {
         _PyErr_SetString(tstate, PyExc_TypeError,
                          "signal handler must be signal.SIG_IGN, "
                          "signal.SIG_DFL, or a callable object");
         return NULL;
     }
-    else {
-        func = signal_handler;
-    }
 
     /* Check for pending signals before changing signal handler */
     if (_PyErr_CheckSignalsTstate(tstate)) {
msg414747 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2022-03-08 10:39
My PR 31759 removes the assumption of small int singletons and replaces C comparison with PyObject_RichCompareBool() Py_EQ calls.

I still prefer Serhiy's solution, but it may cause backwards incompatible breakage. My fix can be backported to 3.9 and 3.10 without breakage. It resolves my failing tests on Emscripten, too.
msg414749 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-03-08 12:29
Agree. There were too many changes in this code, and making SIG_DFL and SIG_IGN functions exposes some issues with sharing objects between interpreters. It is easier to keep them integers for now.
msg414765 - (view) Author: miss-islington (miss-islington) Date: 2022-03-08 18:22
New changeset c8a47e76a391c8818bf10a282cdcd3bb5c23ebf6 by Christian Heimes in branch 'main':
bpo-23325: Fix SIG_IGN and SIG_DFL int comparison in signal module (GH-31759)
https://github.com/python/cpython/commit/c8a47e76a391c8818bf10a282cdcd3bb5c23ebf6
msg414767 - (view) Author: miss-islington (miss-islington) Date: 2022-03-08 18:53
New changeset 95b001fe6766f491f4356f8bcf23d6895bab2342 by Miss Islington (bot) in branch '3.10':
bpo-23325: Fix SIG_IGN and SIG_DFL int comparison in signal module (GH-31759)
https://github.com/python/cpython/commit/95b001fe6766f491f4356f8bcf23d6895bab2342
History
Date User Action Args
2022-04-11 14:58:12adminsetgithub: 67514
2022-03-31 17:54:28brett.cannonsetnosy: + brett.cannon
2022-03-08 18:53:31miss-islingtonsetmessages: + msg414767
2022-03-08 18:22:48miss-islingtonsetmessages: + msg414765
2022-03-08 18:22:42miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request29876
2022-03-08 12:29:48serhiy.storchakasetmessages: + msg414749
2022-03-08 10:39:45christian.heimessetmessages: + msg414747
2022-03-08 10:35:54christian.heimessetpull_requests: + pull_request29871
2022-01-21 09:54:27christian.heimessetmessages: + msg411104
2022-01-20 20:43:16serhiy.storchakasetmessages: + msg411045
2022-01-20 17:00:18christian.heimessetnosy: + christian.heimes

messages: + msg411037
versions: + Python 3.9, Python 3.11
2022-01-17 09:13:52christian.heimeslinkissue40280 dependencies
2022-01-17 09:13:13christian.heimeslinkissue46408 superseder
2020-06-28 11:04:39serhiy.storchakasetversions: + Python 3.10, - Python 3.8
2018-08-25 19:36:50xiang.zhangsetnosy: + vstinner
messages: + msg324096
2018-08-25 17:42:29serhiy.storchakasetmessages: + msg324084
2018-08-25 16:26:34xiang.zhangsetnosy: + xiang.zhang

messages: + msg324075
versions: + Python 3.8, - Python 3.5
2018-08-25 10:41:14serhiy.storchakasetpull_requests: + pull_request8393
2015-07-21 08:13:33ethan.furmansetnosy: - ethan.furman
2015-03-19 15:51:50ethan.furmansetmessages: + msg238538
2015-03-19 07:17:04serhiy.storchakasetmessages: + msg238495
2015-03-17 18:13:15ethan.furmansetnosy: + barry, eli.bendersky
messages: + msg238327
2015-01-26 20:38:30serhiy.storchakacreate