setitimer() can disable timer by mistake

[pitrou]

BPO	30807
Nosy	@pitrou, @vstinner
PRs	bpo-30807: signal.setitimer() may disable the timer by mistake #2493 [3.6] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) #2497 [3.5] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) #2498 [2.7] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) #2499 bpo-30807: signal.setitimer() now uses _PyTime API #3865

^{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 = None
closed_at = <Date 2017-06-30.08:55:38.491>
created_at = <Date 2017-06-29.19:49:09.999>
labels = ['3.7', 'type-bug', 'library']
title = 'setitimer() can disable timer by mistake'
updated_at = <Date 2017-10-13.20:49:45.558>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2017-10-13.20:49:45.558>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-06-30.08:55:38.491>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2017-06-29.19:49:09.999>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 30807
keywords = []
message_count = 26.0
messages = ['297300', '297306', '297308', '297309', '297310', '297311', '297314', '297316', '297318', '297319', '297320', '297321', '297322', '297344', '297346', '297348', '297350', '297353', '297354', '297355', '297356', '297357', '297362', '297363', '297364', '304359']
nosy_count = 2.0
nosy_names = ['pitrou', 'vstinner']
pr_nums = ['2493', '2497', '2498', '2499', '3865']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30807'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

[pitrou]

The C setitimer() function supports intervals with microsecond resolution. However, Python's setitimer() takes a float and then converts it to a C struct timeval, which can round down to zero. The consequence is that the timer is disabled (timeval == {0,0}) while the user asked for a one microsecond timeout.

[vstinner]

I would prefer to reuse pytime.c conversion code which is well tested by test_time. _PyTime_FromObject() already contains code to convert a double to _PyTime_t, and _PyTime_AsTimeval() converts a _PyTime_t to timeval. Use _PyTime_ROUND_CEILING as signal.sigtimedwait().

For signal.sigtimedwait(): see commit 34dc0f4 and http://bugs.python.org/issue22117.

Using ROUND_CEILING is now also used in the select module for a similar reason.

See my https://haypo.github.io/pytime.html article for the horror story of timestamp rounding :-)

[vstinner]

I'm not sure about modifying Python 2.7. For example, we didn't change the select module in Python 2.7 to round correctly the timeout.

[pitrou]

It's not about correctly rounding something, it's about fixing a bug. I don't care about rounding if it means being a microsecond late.

[pitrou]

I would prefer to reuse pytime.c conversion code which is well tested by test_time. _PyTime_FromObject() already contains code to convert a double to _PyTime_t, and _PyTime_AsTimeval() converts a _PyTime_t to timeval. Use _PyTime_ROUND_CEILING as signal.sigtimedwait().

I don't understand how that works. Can you post a snippet implementing what is needed here?

[pitrou]

sigtimedwait() is different: zero doesn't mean anything special. If you mistake zero for 1e-6 or the reverse, it works fine.

[vstinner]

sigtimedwait() is different: zero doesn't mean anything special. If you mistake zero for 1e-6 or the reverse, it works fine.

Well, select.select() is a better example. I had nightmare with the
rounding of its timeout :-)

[pitrou]

Well, select.select() is a better example. I had nightmare with the
rounding of its timeout

Not sure why? If you pass 0 instead of a tiny value, select() shouldn't behave significantly differently.

In any case, I don't want to have nightmares, I simply want to fix a bug that contributed to make one the buildbots fail :-)

[vstinner]

Not sure why? If you pass 0 instead of a tiny value, select() shouldn't behave significantly differently.

I used select() as an example of function where we also changed how the timeout was rounded, so we had to decide how to handle backward compatibility.

Rounding select() timeout has an big impact of power consumption in an event loop, especially when you use poll() which only has a resolution of 1 ms (and not 1 us). If you round 0.1 ms to 0, you enter a busy-loop which burns your CPU during 0.1 ms. If you do that many times, it can be much longer than 0.1 ms and so be very inefficient.

We had this performance issue in asyncio. We fixed it in asyncio *and* select modules.

https://bugs.python.org/issue20311

[pitrou]

I don't understand how 0.1 ms can be rounded down to 0 in a "struct timeval", which has microsecond precision.

[pitrou]

Ok, it's not select(), it's epoll_wait(). I see.

[pitrou]

If I use the PyTime APIs, it will change the signature from
setitimer($module, which, seconds, interval=0.0, /)
to
setitimer($module, which, seconds, interval=None, /).

I'm not sure that's ok in a bugfix release?