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.

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 34dc0f46ae5c0c9ec91d9402fac61111b802855f 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 :-)

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.

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.

> 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?

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

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

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

> 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

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

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

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?

Also using the PyTime APIs will make it annoying to backport to 2.7 (which isn't mandatory, of course).

Antoine Pitrou added the comment:
> 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?

I understand that you want to use _PyTime_FromSecondsObject(), which
is the right function to use. This function uses PyFloat_AsDouble() or
PyLong_AsLongLong().

The current code uses:

    if (!_PyArg_ParseStack(args, nargs, "id|d:setitimer",
        &which, &seconds, &interval)) {
        goto exit;
    }

The "d" format uses PyFloat_AsDouble(). This function uses
Py_TYPE(op)->tp_as_number->nb_float(op) to convert an object to a
float.

Hum, it seems like the main difference is that
_PyTime_FromSecondsObject() doesn't handle objects defining
__float__() the same way. For example, _PyTime_FromSecondsObject()
rounds a decimal.Decimal to an integer, not a float :-/ It looks like
a bug in _PyTime_FromSecondsObject() which should first try to call
PyFloat_AsDouble(), catch TypeError and fallback to
PyLong_AsLongLong().

> Also using the PyTime APIs will make it annoying to backport to 2.7 (which isn't mandatory, of course).

If you want to fix 2.7, I think that your simple test to round
"manually" to 1 microsecond is enough.

It seems _PyTime_ObjectToTimeval() would be better here.  What do you think?

Antoine Pitrou added the comment:
> It seems _PyTime_ObjectToTimeval() would be better here.  What do you think?

Oh, I forgot to document why _PyTime_ObjectToTimeval() still exists
:-) The _PyTime_t type supports a range of [-292 years, +292 years]:
it's enough to pass a timeout value to select.select() or
signal.setitimer(). But it's not enough to pass a timestamp relative
to the UNIX Epoch (1970-01-01 00:00), since we want to support crazy
dates like up to (0001-01-01 00:01) in the datetime module.

So the datetime module uses _PyTime_ObjectToTimeval() to use
internally the time_t type which supports a range of +/-
292,271,023,045 years when time_t is 64-bit long (which becomes the
case on most platforms) :-)

I would prefer to just fix _PyTime_FromSecondsObject() to handle
correctly decimal.Decimal, rather than using
_PyTime_ObjectToTimeval(). I tried to write an exhaustive test suite
for _PyTime_FromSecondsObject() which all corner cases, whereas
_PyTime_ObjectToTimeval() is less well tested and I would like to kill
it (but I don't know how at this point ;-)).

I can work on a patch, but not right now.

You can fix the bug differently if you prefer to not use pytime.

Ok, I will just push the original fix then :-)

New changeset 729780a810bbcb12b245a1b652302a601fc9f6fd by Antoine Pitrou in branch 'master':
bpo-30807: signal.setitimer() may disable the timer by mistake (#2493)
https://github.com/python/cpython/commit/729780a810bbcb12b245a1b652302a601fc9f6fd

(I must admit your latest explanations lost me.  Why shouldn't I use _PyTime_ObjectToTimeval(), which is a convenience function, and instead use several functions in a row to get the right conversion?)

What is the behaviour of setitimer() when the timeout is negative? Is this behaviour well defined (portable)?

It "shall fail" apparently :-)
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setitimer.html

New changeset 6f3cb059fd1f3a57ffd70c9d54a394a85a6ea13d by Antoine Pitrou in branch '3.6':
[3.6] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) (#2497)
https://github.com/python/cpython/commit/6f3cb059fd1f3a57ffd70c9d54a394a85a6ea13d

New changeset 5741b70acf88846a0d3b2d348535f250577b2df6 by Antoine Pitrou in branch '3.5':
[3.5] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) (#2498)
https://github.com/python/cpython/commit/5741b70acf88846a0d3b2d348535f250577b2df6

New changeset a45a99b47ff241ce0ae2f0bba59b89e4e012d47c by Antoine Pitrou in branch '2.7':
[2.7] bpo-30807: signal.setitimer() may disable the timer by mistake (GH-2493) (#2499)
https://github.com/python/cpython/commit/a45a99b47ff241ce0ae2f0bba59b89e4e012d47c

New changeset ef611c96eab0ab667ebb43fdf429b319f6d99890 by Victor Stinner in branch 'master':
bpo-30807: signal.setitimer() now uses _PyTime API (GH-3865)
https://github.com/python/cpython/commit/ef611c96eab0ab667ebb43fdf429b319f6d99890