https://github.com/python/cpython/commit/44937d11a6a045a624918db78aa36e715ffabcd4
commit: 44937d11a6a045a624918db78aa36e715ffabcd4
branch: main
author: Sam Gross <colesb...@gmail.com>
committer: colesbury <colesb...@gmail.com>
date: 2024-07-11T10:21:09-04:00
summary:

gh-121592: Make select.poll() and related objects thread-safe (#121594)

This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(gh-121544).

files:
M Modules/clinic/selectmodule.c.h
M Modules/selectmodule.c

diff --git a/Modules/clinic/selectmodule.c.h b/Modules/clinic/selectmodule.c.h
index dc7d3fb814396d..0ccbf63b688f1b 100644
--- a/Modules/clinic/selectmodule.c.h
+++ b/Modules/clinic/selectmodule.c.h
@@ -6,6 +6,7 @@ preserve
 #  include "pycore_gc.h"          // PyGC_Head
 #  include "pycore_runtime.h"     // _Py_ID()
 #endif
+#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION()
 #include "pycore_long.h"          // _PyLong_UnsignedShort_Converter()
 #include "pycore_modsupport.h"    // _PyArg_CheckPositional()
 
@@ -110,7 +111,9 @@ select_poll_register(pollObject *self, PyObject *const 
*args, Py_ssize_t nargs)
         goto exit;
     }
 skip_optional:
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_poll_register_impl(self, fd, eventmask);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -155,7 +158,9 @@ select_poll_modify(pollObject *self, PyObject *const *args, 
Py_ssize_t nargs)
     if (!_PyLong_UnsignedShort_Converter(args[1], &eventmask)) {
         goto exit;
     }
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_poll_modify_impl(self, fd, eventmask);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -187,7 +192,9 @@ select_poll_unregister(pollObject *self, PyObject *arg)
     if (fd < 0) {
         goto exit;
     }
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_poll_unregister_impl(self, fd);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -230,7 +237,9 @@ select_poll_poll(pollObject *self, PyObject *const *args, 
Py_ssize_t nargs)
     }
     timeout_obj = args[0];
 skip_optional:
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_poll_poll_impl(self, timeout_obj);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -281,7 +290,9 @@ select_devpoll_register(devpollObject *self, PyObject 
*const *args, Py_ssize_t n
         goto exit;
     }
 skip_optional:
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_devpoll_register_impl(self, fd, eventmask);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -332,7 +343,9 @@ select_devpoll_modify(devpollObject *self, PyObject *const 
*args, Py_ssize_t nar
         goto exit;
     }
 skip_optional:
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_devpoll_modify_impl(self, fd, eventmask);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -364,7 +377,9 @@ select_devpoll_unregister(devpollObject *self, PyObject 
*arg)
     if (fd < 0) {
         goto exit;
     }
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_devpoll_unregister_impl(self, fd);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -407,7 +422,9 @@ select_devpoll_poll(devpollObject *self, PyObject *const 
*args, Py_ssize_t nargs
     }
     timeout_obj = args[0];
 skip_optional:
+    Py_BEGIN_CRITICAL_SECTION(self);
     return_value = select_devpoll_poll_impl(self, timeout_obj);
+    Py_END_CRITICAL_SECTION();
 
 exit:
     return return_value;
@@ -434,7 +451,13 @@ select_devpoll_close_impl(devpollObject *self);
 static PyObject *
 select_devpoll_close(devpollObject *self, PyObject *Py_UNUSED(ignored))
 {
-    return select_devpoll_close_impl(self);
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = select_devpoll_close_impl(self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
 }
 
 #endif /* (defined(HAVE_POLL) && !defined(HAVE_BROKEN_POLL)) && 
defined(HAVE_SYS_DEVPOLL_H) */
@@ -456,7 +479,13 @@ select_devpoll_fileno_impl(devpollObject *self);
 static PyObject *
 select_devpoll_fileno(devpollObject *self, PyObject *Py_UNUSED(ignored))
 {
-    return select_devpoll_fileno_impl(self);
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = select_devpoll_fileno_impl(self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
 }
 
 #endif /* (defined(HAVE_POLL) && !defined(HAVE_BROKEN_POLL)) && 
defined(HAVE_SYS_DEVPOLL_H) */
@@ -615,7 +644,13 @@ select_epoll_close_impl(pyEpoll_Object *self);
 static PyObject *
 select_epoll_close(pyEpoll_Object *self, PyObject *Py_UNUSED(ignored))
 {
-    return select_epoll_close_impl(self);
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = select_epoll_close_impl(self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
 }
 
 #endif /* defined(HAVE_EPOLL) */
@@ -1108,7 +1143,13 @@ select_kqueue_close_impl(kqueue_queue_Object *self);
 static PyObject *
 select_kqueue_close(kqueue_queue_Object *self, PyObject *Py_UNUSED(ignored))
 {
-    return select_kqueue_close_impl(self);
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = select_kqueue_close_impl(self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
 }
 
 #endif /* defined(HAVE_KQUEUE) */
@@ -1319,4 +1360,4 @@ select_kqueue_control(kqueue_queue_Object *self, PyObject 
*const *args, Py_ssize
 #ifndef SELECT_KQUEUE_CONTROL_METHODDEF
     #define SELECT_KQUEUE_CONTROL_METHODDEF
 #endif /* !defined(SELECT_KQUEUE_CONTROL_METHODDEF) */
-/*[clinic end generated code: output=4fc17ae9b6cfdc86 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=f31e724f492225b1 input=a9049054013a1b77]*/
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
index 3eaee22c652c28..0a5b5a703a5aa1 100644
--- a/Modules/selectmodule.c
+++ b/Modules/selectmodule.c
@@ -473,6 +473,7 @@ update_ufd_array(pollObject *self)
 }
 
 /*[clinic input]
+@critical_section
 select.poll.register
 
     fd: fildes
@@ -486,7 +487,7 @@ Register a file descriptor with the polling object.
 
 static PyObject *
 select_poll_register_impl(pollObject *self, int fd, unsigned short eventmask)
-/*[clinic end generated code: output=0dc7173c800a4a65 input=34e16cfb28d3c900]*/
+/*[clinic end generated code: output=0dc7173c800a4a65 input=c475e029ce6c2830]*/
 {
     PyObject *key, *value;
     int err;
@@ -514,6 +515,7 @@ select_poll_register_impl(pollObject *self, int fd, 
unsigned short eventmask)
 
 
 /*[clinic input]
+@critical_section
 select.poll.modify
 
     fd: fildes
@@ -528,7 +530,7 @@ Modify an already registered file descriptor.
 
 static PyObject *
 select_poll_modify_impl(pollObject *self, int fd, unsigned short eventmask)
-/*[clinic end generated code: output=1a7b88bf079eff17 input=a8e383df075c32cf]*/
+/*[clinic end generated code: output=1a7b88bf079eff17 input=38c9db5346711872]*/
 {
     PyObject *key, *value;
     int err;
@@ -566,6 +568,7 @@ select_poll_modify_impl(pollObject *self, int fd, unsigned 
short eventmask)
 
 
 /*[clinic input]
+@critical_section
 select.poll.unregister
 
     fd: fildes
@@ -576,7 +579,7 @@ Remove a file descriptor being tracked by the polling 
object.
 
 static PyObject *
 select_poll_unregister_impl(pollObject *self, int fd)
-/*[clinic end generated code: output=8c9f42e75e7d291b input=4b4fccc1040e79cb]*/
+/*[clinic end generated code: output=8c9f42e75e7d291b input=ae6315d7f5243704]*/
 {
     PyObject *key;
 
@@ -599,6 +602,7 @@ select_poll_unregister_impl(pollObject *self, int fd)
 }
 
 /*[clinic input]
+@critical_section
 select.poll.poll
 
     timeout as timeout_obj: object = None
@@ -614,7 +618,7 @@ report, as a list of (fd, event) 2-tuples.
 
 static PyObject *
 select_poll_poll_impl(pollObject *self, PyObject *timeout_obj)
-/*[clinic end generated code: output=876e837d193ed7e4 input=c2f6953ec45e5622]*/
+/*[clinic end generated code: output=876e837d193ed7e4 input=54310631457efdec]*/
 {
     PyObject *result_list = NULL;
     int poll_result, i, j;
@@ -857,6 +861,7 @@ internal_devpoll_register(devpollObject *self, int fd,
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.register
 
     fd: fildes
@@ -872,12 +877,13 @@ Register a file descriptor with the polling object.
 static PyObject *
 select_devpoll_register_impl(devpollObject *self, int fd,
                              unsigned short eventmask)
-/*[clinic end generated code: output=6e07fe8b74abba0c input=22006fabe9567522]*/
+/*[clinic end generated code: output=6e07fe8b74abba0c input=8d48bd2653a61c42]*/
 {
     return internal_devpoll_register(self, fd, eventmask, 0);
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.modify
 
     fd: fildes
@@ -893,12 +899,13 @@ Modify a possible already registered file descriptor.
 static PyObject *
 select_devpoll_modify_impl(devpollObject *self, int fd,
                            unsigned short eventmask)
-/*[clinic end generated code: output=bc2e6d23aaff98b4 input=09fa335db7cdc09e]*/
+/*[clinic end generated code: output=bc2e6d23aaff98b4 input=773b37e9abca2460]*/
 {
     return internal_devpoll_register(self, fd, eventmask, 1);
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.unregister
 
     fd: fildes
@@ -909,7 +916,7 @@ Remove a file descriptor being tracked by the polling 
object.
 
 static PyObject *
 select_devpoll_unregister_impl(devpollObject *self, int fd)
-/*[clinic end generated code: output=95519ffa0c7d43fe input=b4ea42a4442fd467]*/
+/*[clinic end generated code: output=95519ffa0c7d43fe input=6052d368368d4d05]*/
 {
     if (self->fd_devpoll < 0)
         return devpoll_err_closed();
@@ -926,6 +933,7 @@ select_devpoll_unregister_impl(devpollObject *self, int fd)
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.poll
     timeout as timeout_obj: object = None
       The maximum time to wait in milliseconds, or else None (or a negative
@@ -940,7 +948,7 @@ report, as a list of (fd, event) 2-tuples.
 
 static PyObject *
 select_devpoll_poll_impl(devpollObject *self, PyObject *timeout_obj)
-/*[clinic end generated code: output=2654e5457cca0b3c input=3c3f0a355ec2bedb]*/
+/*[clinic end generated code: output=2654e5457cca0b3c input=fe7a3f6dcbc118c5]*/
 {
     struct dvpoll dvp;
     PyObject *result_list = NULL;
@@ -1059,6 +1067,7 @@ devpoll_internal_close(devpollObject *self)
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.close
 
 Close the devpoll file descriptor.
@@ -1068,7 +1077,7 @@ Further operations on the devpoll object will raise an 
exception.
 
 static PyObject *
 select_devpoll_close_impl(devpollObject *self)
-/*[clinic end generated code: output=26b355bd6429f21b input=6273c30f5560a99b]*/
+/*[clinic end generated code: output=26b355bd6429f21b input=408fde21a377ccfb]*/
 {
     errno = devpoll_internal_close(self);
     if (errno < 0) {
@@ -1088,6 +1097,7 @@ devpoll_get_closed(devpollObject *self, void 
*Py_UNUSED(ignored))
 }
 
 /*[clinic input]
+@critical_section
 select.devpoll.fileno
 
 Return the file descriptor.
@@ -1095,7 +1105,7 @@ Return the file descriptor.
 
 static PyObject *
 select_devpoll_fileno_impl(devpollObject *self)
-/*[clinic end generated code: output=26920929f8d292f4 input=ef15331ebde6c368]*/
+/*[clinic end generated code: output=26920929f8d292f4 input=8c9db2efa1ade538]*/
 {
     if (self->fd_devpoll < 0)
         return devpoll_err_closed();
@@ -1378,6 +1388,7 @@ pyepoll_dealloc(pyEpoll_Object *self)
 }
 
 /*[clinic input]
+@critical_section
 select.epoll.close
 
 Close the epoll control file descriptor.
@@ -1387,7 +1398,7 @@ Further operations on the epoll object will raise an 
exception.
 
 static PyObject *
 select_epoll_close_impl(pyEpoll_Object *self)
-/*[clinic end generated code: output=ee2144c446a1a435 input=ca6c66ba5a736bfd]*/
+/*[clinic end generated code: output=ee2144c446a1a435 input=f626a769192e1dbe]*/
 {
     errno = pyepoll_internal_close(self);
     if (errno < 0) {
@@ -2023,10 +2034,8 @@ kqueue_tracking_init(PyObject *module) {
 }
 
 static int
-kqueue_tracking_add(_selectstate *state, kqueue_queue_Object *self) {
-    if (!state->kqueue_tracking_initialized) {
-        kqueue_tracking_init(PyType_GetModule(Py_TYPE(self)));
-    }
+kqueue_tracking_add_lock_held(_selectstate *state, kqueue_queue_Object *self)
+{
     assert(self->kqfd >= 0);
     _kqueue_list_item *item = PyMem_New(_kqueue_list_item, 1);
     if (item == NULL) {
@@ -2039,8 +2048,23 @@ kqueue_tracking_add(_selectstate *state, 
kqueue_queue_Object *self) {
     return 0;
 }
 
+static int
+kqueue_tracking_add(_selectstate *state, kqueue_queue_Object *self)
+{
+    int ret;
+    PyObject *module = PyType_GetModule(Py_TYPE(self));
+    Py_BEGIN_CRITICAL_SECTION(module);
+    if (!state->kqueue_tracking_initialized) {
+        kqueue_tracking_init(module);
+    }
+    ret = kqueue_tracking_add_lock_held(state, self);
+    Py_END_CRITICAL_SECTION();
+    return ret;
+}
+
 static void
-kqueue_tracking_remove(_selectstate *state, kqueue_queue_Object *self) {
+kqueue_tracking_remove_lock_held(_selectstate *state, kqueue_queue_Object 
*self)
+{
     _kqueue_list *listptr = &state->kqueue_open_list;
     while (*listptr != NULL) {
         _kqueue_list_item *item = *listptr;
@@ -2056,6 +2080,14 @@ kqueue_tracking_remove(_selectstate *state, 
kqueue_queue_Object *self) {
     assert(0);
 }
 
+static void
+kqueue_tracking_remove(_selectstate *state, kqueue_queue_Object *self)
+{
+    Py_BEGIN_CRITICAL_SECTION(PyType_GetModule(Py_TYPE(self)));
+    kqueue_tracking_remove_lock_held(state, self);
+    Py_END_CRITICAL_SECTION();
+}
+
 static int
 kqueue_queue_internal_close(kqueue_queue_Object *self)
 {
@@ -2150,6 +2182,7 @@ kqueue_queue_finalize(kqueue_queue_Object *self)
 }
 
 /*[clinic input]
+@critical_section
 select.kqueue.close
 
 Close the kqueue control file descriptor.
@@ -2159,7 +2192,7 @@ Further operations on the kqueue object will raise an 
exception.
 
 static PyObject *
 select_kqueue_close_impl(kqueue_queue_Object *self)
-/*[clinic end generated code: output=d1c7df0b407a4bc1 input=0b12d95430e0634c]*/
+/*[clinic end generated code: output=d1c7df0b407a4bc1 input=6d763c858b17b690]*/
 {
     errno = kqueue_queue_internal_close(self);
     if (errno < 0) {

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com

Reply via email to