diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst index 836aa91bb0885b1..99fed7008f5b8c7 100644 --- a/Doc/library/socket.rst +++ b/Doc/library/socket.rst @@ -2094,6 +2094,24 @@ Socket Objects For further information, please consult the :ref:`notes on socket timeouts `. + .. note:: + + Do not call :meth:`setblocking` or :meth:`settimeout` while another + thread is performing an operation on the same socket. Changing the + blocking mode concurrently with operations such as :meth:`connect`, + :meth:`accept`, :meth:`recv`, or :meth:`send` can cause the operation + to block unexpectedly or to fail with a non-blocking error. + + Applications must use external synchronization when changing a + socket's blocking mode or timeout. This also applies to socket + objects and file descriptors that share the same underlying socket, + since the blocking mode is shared at the operating-system level. + + Reading a socket's current timeout or blocking mode with + :meth:`gettimeout` or :meth:`getblocking` remains safe to do + concurrently, only *changing* it during an operation requires the + synchronization described above. + .. versionchanged:: 3.7 The method no longer toggles :const:`SOCK_NONBLOCK` flag on :attr:`socket.type`. diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index d8ec306a0dae3fc..7c622a6eaeeea1b 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -53,6 +53,10 @@ extern "C" { _Py_atomic_load_uint64_acquire(&value) #define FT_ATOMIC_LOAD_UINT64_RELAXED(value) \ _Py_atomic_load_uint64_relaxed(&value) +#define FT_ATOMIC_LOAD_INT64_ACQUIRE(value) \ + ((int64_t)_Py_atomic_load_uint64_acquire((const uint64_t *)&value)) +#define FT_ATOMIC_STORE_INT64_RELEASE(value, new_value) \ + _Py_atomic_store_uint64_release((uint64_t *)&value, (uint64_t)new_value) #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) \ _Py_atomic_load_ulong_relaxed(&value) #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \ @@ -159,6 +163,8 @@ extern "C" { #define FT_ATOMIC_LOAD_UINT32_RELAXED(value) value #define FT_ATOMIC_LOAD_UINT64_ACQUIRE(value) value #define FT_ATOMIC_LOAD_UINT64_RELAXED(value) value +#define FT_ATOMIC_LOAD_INT64_ACQUIRE(value) value +#define FT_ATOMIC_STORE_INT64_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) value #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value diff --git a/Lib/test/test_free_threading/test_socket.py b/Lib/test/test_free_threading/test_socket.py new file mode 100644 index 000000000000000..622e94337a678a9 --- /dev/null +++ b/Lib/test/test_free_threading/test_socket.py @@ -0,0 +1,46 @@ +import socket +import unittest + +from test import support +from test.support import threading_helper + +N = 200 if support.check_sanitizer(thread=True) else 1000 + + +@threading_helper.requires_working_threading() +class TestSocketTimeoutRaces(unittest.TestCase): + # gh-153852: the socket timeout was read/written non-atomically, so reading + # it with gettimeout()/getblocking() raced with settimeout()/setblocking() + # on the same socket. + + def new_socket(self): + s = socket.socket() + self.addCleanup(s.close) + return s + + def check_race(self, read, write): + def reader(): + for _ in range(N): + read() + + def writer(): + for _ in range(N): + write() + + threading_helper.run_concurrently([reader, writer]) + + def test_gettimeout_vs_setblocking(self): + s = self.new_socket() + self.check_race(s.gettimeout, lambda: s.setblocking(False)) + + def test_gettimeout_vs_settimeout(self): + s = self.new_socket() + self.check_race(s.gettimeout, lambda: s.settimeout(1.0)) + + def test_getblocking_vs_settimeout(self): + s = self.new_socket() + self.check_race(s.getblocking, lambda: s.settimeout(0)) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-07-18-15-30-56.gh-issue-153935.xpkjEc.rst b/Misc/NEWS.d/next/Library/2026-07-18-15-30-56.gh-issue-153935.xpkjEc.rst new file mode 100644 index 000000000000000..8fd42b20ce97d62 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-18-15-30-56.gh-issue-153935.xpkjEc.rst @@ -0,0 +1 @@ +Fix a data race on a socket's timeout in the :term:`free-threaded build`. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 084c2dbcff066e5..bff20828daf7503 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -110,6 +110,7 @@ Local naming conventions: #include "pycore_fileutils.h" // _Py_set_inheritable() #include "pycore_moduleobject.h" // _PyModule_GetState #include "pycore_object.h" // _PyObject_VisitType() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT64_ACQUIRE() #include "pycore_time.h" // _PyTime_AsMilliseconds() #include "pycore_tuple.h" // _PyTuple_FromPairSteal #include "pycore_pystate.h" // _Py_AssertHoldsTstate() @@ -591,6 +592,12 @@ get_sock_fd(PySocketSockObject *s) #endif } +static inline PyTime_t +get_sock_timeout(PySocketSockObject *s) +{ + return FT_ATOMIC_LOAD_INT64_ACQUIRE(s->sock_timeout); +} + #define _PySocketSockObject_CAST(op) ((PySocketSockObject *)(op)) static inline socket_state * @@ -681,7 +688,7 @@ class _socket.socket "PySocketSockObject *" "clinic_state()->sock_type" #else /* If there's no timeout left, we don't have to call select, so it's a safe, * little white lie. */ -#define IS_SELECTABLE(s) (_PyIsSelectable_fd((s)->sock_fd) || (s)->sock_timeout <= 0) +#define IS_SELECTABLE(s) (_PyIsSelectable_fd((s)->sock_fd) || get_sock_timeout(s) <= 0) #endif // SCM_RIGHTS, sendmsg(), recvmsg() and sethostname() don't work properly on @@ -1068,7 +1075,7 @@ sock_call_ex(PySocketSockObject *s, /* retry sock_func() */ } - if (s->sock_timeout > 0 + if (get_sock_timeout(s) > 0 && (CHECK_ERRNO(EWOULDBLOCK) || CHECK_ERRNO(EAGAIN))) { /* False positive: sock_func() failed with EWOULDBLOCK or EAGAIN. @@ -1094,7 +1101,8 @@ sock_call(PySocketSockObject *s, int (*func) (PySocketSockObject *s, void *data), void *data) { - return sock_call_ex(s, writing, func, data, 0, NULL, s->sock_timeout); + return sock_call_ex(s, writing, func, data, 0, NULL, + get_sock_timeout(s)); } @@ -1158,6 +1166,9 @@ new_sockobject(socket_state *state, SOCKET_T fd, int family, int type, if (s == NULL) { return NULL; } +#ifdef Py_GIL_DISABLED + s->sock_timeout_mutex = (PyMutex){0}; +#endif if (init_sockobject(state, s, fd, family, type, proto) == -1) { Py_DECREF(s); return NULL; @@ -3168,9 +3179,15 @@ sock_setblocking(PyObject *self, PyObject *arg) if (block < 0) return NULL; - PySocketSockObject *s = _PySocketSockObject_CAST(self); - s->sock_timeout = _PyTime_FromSeconds(block ? -1 : 0); - if (internal_setblocking(s, block) == -1) { + PySocketSockObject *s = _PySocketSockObject_CAST(self); + FT_MUTEX_LOCK(&s->sock_timeout_mutex); + int result = internal_setblocking(s, block); + if (result == 0) { + FT_ATOMIC_STORE_INT64_RELEASE( + s->sock_timeout, _PyTime_FromSeconds(block ? -1 : 0)); + } + FT_MUTEX_UNLOCK(&s->sock_timeout_mutex); + if (result < 0) { return NULL; } Py_RETURN_NONE; @@ -3190,8 +3207,8 @@ setblocking(False) is equivalent to settimeout(0.0)."); static PyObject * sock_getblocking(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PySocketSockObject *s = _PySocketSockObject_CAST(self); - if (s->sock_timeout) { + PySocketSockObject *s = _PySocketSockObject_CAST(self); + if (get_sock_timeout(s)) { Py_RETURN_TRUE; } else { @@ -3261,8 +3278,6 @@ sock_settimeout(PyObject *self, PyObject *arg) return NULL; PySocketSockObject *s = _PySocketSockObject_CAST(self); - s->sock_timeout = timeout; - int block = timeout < 0; /* Blocking mode for a Python socket object means that operations like :meth:`recv` or :meth:`sendall` will block the execution of @@ -3285,7 +3300,13 @@ sock_settimeout(PyObject *self, PyObject *arg) ``> 0`` ``True`` non-blocking */ - if (internal_setblocking(s, block) == -1) { + FT_MUTEX_LOCK(&s->sock_timeout_mutex); + int result = internal_setblocking(s, block); + if (result == 0) { + FT_ATOMIC_STORE_INT64_RELEASE(s->sock_timeout, timeout); + } + FT_MUTEX_UNLOCK(&s->sock_timeout_mutex); + if (result < 0) { return NULL; } Py_RETURN_NONE; @@ -3305,11 +3326,12 @@ static PyObject * sock_gettimeout_impl(PyObject *self, void *Py_UNUSED(ignored)) { PySocketSockObject *s = _PySocketSockObject_CAST(self); - if (s->sock_timeout < 0) { + PyTime_t sock_timeout = get_sock_timeout(s); + if (sock_timeout < 0) { Py_RETURN_NONE; } else { - double seconds = PyTime_AsSecondsDouble(s->sock_timeout); + double seconds = PyTime_AsSecondsDouble(sock_timeout); return PyFloat_FromDouble(seconds); } } @@ -3706,10 +3728,11 @@ internal_connect(PySocketSockObject *s, struct sockaddr *addr, int addrlen, If the socket is non-blocking, raise InterruptedError. The caller is responsible to wait until the connection completes, fails or timed out (it's the case in asyncio for example). */ - wait_connect = (s->sock_timeout != 0 && IS_SELECTABLE(s)); + wait_connect = (get_sock_timeout(s) != 0 && IS_SELECTABLE(s)); } else { - wait_connect = (s->sock_timeout > 0 && err == SOCK_INPROGRESS_ERR + wait_connect = (get_sock_timeout(s) > 0 + && err == SOCK_INPROGRESS_ERR && IS_SELECTABLE(s)); } @@ -3727,13 +3750,13 @@ internal_connect(PySocketSockObject *s, struct sockaddr *addr, int addrlen, if (raise) { /* socket.connect() raises an exception on error */ if (sock_call_ex(s, 1, sock_connect_impl, NULL, - 1, NULL, s->sock_timeout) < 0) + 1, NULL, get_sock_timeout(s)) < 0) return -1; } else { /* socket.connect_ex() returns the error code on error */ if (sock_call_ex(s, 1, sock_connect_impl, NULL, - 1, &err, s->sock_timeout) < 0) + 1, &err, get_sock_timeout(s)) < 0) return err; } return 0; @@ -4688,8 +4711,8 @@ _socket_socket_sendall_impl(PySocketSockObject *s, Py_buffer *pbuf, char *buf; Py_ssize_t len, n; struct sock_send ctx; - int has_timeout = (s->sock_timeout > 0); - PyTime_t timeout = s->sock_timeout; + PyTime_t timeout = get_sock_timeout(s); + int has_timeout = (timeout > 0); PyTime_t deadline = 0; int deadline_initialized = 0; PyObject *res = NULL; @@ -5604,6 +5627,9 @@ sock_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (new != NULL) { ((PySocketSockObject *)new)->sock_fd = INVALID_SOCKET; ((PySocketSockObject *)new)->sock_timeout = _PyTime_FromSeconds(-1); +#ifdef Py_GIL_DISABLED + ((PySocketSockObject *)new)->sock_timeout_mutex = (PyMutex){0}; +#endif ((PySocketSockObject *)new)->errorhandler = &set_error; #ifdef MS_WINDOWS ((PySocketSockObject *)new)->quickack = 0; diff --git a/Modules/socketmodule.h b/Modules/socketmodule.h index ac770889ae87f2c..935065fa8bb3990 100644 --- a/Modules/socketmodule.h +++ b/Modules/socketmodule.h @@ -346,6 +346,9 @@ typedef struct { sets a Python exception */ PyTime_t sock_timeout; /* Operation timeout in seconds; 0.0 means non-blocking */ +#ifdef Py_GIL_DISABLED + PyMutex sock_timeout_mutex; +#endif struct _socket_state *state; #ifdef MS_WINDOWS int quickack;