In select.poll.poll() ms can be 0 if timeout < 0

[serhiy-storchaka]

BPO	31786
Nosy	@vstinner, @berkerpeksag, @serhiy-storchaka, @pablogsal, @volans-
PRs	bpo-31786: Make select.poll.poll block if ms < 0 for every value of ms #4003 [3.6] bpo-31786: Make select.poll.poll block if ms < 0 for every value of ms (GH-4003) #4022 [2.7] bpo-31786: Add test for select.poll.poll() with infinite timeout. (GH-4003). #4031

^{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-10-18.08:34:19.835>
created_at = <Date 2017-10-14.08:07:08.015>
labels = ['extension-modules', 'type-bug', '3.7']
title = 'In select.poll.poll() ms can be 0 if timeout < 0'
updated_at = <Date 2017-10-18.08:34:19.834>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2017-10-18.08:34:19.834>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-10-18.08:34:19.835>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2017-10-14.08:07:08.015>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 31786
keywords = ['patch']
message_count = 12.0
messages = ['304386', '304427', '304431', '304439', '304440', '304445', '304456', '304463', '304511', '304559', '304562', '304564']
nosy_count = 5.0
nosy_names = ['vstinner', 'berker.peksag', 'serhiy.storchaka', 'pablogsal', 'Riccardo Coccioli']
pr_nums = ['4003', '4022', '4031']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31786'
versions = ['Python 3.6', 'Python 3.7']

[serhiy-storchaka]

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.

[vstinner]

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.

[serhiy-storchaka]

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

[pablogsal]

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:

#4003

[serhiy-storchaka]

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?

[pablogsal]

I have updated both the test and the implementation to address your feedback.

[vstinner]

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.

[vstinner]

It seems like _PyTime_ROUND_CEILING is not needed in Python currently.

Oops, my pending PR 3983 uses _PyTime_ROUND_CEILING :-) Please keep it.

[serhiy-storchaka]

New changeset 2c15b29 by Serhiy Storchaka (Pablo Galindo) in branch 'master':
bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (bpo-4003)
2c15b29

[serhiy-storchaka]

New changeset 95602b3 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). (bpo-4022)
95602b3

[serhiy-storchaka]

New changeset ed267e3 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). (bpo-4031)
ed267e3

[serhiy-storchaka]

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.