Turn signal.SIG* constants into enums

[giampaolo]

BPO	21076
Nosy	@gvanrossum, @vstinner, @giampaolo, @serhiy-storchaka
Files	signals.patch signals2.patch signals3.patch signals4.patch signal_no_enum_handlers.patch

^{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 = 'https://github.com/giampaolo'
closed_at = <Date 2020-06-25.09:30:58.351>
created_at = <Date 2014-03-27.15:48:50.909>
labels = ['type-bug', 'library']
title = 'Turn signal.SIG* constants into enums'
updated_at = <Date 2020-06-25.09:30:58.350>
user = 'https://github.com/giampaolo'

bugs.python.org fields:

activity = <Date 2020-06-25.09:30:58.350>
actor = 'vstinner'
assignee = 'giampaolo.rodola'
closed = True
closed_date = <Date 2020-06-25.09:30:58.351>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2014-03-27.15:48:50.909>
creator = 'giampaolo.rodola'
dependencies = []
files = ['34636', '34643', '34653', '34655', '37865']
hgrepos = []
issue_num = 21076
keywords = ['patch']
message_count = 22.0
messages = ['214959', '214960', '214985', '214993', '215003', '215042', '215059', '215072', '215332', '215519', '215521', '215526', '215528', '234608', '234610', '234768', '234770', '234776', '238317', '238319', '239556', '372322']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'vstinner', 'giampaolo.rodola', 'neologix', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21076'
versions = ['Python 3.5']

[giampaolo]

Relevant discussion + BDFL approval:
https://mail.python.org/pipermail/python-ideas/2014-March/027286.html
Patch (not tested on Windows) is in attachment.

[gvanrossum]

OK, somebody please review this (not me).

[neologix]

This patch can't be reviewed: please re-generate without --git.

[giampaolo]

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: http://docs.python.org/devguide/committing.html
Should that be changed?

[giampaolo]

OK, it appears it works now. Sorry for the notification noise.

[giampaolo]

(replying here 'cause rietveld keeps giving me an exception when I submit a reply)

On 2014/03/28 00:49:47, haypo wrote:

http://bugs.python.org/review/21076/diff/11457/Lib/signal.py
File Lib/signal.py (right):

http://bugs.python.org/review/21076/diff/11457/Lib/signal.py#newcode7
Lib/signal.py:7: if name.isupper()
You can probably remove this test.

http://bugs.python.org/review/21076/diff/11457/Lib/signal.py#newcode8
Lib/signal.py:8: and (name.startswith('SIG') and not name.startswith('SIG_'))
Why do you ignore SIG_DFL, SIG_IGN, SIG_BLOCK, SIG_UNBLOCK, SIG_SETMASK? It may
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 http://hg.python.org/cpython/file/d8659dbebfd1/Lib/socket.py#l262
I will do that.

[giampaolo]

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).

[giampaolo]

Updated patch following Victor's suggestions is in attachment.

[giampaolo]

If there are no other concerns I will commit latest patch tomorrow, then do the renaming.

[python-dev]

New changeset c9239171e429 by Giampaolo Rodola' in branch 'default':
fix bpo-21076: turn signal module constants into enums
http://hg.python.org/cpython/rev/c9239171e429

[serhiy-storchaka]

http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.x/builds/4131/steps/compile/logs/stdio

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

[python-dev]

New changeset df5120efb86e by Victor Stinner in branch 'default':
Issue bpo-21076: the C signal module has been renamed to _signal
http://hg.python.org/cpython/rev/df5120efb86e

[python-dev]

New changeset b1f5b5d7997f by Victor Stinner in branch 'default':
Issue bpo-21076: sigpending() is not available on Windows
http://hg.python.org/cpython/rev/b1f5b5d7997f

[serhiy-storchaka]

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.

[serhiy-storchaka]

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>

[serhiy-storchaka]

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.

[ethanfurman]

I know nothing about this part of CPython, but wouldn't the correct solution be to not compare by identity?

[serhiy-storchaka]

I have other proposition -- turn them into functions (bpo-23325).

[ethanfurman]

Working on bpo-23673 I saw this in the new signal.py:

+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.

[ethanfurman]

Removing the 'enum_to_int' function would also take care of the accepting inappropriate types problem.

[ethanfurman]

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.

[vstinner]

signal constants are now enum, I close the issue. For further enhancements, please open a separated issue.

$ python3
Python 3.8.3 (default, May 15 2020, 00:00:00) 
>>> import signal; signal.SIGTERM
<Signals.SIGTERM: 15>