[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +3845

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
resolution:  -> fixed
stage: backport needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
pull_requests: +2559

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
pull_requests: +2560

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
pull_requests: +2558

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
stage: patch review -> backport needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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",
, , )) {
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().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

> 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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Antoine Pitrou added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[STINNER Victor]

STINNER Victor added the comment:

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

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
pull_requests: +2552

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30807] setitimer() can disable timer by mistake

[Antoine Pitrou]

New submission from Antoine 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.

--
components: Library (Lib)
messages: 297300
nosy: pitrou
priority: normal
severity: normal
stage: needs patch
status: open
title: setitimer() can disable timer by mistake
type: behavior
versions: Python 2.7, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com