New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setitimer() can disable timer by mistake #74990
Comments
The C setitimer() function supports intervals with microsecond resolution. However, Python's setitimer() takes a float and then converts it to a C |
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 :-) |
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 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. |
Well, select.select() is a better example. I had nightmare with the |
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 :-) |
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. |
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 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:
I understand that you want to use _PyTime_FromSecondsObject(), which 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 Hum, it seems like the main difference is that |
If you want to fix 2.7, I think that your simple test to round |
It seems _PyTime_ObjectToTimeval() would be better here. What do you think? |
Antoine Pitrou added the comment:
Oh, I forgot to document why _PyTime_ObjectToTimeval() still exists So the datetime module uses _PyTime_ObjectToTimeval() to use I would prefer to just fix _PyTime_FromSecondsObject() to handle 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 :-) |
(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 :-) |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: