| msg304386 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-10-14 08:07 |
| According to the documentation select.poll.poll() is blocked for infinite timeout if the timeout argument is negative. But due to rounding it is possible that timeout < 0, but an integer ms passed to the poll() syscall is 0, and poll() is not blocked. |
|
|
| msg304427 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-10-15 09:40 |
| I suggest to implement ROUND_UP in pytime and use it in all functions accepting a timeout, not only poll. Search for ROUND_CEILING in the code to find them. But functions accepting time like clock_settime() must continue to use ROUND_CEILING. Does someone want to work on such patch? The new rounding mode must be test in test_time, you should just add the new mode at the top once if I recall correctly. --- select.poll currently uses ROUND_CEILING: round towards +infinity. It looks like you would prefer ROUND_UP: round away from zero, right? In the history of CPython, the rounding mode of functions accepting time, timeout, duration, etc. changed many times, mostly because the C code changed in subtle ways to fix different bugs. I mean that the exact rounding mode wasn't really chosen on purpose. I'm now trying to get better defined rouding modes in pytime, but it seems poll uses the wrong one. https://haypo.github.io/pytime.html I think that my problem is that I wanted to reuse the same rounding modes for different use cases. It seems like clocks need ROUND_CEILING but timeouts need ROUND_UP. Well, in the case of poll, the previous code before moving to pytime was using ceil() which uses ROUND_CEILING. |
|
|
| msg304431 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-15 11:20 |
| This particular issue can be fixed by adding the condition (PyFloat_Check(timeout_obj) && PyFloat_AsDouble(timeout_obj) < 0). The problem is only with writing good test for infinity timeout. This test could be also used for testing . |
|
|
| msg304439 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2017-10-15 14:13 |
| I have added a Pull Request fixing this issue. The current implementation is checking in the syscall if the value for ms before rounding was negative. To test for this, I call poll.poll in a thread and check that the thread is alive after a join with timeout (so this means it has blocked). Then to clean up, I write to a pipe and therefore it unblocks. The implementation is available in the PR: https://github.com/python/cpython/pull/4003 |
|
|
| msg304440 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-15 14:32 |
| Thank you Pablo. The initial test leaked a thread, now it is much better. Could you please make the test testing different timeout values: None, -1, -1.0, -0.1, -1e-100? |
|
|
| msg304445 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2017-10-15 19:18 |
| I have updated both the test and the implementation to address your feedback. |
|
|
| msg304456 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-10-16 07:42 |
| Ok, so it seems like we need 3 rounding modes: * _PyTime_ROUND_FLOOR: read a clock, like datetime.datetime.now(). We need to round nanoseconds since datetime.datetime only supports 1 us resolution * _PyTime_ROUND_HALF_EVEN: "round from a Python float" like round(float), used by datetime.datetime.fromtimestamp() * _PyTime_ROUND_UP: round timeouts, socket.settimeout(), lock.acquire(timeout), poll(timeout), etc. _PyTime_ROUND_UP and _PyTime_ROUND_CEILING are the same for positive numbers, but using _PyTime_ROUND_CEILING causes this bug: values in ]-0.5; 0.0[ are rounding to zero which gives the wrong behaviour. It seems like _PyTime_ROUND_CEILING is not needed in Python currently. |
|
|
| msg304463 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-10-16 08:28 |
| > It seems like _PyTime_ROUND_CEILING is not needed in Python currently. Oops, my pending PR 3983 uses _PyTime_ROUND_CEILING :-) Please keep it. |
|
|
| msg304511 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-17 14:40 |
| New changeset 2c15b29aea5d6b9c61aa42d2c24a07ff1edb4b46 by Serhiy Storchaka (Pablo Galindo) in branch 'master': bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (#4003) https://github.com/python/cpython/commit/2c15b29aea5d6b9c61aa42d2c24a07ff1edb4b46 |
|
|
| msg304559 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-18 08:12 |
| New changeset 95602b368b87da3702a0f340ded2a23e823bb104 by Serhiy Storchaka (Pablo Galindo) in branch '3.6': [3.6] bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (GH-4003). (#4022) https://github.com/python/cpython/commit/95602b368b87da3702a0f340ded2a23e823bb104 |
|
|
| msg304562 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-18 08:28 |
| New changeset ed267e3305a54eddae8106bdaae2c62d4c3b7db6 by Serhiy Storchaka in branch '2.7': [2.7] bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (GH-4003). (#4031) https://github.com/python/cpython/commit/ed267e3305a54eddae8106bdaae2c62d4c3b7db6 |
|
|
| msg304564 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-18 08:34 |
| Thank you for your contribution Pablo. The issue is fixed in 3.6 and 3.7. It is hard to fix it in 2.7. |
|
|