Skip to content

[WIP] bpo-28708: Add setsize keywoard-only parameter to select.select() #13845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[WIP] bpo-28708: Add setsize keywoard-only parameter to select.select() #13845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Doc/library/select.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ The module defines the following:
:ref:`kevent-objects` below for the methods supported by kevent objects.


.. function:: select(rlist, wlist, xlist[, timeout])
.. function:: select(rlist, wlist, xlist[, timeout], *, setsize=FD_SETSIZE)

This is a straightforward interface to the Unix :c:func:`select` system call.
The first three arguments are sequences of 'waitable objects': either
Expand Down Expand Up @@ -157,13 +157,22 @@ The module defines the following:
library, and does not handle file descriptors that don't originate from
WinSock.

.. versionchanged:: 3.9
Added *setsize* parameter.

.. versionchanged:: 3.5
The function is now retried with a recomputed timeout when interrupted by
a signal, except if the signal handler raises an exception (see
:pep:`475` for the rationale), instead of raising
:exc:`InterruptedError`.


.. attribute:: FD_SETSIZE

Set size used by :func:`select.select` by default.

.. versionadded:: 3.9

.. attribute:: PIPE_BUF

The minimum number of bytes which can be written without blocking to a pipe
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2019-06-05-18-05-58.bpo-28708.jgeS7X.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Add *setsize* keywoard-only parameter to :func:`select.select`. It becomes
possible to configure the set size used internally by :func:`select.select`
at runtime.
38 changes: 29 additions & 9 deletions Modules/clinic/selectmodule.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 44 additions & 40 deletions Modules/selectmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ typedef struct {
} pylist;

static void
reap_obj(pylist fd2obj[FD_SETSIZE + 1])
reap_obj(pylist *fd2obj, size_t setsize)
{
unsigned int i;
for (i = 0; i < (unsigned int)FD_SETSIZE + 1 && fd2obj[i].sentinel >= 0; i++) {
size_t i;
for (i = 0; i < setsize + 1 && fd2obj[i].sentinel >= 0; i++) {
Py_CLEAR(fd2obj[i].obj);
}
fd2obj[0].sentinel = -1;
Expand All @@ -108,10 +108,10 @@ reap_obj(pylist fd2obj[FD_SETSIZE + 1])
returns a number >= 0
*/
static int
seq2set(PyObject *seq, fd_set *set, pylist fd2obj[FD_SETSIZE + 1])
seq2set(PyObject *seq, fd_set *set, pylist *fd2obj, size_t setsize)
{
int max = -1;
unsigned int index = 0;
size_t index = 0;
Py_ssize_t i;
PyObject* fast_seq = NULL;
PyObject* o = NULL;
Expand Down Expand Up @@ -145,10 +145,17 @@ seq2set(PyObject *seq, fd_set *set, pylist fd2obj[FD_SETSIZE + 1])
if (v > max)
max = v;
#endif /* _MSC_VER */
if ((size_t)v >= setsize) {
PyErr_Format(PyExc_ValueError,
"file descriptor %i greater than "
"or equal to setsize=%zu",
v, setsize);
goto finally;
}
FD_SET(v, set);

/* add object and its file descriptor to the list */
if (index >= (unsigned int)FD_SETSIZE) {
if (index >= setsize) {
PyErr_SetString(PyExc_ValueError,
"too many file descriptors in select()");
goto finally;
Expand All @@ -169,13 +176,13 @@ seq2set(PyObject *seq, fd_set *set, pylist fd2obj[FD_SETSIZE + 1])

/* returns NULL and sets the Python exception if an error occurred */
static PyObject *
set2list(fd_set *set, pylist fd2obj[FD_SETSIZE + 1])
set2list(fd_set *set, pylist *fd2obj, size_t setsize)
{
int i, j, count=0;
size_t i, j, count=0;
PyObject *list, *o;
SOCKET fd;

for (j = 0; fd2obj[j].sentinel >= 0; j++) {
for (j = 0; j < setsize && fd2obj[j].sentinel >= 0; j++) {
if (FD_ISSET(fd2obj[j].fd, set))
count++;
}
Expand All @@ -184,7 +191,7 @@ set2list(fd_set *set, pylist fd2obj[FD_SETSIZE + 1])
return NULL;

i = 0;
for (j = 0; fd2obj[j].sentinel >= 0; j++) {
for (j = 0; j < setsize && fd2obj[j].sentinel >= 0; j++) {
fd = fd2obj[j].fd;
if (FD_ISSET(fd, set)) {
o = fd2obj[j].obj;
Expand Down Expand Up @@ -215,6 +222,8 @@ select.select
xlist: object
timeout as timeout_obj: object = None
/
*
setsize as setsize_int: int(c_default="FD_SETSIZE") = -1

Wait until one or more file descriptors are ready for some kind of I/O.

Expand Down Expand Up @@ -242,29 +251,26 @@ descriptors can be used.

static PyObject *
select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist,
PyObject *xlist, PyObject *timeout_obj)
/*[clinic end generated code: output=2b3cfa824f7ae4cf input=177e72184352df25]*/
PyObject *xlist, PyObject *timeout_obj, int setsize_int)
/*[clinic end generated code: output=9563bc11a5b35909 input=32c56b0582ddd47a]*/
{
#ifdef SELECT_USES_HEAP
size_t setsize;
pylist *rfd2obj, *wfd2obj, *efd2obj;
#else /* !SELECT_USES_HEAP */
/* XXX: All this should probably be implemented as follows:
* - find the highest descriptor we're interested in
* - add one
* - that's the size
* See: Stevens, APitUE, $12.5.1
*/
pylist rfd2obj[FD_SETSIZE + 1];
pylist wfd2obj[FD_SETSIZE + 1];
pylist efd2obj[FD_SETSIZE + 1];
#endif /* SELECT_USES_HEAP */
PyObject *ret = NULL;
fd_set ifdset, ofdset, efdset;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these three sets will need to be allocated differently. We'll probably need a different fd_set structure something like:

typedef struct fd_set_2
{
    u_int fd_count
    SOCKET *fd_array;
} fd_set_2;

and then dynamically allocate the fd_array to be a SOCKET* array of size set_size.
Knowing that the select implementation only looks at the fd_count for the size of the array (and doesn't do any boundary checking), we can cast those sets to fd_set and then pass them to the select call.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that struct type isn't going to work, because that adds a level of indirection that the select implementation doesn't expect.
We'll need to allocate a continuous area of memory of size (sizeof(u_int) + setsize*sizeof(SOCKET))

struct timeval tv, *tvp;
int imax, omax, emax, max;
int n;
_PyTime_t timeout, deadline = 0;

if (setsize_int <= 0) {
PyErr_SetString(PyExc_ValueError, "setsize must be greater than 0");
return NULL;
}
else {
setsize = (size_t)setsize_int;
}

if (timeout_obj == Py_None)
tvp = (struct timeval *)NULL;
else {
Expand All @@ -286,30 +292,28 @@ select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist,
tvp = &tv;
}

#ifdef SELECT_USES_HEAP
/* Allocate memory for the lists */
rfd2obj = PyMem_NEW(pylist, FD_SETSIZE + 1);
wfd2obj = PyMem_NEW(pylist, FD_SETSIZE + 1);
efd2obj = PyMem_NEW(pylist, FD_SETSIZE + 1);
rfd2obj = PyMem_NEW(pylist, setsize + 1);
wfd2obj = PyMem_NEW(pylist, setsize + 1);
efd2obj = PyMem_NEW(pylist, setsize + 1);
if (rfd2obj == NULL || wfd2obj == NULL || efd2obj == NULL) {
if (rfd2obj) PyMem_DEL(rfd2obj);
if (wfd2obj) PyMem_DEL(wfd2obj);
if (efd2obj) PyMem_DEL(efd2obj);
return PyErr_NoMemory();
}
#endif /* SELECT_USES_HEAP */

/* Convert sequences to fd_sets, and get maximum fd number
* propagates the Python exception set in seq2set()
*/
rfd2obj[0].sentinel = -1;
wfd2obj[0].sentinel = -1;
efd2obj[0].sentinel = -1;
if ((imax = seq2set(rlist, &ifdset, rfd2obj)) < 0)
if ((imax = seq2set(rlist, &ifdset, rfd2obj, setsize)) < 0)
goto finally;
if ((omax = seq2set(wlist, &ofdset, wfd2obj)) < 0)
if ((omax = seq2set(wlist, &ofdset, wfd2obj, setsize)) < 0)
goto finally;
if ((emax = seq2set(xlist, &efdset, efd2obj)) < 0)
if ((emax = seq2set(xlist, &efdset, efd2obj, setsize)) < 0)
goto finally;

max = imax;
Expand Down Expand Up @@ -361,9 +365,9 @@ select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist,
convenient to test for this after all three calls... but
is that acceptable?
*/
rlist = set2list(&ifdset, rfd2obj);
wlist = set2list(&ofdset, wfd2obj);
xlist = set2list(&efdset, efd2obj);
rlist = set2list(&ifdset, rfd2obj, setsize);
wlist = set2list(&ofdset, wfd2obj, setsize);
xlist = set2list(&efdset, efd2obj, setsize);
if (PyErr_Occurred())
ret = NULL;
else
Expand All @@ -375,14 +379,12 @@ select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist,
}

finally:
reap_obj(rfd2obj);
reap_obj(wfd2obj);
reap_obj(efd2obj);
#ifdef SELECT_USES_HEAP
reap_obj(rfd2obj, setsize);
reap_obj(wfd2obj, setsize);
reap_obj(efd2obj, setsize);
PyMem_DEL(rfd2obj);
PyMem_DEL(wfd2obj);
PyMem_DEL(efd2obj);
#endif /* SELECT_USES_HEAP */
return ret;
}

Expand Down Expand Up @@ -2495,6 +2497,8 @@ PyInit_select(void)
if (m == NULL)
return NULL;

PyModule_AddIntConstant(m, "FD_SETSIZE", FD_SETSIZE);

Py_INCREF(PyExc_OSError);
PyModule_AddObject(m, "error", PyExc_OSError);

Expand Down