[Python-checkins] bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (#4003)

Serhiy Storchaka webhook-mailer at python.org
Tue Oct 17 10:40:17 EDT 2017


https://github.com/python/cpython/commit/2c15b29aea5d6b9c61aa42d2c24a07ff1edb4b46
commit: 2c15b29aea5d6b9c61aa42d2c24a07ff1edb4b46
branch: master
author: Pablo Galindo <Pablogsal at gmail.com>
committer: Serhiy Storchaka <storchaka at gmail.com>
date: 2017-10-17T17:14:41+03:00
summary:

bpo-31786: Make functions in the select module blocking when timeout is a small negative value. (#4003)

files:
A Misc/NEWS.d/next/Core and Builtins/2017-10-15-23-44-57.bpo-31786.XwdEP4.rst
M Include/pytime.h
M Lib/test/test_poll.py
M Lib/test/test_time.py
M Modules/_testcapimodule.c
M Modules/selectmodule.c
M Python/pytime.c

diff --git a/Include/pytime.h b/Include/pytime.h
index 488fdc439b9..9ec9b03f19e 100644
--- a/Include/pytime.h
+++ b/Include/pytime.h
@@ -29,9 +29,20 @@ typedef enum {
     _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 16c2d2e123b..028dd2d08a3 100644
--- a/Lib/test/test_poll.py
+++ b/Lib/test/test_poll.py
@@ -204,6 +204,28 @@ def test_threaded_poll(self):
             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 c92e66bf49a..61dda09c184 100644
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -33,6 +33,8 @@ class _PyTime(enum.IntEnum):
     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 @@ class _PyTime(enum.IntEnum):
     (_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 00000000000..f0326e30ecd
--- /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 6e31105712f..0e1e772b5c1 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -3012,7 +3012,8 @@ check_time_rounding(int round)
 {
     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 66d1a37fb4a..530552324be 100644
--- a/Modules/selectmodule.c
+++ b/Modules/selectmodule.c
@@ -213,7 +213,7 @@ select_select(PyObject *self, PyObject *args)
         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 @@ select_select(PyObject *self, PyObject *args)
             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 @@ poll_poll(pollObject *self, PyObject *args)
     }
     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 @@ poll_poll(pollObject *self, PyObject *args)
             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 @@ devpoll_poll(devpollObject *self, PyObject *args)
     }
     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 @@ devpoll_poll(devpollObject *self, PyObject *args)
             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 @@ pyepoll_poll(pyEpoll_Object *self, PyObject *args, PyObject *kwds)
         /* 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 @@ kqueue_queue_control(kqueue_queue_Object *self, PyObject *args)
     }
     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 7b55b10d108..f19bb3673e4 100644
--- a/Python/pytime.c
+++ b/Python/pytime.c
@@ -120,9 +120,13 @@ _PyTime_Round(double x, _PyTime_round_t round)
     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 @@ _PyTime_Divide(const _PyTime_t t, const _PyTime_t k,
             return t / k;
         }
     }
-    else {
+    else if (round == _PyTime_ROUND_FLOOR){
         if (t >= 0) {
             return t / k;
         }
@@ -435,6 +439,15 @@ _PyTime_Divide(const _PyTime_t t, const _PyTime_t k,
             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



More information about the Python-checkins mailing list