Title: Turn signal.SIG* constants into enums
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.5
Status: open Resolution:
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: giampaolo.rodola, gvanrossum, haypo, neologix, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2014-03-27 15:48 by giampaolo.rodola, last changed 2015-07-21 07:22 by ethan.furman.

File name Uploaded Description Edit
signals.patch giampaolo.rodola, 2014-03-27 15:48
signals2.patch giampaolo.rodola, 2014-03-27 23:28 review
signals3.patch giampaolo.rodola, 2014-03-28 17:13 review
signals4.patch giampaolo.rodola, 2014-03-28 19:40 review
signal_no_enum_handlers.patch serhiy.storchaka, 2015-01-26 18:59 review
Messages (21)
msg214959 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-27 15:48
Relevant discussion + BDFL approval:
Patch (not tested on Windows) is in attachment.
msg214960 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2014-03-27 16:09
OK, somebody please review this (not me).
msg214985 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2014-03-27 22:10
This patch can't be reviewed: please re-generate without --git.
msg214993 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-27 23:02
This time I made it without --git but that didn't help either.
Not sure what to do. :-\

Note: the devguide recommends using --git BTW:
Should that be changed?
msg215003 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-27 23:35
OK, it appears it works now. Sorry for the notification noise.
msg215042 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-28 12:52
(replying here 'cause rietveld keeps giving me an exception when I submit a reply)

On 2014/03/28 00:49:47, haypo wrote:
> File Lib/ (right):
> Lib/ if name.isupper()
> You can probably remove this test.
> Lib/ and (name.startswith('SIG') and not name.startswith('SIG_'))
> also be interesting to provide enums for them. They are just integers (0, 1 or
> 2) on my Linux.

I guess it makes sense.
I'm now realizing that the patch as-is is incomplete as the other "get" APIs (signal.getsignal() and others) still return integers: they should be overridden in order to convert integers into enums, similarly to
I will do that.
msg215059 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-28 17:13
New patch in attachment provides 3 enum containers and overrides all the necessary signal module APIs so that they interoperate with enums on both "get" and "set".
I decided *not* to rename signamodule.c to _signamodule.c in this patch so that it is easier to review (I will do the renaming as a second step).
msg215072 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-03-28 19:40
Updated patch following Victor's suggestions is in attachment.
msg215332 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-04-01 16:54
If there are no other concerns I will commit latest patch tomorrow, then do the renaming.
msg215519 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-04 13:34
New changeset c9239171e429 by Giampaolo Rodola' in branch 'default':
fix #21076: turn signal module constants into enums
msg215521 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-04-04 13:50

gcc -pthread   -Xlinker -export-dynamic -o Modules/_testembed Modules/_testembed.o libpython3.5dm.a -lpthread -ldl  -lutil   -lm  
libpython3.5dm.a(config.o):(.data+0x18): undefined reference to `PyInit_signal'
collect2: ld returned 1 exit status
make: *** [Modules/_testembed] Error 1
msg215526 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-04 14:31
New changeset df5120efb86e by Victor Stinner in branch 'default':
Issue #21076: the C signal module has been renamed to _signal
msg215528 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-04 15:00
New changeset b1f5b5d7997f by Victor Stinner in branch 'default':
Issue #21076: sigpending() is not available on Windows
msg234608 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-24 12:59
Now signal.signal() accepts inappropriate types.

>>> signal.signal(signal.SIGHUP, 0.0)
<Handlers.SIG_DFL: 0>
>>> signal.signal(signal.SIGHUP, '0')
<Handlers.SIG_DFL: 0>

In 3.4 it raised an exception.
msg234610 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-24 13:02
And more:

>>> import signal
>>> signal.signal(1.2, signal.SIG_DFL)
<Handlers.SIG_DFL: 0>
>>> signal.signal('1', signal.SIG_DFL)
<Handlers.SIG_DFL: 0>
msg234768 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-26 18:59
And more, as far as standard signal handler is tested for identity, signal.SIG_DFL and _signal.SIG_DFL should be the same object. Current code works only due to coincidence of two circumstances:

1) Small integers are cached in CPython.
2) SIG_DFL and SIG_IGN are small integers on common platforms.

When small integer caching is disabled (NSMALLPOSINTS == NSMALLPOSINTS == 0) or when platform depended macros SIG_DFL and SIG_IGN are not small integers, the signal module will mystically fail.

The simplest way to solve this issue is to backout turning SIG_DFL and SIG_IGN into enums.
msg234770 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-01-26 19:06
I know nothing about this part of CPython, but wouldn't the correct solution be to not compare by identity?
msg234776 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-26 20:39
I have other proposition -- turn them into functions (issue23325).
msg238317 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-03-17 17:06
Working on issue23673 I saw this in the new

+def _enum_to_int(value):
+    """Convert an IntEnum member to a numeric value.
+    If it's not a IntEnum member return the value itself.
+    """
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return value

The SIG, etc, constants are based on IntEnum, so they are already numeric values, and this function is unnecessary.
msg238319 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-03-17 17:09
Removing the 'enum_to_int' function would also take care of the accepting inappropriate types problem.
msg239556 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-03-30 01:55
Okay, in a perfect world that _enum_to_int function would be unnecessary, but as Serhiy pointed out the C code is doing pointer comparison, so unless the exact same int is passed in it does not work.

I'm looking at adjusting the C code.
Date User Action Args
2015-07-21 07:22:03ethan.furmansetnosy: - ethan.furman
2015-03-30 01:55:12ethan.furmansetmessages: + msg239556
2015-03-17 17:09:23ethan.furmansetmessages: + msg238319
2015-03-17 17:06:24ethan.furmansetmessages: + msg238317
2015-01-26 20:39:42serhiy.storchakasetmessages: + msg234776
2015-01-26 19:06:55ethan.furmansetmessages: + msg234770
2015-01-26 18:59:07serhiy.storchakasetfiles: + signal_no_enum_handlers.patch
type: behavior
messages: + msg234768

resolution: fixed ->
stage: resolved -> patch review
2015-01-24 13:02:21serhiy.storchakasetmessages: + msg234610
2015-01-24 12:59:12serhiy.storchakasetstatus: closed -> open

messages: + msg234608
2014-04-04 18:27:23giampaolo.rodolasetstatus: open -> closed
assignee: giampaolo.rodola
resolution: fixed
stage: patch review -> resolved
2014-04-04 15:00:14python-devsetmessages: + msg215528
2014-04-04 14:31:46python-devsetmessages: + msg215526
2014-04-04 13:50:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg215521
2014-04-04 13:34:28python-devsetnosy: + python-dev
messages: + msg215519
2014-04-01 16:54:09giampaolo.rodolasetmessages: + msg215332
2014-03-28 19:40:10giampaolo.rodolasetfiles: + signals4.patch

messages: + msg215072
2014-03-28 17:13:39giampaolo.rodolasetfiles: + signals3.patch

messages: + msg215059
2014-03-28 12:52:09giampaolo.rodolasetmessages: + msg215042
2014-03-27 23:35:25giampaolo.rodolasetmessages: + msg215003
2014-03-27 23:28:41giampaolo.rodolasetfiles: + signals2.patch
2014-03-27 23:09:07giampaolo.rodolasetfiles: - signals.patch
2014-03-27 23:02:10giampaolo.rodolasetmessages: + msg214993
2014-03-27 22:58:35giampaolo.rodolasetfiles: + signals.patch
2014-03-27 22:56:54giampaolo.rodolasetfiles: - signals.patch
2014-03-27 22:56:32giampaolo.rodolasetfiles: + signals.patch
2014-03-27 22:38:24hayposetnosy: + haypo
2014-03-27 22:10:13neologixsetnosy: + neologix
messages: + msg214985
2014-03-27 16:10:09ethan.furmansetnosy: + ethan.furman
2014-03-27 16:09:39gvanrossumsetstage: patch review
2014-03-27 16:09:27gvanrossumsetmessages: + msg214960
2014-03-27 15:55:28giampaolo.rodolasetfiles: - signals.patch
2014-03-27 15:55:19giampaolo.rodolasetfiles: + signals.patch
2014-03-27 15:48:50giampaolo.rodolacreate