bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (#4003)
diff --git a/Include/pytime.h b/Include/pytime.h
index 488fdc4..9ec9b03 100644
--- a/Include/pytime.h
+++ b/Include/pytime.h
@@ -29,9 +29,20 @@
_PyTime_ROUND_CEILING=1,
/* Round to nearest with ties going to nearest even integer.
For example, used to round from a Python float. */
- _PyTime_ROUND_HALF_EVEN
+ _PyTime_ROUND_HALF_EVEN=2,
+ /* Round away from zero
+ For example, used for timeout. _PyTime_ROUND_CEILING rounds
+ -1e-9 to 0 milliseconds which causes bpo-31786 issue.
+ _PyTime_ROUND_UP rounds -1e-9 to -1 millisecond which keeps
+ the timeout sign as expected. select.poll(timeout) must block
+ for negative values." */
+ _PyTime_ROUND_UP=3,
+ /* _PyTime_ROUND_TIMEOUT (an alias for _PyTime_ROUND_UP) should be
+ used for timeouts. */
+ _PyTime_ROUND_TIMEOUT = _PyTime_ROUND_UP
} _PyTime_round_t;
+
/* Convert a time_t to a PyLong. */
PyAPI_FUNC(PyObject *) _PyLong_FromTime_t(
time_t sec);
diff --git a/Lib/test/test_poll.py b/Lib/test/test_poll.py
index 16c2d2e..028dd2d 100644
--- a/Lib/test/test_poll.py
+++ b/Lib/test/test_poll.py
@@ -204,6 +204,28 @@
os.write(w, b'spam')
t.join()
+ @unittest.skipUnless(threading, 'Threading required for this test.')
+ @reap_threads
+ def test_poll_blocks_with_negative_ms(self):
+ for timeout_ms in [None, -1, -1.0, -0.1, -1e-100]:
+ # Create two file descriptors. This will be used to unlock
+ # the blocking call to poll.poll inside the thread
+ r, w = os.pipe()
+ pollster = select.poll()
+ pollster.register(r, select.POLLIN)
+
+ poll_thread = threading.Thread(target=pollster.poll, args=(timeout_ms,))
+ poll_thread.start()
+ poll_thread.join(timeout=0.1)
+ self.assertTrue(poll_thread.is_alive())
+
+ # Write to the pipe so pollster.poll unblocks and the thread ends.
+ os.write(w, b'spam')
+ poll_thread.join()
+ self.assertFalse(poll_thread.is_alive())
+ os.close(r)
+ os.close(w)
+
def test_main():
run_unittest(PollTests)
diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index c92e66b..61dda09 100644
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -33,6 +33,8 @@
ROUND_CEILING = 1
# Round to nearest with ties going to nearest even integer
ROUND_HALF_EVEN = 2
+ # Round away from zero
+ ROUND_UP = 3
# Rounding modes supported by PyTime
ROUNDING_MODES = (
@@ -40,6 +42,7 @@
(_PyTime.ROUND_FLOOR, decimal.ROUND_FLOOR),
(_PyTime.ROUND_CEILING, decimal.ROUND_CEILING),
(_PyTime.ROUND_HALF_EVEN, decimal.ROUND_HALF_EVEN),
+ (_PyTime.ROUND_UP, decimal.ROUND_UP),
)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-15-23-44-57.bpo-31786.XwdEP4.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-15-23-44-57.bpo-31786.XwdEP4.rst
new file mode 100644
index 0000000..f0326e3
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-15-23-44-57.bpo-31786.XwdEP4.rst
@@ -0,0 +1,3 @@
+Fix timeout rounding in the select module to round correctly negative timeouts between -1.0 and 0.0.
+The functions now block waiting for events as expected. Previously, the call was incorrectly non-blocking.
+Patch by Pablo Galindo.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 6e31105..0e1e772 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -3012,7 +3012,8 @@
{
if (round != _PyTime_ROUND_FLOOR
&& round != _PyTime_ROUND_CEILING
- && round != _PyTime_ROUND_HALF_EVEN) {
+ && round != _PyTime_ROUND_HALF_EVEN
+ && round != _PyTime_ROUND_UP) {
PyErr_SetString(PyExc_ValueError, "invalid rounding");
return -1;
}
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
index 66d1a37..5305523 100644
--- a/Modules/selectmodule.c
+++ b/Modules/selectmodule.c
@@ -213,7 +213,7 @@
tvp = (struct timeval *)NULL;
else {
if (_PyTime_FromSecondsObject(&timeout, timeout_obj,
- _PyTime_ROUND_CEILING) < 0) {
+ _PyTime_ROUND_TIMEOUT) < 0) {
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
PyErr_SetString(PyExc_TypeError,
"timeout must be a float or None");
@@ -221,7 +221,7 @@
return NULL;
}
- if (_PyTime_AsTimeval(timeout, &tv, _PyTime_ROUND_CEILING) == -1)
+ if (_PyTime_AsTimeval(timeout, &tv, _PyTime_ROUND_TIMEOUT) == -1)
return NULL;
if (tv.tv_sec < 0) {
PyErr_SetString(PyExc_ValueError, "timeout must be non-negative");
@@ -540,7 +540,7 @@
}
else {
if (_PyTime_FromMillisecondsObject(&timeout, timeout_obj,
- _PyTime_ROUND_CEILING) < 0) {
+ _PyTime_ROUND_TIMEOUT) < 0) {
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
PyErr_SetString(PyExc_TypeError,
"timeout must be an integer or None");
@@ -548,7 +548,7 @@
return NULL;
}
- ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
+ ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_TIMEOUT);
if (ms < INT_MIN || ms > INT_MAX) {
PyErr_SetString(PyExc_OverflowError, "timeout is too large");
return NULL;
@@ -896,7 +896,7 @@
}
else {
if (_PyTime_FromMillisecondsObject(&timeout, timeout_obj,
- _PyTime_ROUND_CEILING) < 0) {
+ _PyTime_ROUND_TIMEOUT) < 0) {
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
PyErr_SetString(PyExc_TypeError,
"timeout must be an integer or None");
@@ -904,7 +904,7 @@
return NULL;
}
- ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
+ ms = _PyTime_AsMilliseconds(timeout, _PyTime_ROUND_TIMEOUT);
if (ms < -1 || ms > INT_MAX) {
PyErr_SetString(PyExc_OverflowError, "timeout is too large");
return NULL;
@@ -1513,7 +1513,7 @@
/* epoll_wait() has a resolution of 1 millisecond, round towards
infinity to wait at least timeout seconds. */
if (_PyTime_FromSecondsObject(&timeout, timeout_obj,
- _PyTime_ROUND_CEILING) < 0) {
+ _PyTime_ROUND_TIMEOUT) < 0) {
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
PyErr_SetString(PyExc_TypeError,
"timeout must be an integer or None");
@@ -2128,7 +2128,7 @@
}
else {
if (_PyTime_FromSecondsObject(&timeout,
- otimeout, _PyTime_ROUND_CEILING) < 0) {
+ otimeout, _PyTime_ROUND_TIMEOUT) < 0) {
PyErr_Format(PyExc_TypeError,
"timeout argument must be a number "
"or None, got %.200s",
diff --git a/Python/pytime.c b/Python/pytime.c
index 7b55b10..f19bb36 100644
--- a/Python/pytime.c
+++ b/Python/pytime.c
@@ -120,9 +120,13 @@
else if (round == _PyTime_ROUND_CEILING) {
d = ceil(d);
}
- else {
+ else if (round == _PyTime_ROUND_FLOOR) {
d = floor(d);
}
+ else {
+ assert(round == _PyTime_ROUND_UP);
+ d = (d >= 0.0) ? ceil(d) : floor(d);
+ }
return d;
}
@@ -427,7 +431,7 @@
return t / k;
}
}
- else {
+ else if (round == _PyTime_ROUND_FLOOR){
if (t >= 0) {
return t / k;
}
@@ -435,6 +439,15 @@
return (t - (k - 1)) / k;
}
}
+ else {
+ assert(round == _PyTime_ROUND_UP);
+ if (t >= 0) {
+ return (t + k - 1) / k;
+ }
+ else {
+ return (t - (k - 1)) / k;
+ }
+ }
}
_PyTime_t