From 0cdf5f42898350261c5ff65d96334e736130780f Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Sat, 30 Jun 2018 15:43:23 +0300 Subject: [PATCH] bpo-32568: make select.epoll() and its docs consistent (#7840) * `flags` is indeed deprecated, but there is a validation on its value for backwards compatibility reasons. This adds mention of this in the docs. * The docs say that `sizehint` is deprecated and ignored, but it is still used when `epoll_create1()` is unavailable. This adds mention of this in the docs. * `sizehint=-1` is acceptable again, and is replaced with `FD_SETSIZE-1`. This is needed to have a default value available at the Python level, since `FD_SETSIZE` is not exposed to Python. (see: bpo-31938) * Reject `sizehint=0` since it is invalid to pass on to `epoll_create()`. The relevant tests have also been updated. --- Doc/library/select.rst | 11 +++++++- Lib/test/test_epoll.py | 25 ++++++++++++------- .../2018-06-21-09-33-02.bpo-32568.f_meGY.rst | 2 ++ Modules/selectmodule.c | 10 +++++--- 4 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-06-21-09-33-02.bpo-32568.f_meGY.rst diff --git a/Doc/library/select.rst b/Doc/library/select.rst index e252e7adb92..5ffb21583fa 100644 --- a/Doc/library/select.rst +++ b/Doc/library/select.rst @@ -57,7 +57,16 @@ The module defines the following: (Only supported on Linux 2.5.44 and newer.) Return an edge polling object, which can be used as Edge or Level Triggered interface for I/O - events. *sizehint* and *flags* are deprecated and completely ignored. + events. + + *sizehint* informs epoll about the expected number of events to be + registered. It must be positive, or `-1` to use the default. It is only + used on older systems where :c:func:`epoll_create1` is not available; + otherwise it has no effect (though its value is still checked). + + *flags* is deprecated and completely ignored. However, when supplied, its + value must be ``0`` or ``select.EPOLL_CLOEXEC``, otherwise ``OSError`` is + raised. See the :ref:`epoll-objects` section below for the methods supported by epolling objects. diff --git a/Lib/test/test_epoll.py b/Lib/test/test_epoll.py index 549e0f71216..efb54f42deb 100644 --- a/Lib/test/test_epoll.py +++ b/Lib/test/test_epoll.py @@ -74,11 +74,11 @@ class TestEPoll(unittest.TestCase): ep.close() self.assertTrue(ep.closed) self.assertRaises(ValueError, ep.fileno) + if hasattr(select, "EPOLL_CLOEXEC"): - select.epoll(select.EPOLL_CLOEXEC).close() + select.epoll(-1, select.EPOLL_CLOEXEC).close() select.epoll(flags=select.EPOLL_CLOEXEC).close() select.epoll(flags=0).close() - self.assertRaises(OSError, select.epoll, flags=12356) def test_badcreate(self): self.assertRaises(TypeError, select.epoll, 1, 2, 3) @@ -88,6 +88,13 @@ class TestEPoll(unittest.TestCase): self.assertRaises(TypeError, select.epoll, ['foo']) self.assertRaises(TypeError, select.epoll, {}) + self.assertRaises(ValueError, select.epoll, 0) + self.assertRaises(ValueError, select.epoll, -2) + self.assertRaises(ValueError, select.epoll, sizehint=-2) + + if hasattr(select, "EPOLL_CLOEXEC"): + self.assertRaises(OSError, select.epoll, flags=12356) + def test_context_manager(self): with select.epoll(16) as ep: self.assertGreater(ep.fileno(), 0) @@ -117,19 +124,19 @@ class TestEPoll(unittest.TestCase): try: # TypeError: argument must be an int, or have a fileno() method. self.assertRaises(TypeError, ep.register, object(), - select.EPOLLIN | select.EPOLLOUT) + select.EPOLLIN | select.EPOLLOUT) self.assertRaises(TypeError, ep.register, None, - select.EPOLLIN | select.EPOLLOUT) + select.EPOLLIN | select.EPOLLOUT) # ValueError: file descriptor cannot be a negative integer (-1) self.assertRaises(ValueError, ep.register, -1, - select.EPOLLIN | select.EPOLLOUT) + select.EPOLLIN | select.EPOLLOUT) # OSError: [Errno 9] Bad file descriptor self.assertRaises(OSError, ep.register, 10000, - select.EPOLLIN | select.EPOLLOUT) + select.EPOLLIN | select.EPOLLOUT) # registering twice also raises an exception ep.register(server, select.EPOLLIN | select.EPOLLOUT) self.assertRaises(OSError, ep.register, server, - select.EPOLLIN | select.EPOLLOUT) + select.EPOLLIN | select.EPOLLOUT) finally: ep.close() @@ -160,9 +167,9 @@ class TestEPoll(unittest.TestCase): ep = select.epoll(16) ep.register(server.fileno(), - select.EPOLLIN | select.EPOLLOUT | select.EPOLLET) + select.EPOLLIN | select.EPOLLOUT | select.EPOLLET) ep.register(client.fileno(), - select.EPOLLIN | select.EPOLLOUT | select.EPOLLET) + select.EPOLLIN | select.EPOLLOUT | select.EPOLLET) now = time.monotonic() events = ep.poll(1, 4) diff --git a/Misc/NEWS.d/next/Library/2018-06-21-09-33-02.bpo-32568.f_meGY.rst b/Misc/NEWS.d/next/Library/2018-06-21-09-33-02.bpo-32568.f_meGY.rst new file mode 100644 index 00000000000..3ade8cf6f3d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-06-21-09-33-02.bpo-32568.f_meGY.rst @@ -0,0 +1,2 @@ +Make select.epoll() and its documentation consistent regarding *sizehint* and +*flags*. diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index a366d1b88b6..6976fb5139a 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -1297,14 +1297,17 @@ newPyEpoll_Object(PyTypeObject *type, int sizehint, SOCKET fd) static PyObject * pyepoll_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { - int flags = 0, sizehint = FD_SETSIZE - 1; + int flags = 0, sizehint = -1; static char *kwlist[] = {"sizehint", "flags", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwds, "|ii:epoll", kwlist, &sizehint, &flags)) return NULL; - if (sizehint < 0) { - PyErr_SetString(PyExc_ValueError, "negative sizehint"); + if (sizehint == -1) { + sizehint = FD_SETSIZE - 1; + } + else if (sizehint <= 0) { + PyErr_SetString(PyExc_ValueError, "sizehint must be positive or -1"); return NULL; } @@ -1314,7 +1317,6 @@ pyepoll_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; } #endif - return newPyEpoll_Object(type, sizehint, -1); }