From 188d175d6467d50a82c72bbaa2696d17bc5e439e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 18 Sep 2023 13:25:07 -0600 Subject: [PATCH 1/2] Expose PyThread_acquire_lock_timed_with_retries(). --- Include/internal/pycore_pythread.h | 15 +++++++++ Modules/_threadmodule.c | 52 ++---------------------------- Python/thread.c | 50 ++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/Include/internal/pycore_pythread.h b/Include/internal/pycore_pythread.h index f679c1bdb75499..607bd22a4a0a42 100644 --- a/Include/internal/pycore_pythread.h +++ b/Include/internal/pycore_pythread.h @@ -84,6 +84,21 @@ extern int _PyThread_at_fork_reinit(PyThread_type_lock *lock); #endif /* HAVE_FORK */ +// unset: -1 seconds, in nanoseconds +#define PyThread_UNSET_TIMEOUT ((_PyTime_t)(-1 * 1000 * 1000 * 1000)) + +/* Helper to acquire an interruptible lock with a timeout. If the lock acquire + * is interrupted, signal handlers are run, and if they raise an exception, + * PY_LOCK_INTR is returned. Otherwise, PY_LOCK_ACQUIRED or PY_LOCK_FAILURE + * are returned, depending on whether the lock can be acquired within the + * timeout. + */ +// Exported for the _xxinterpchannels module. +PyAPI_FUNC(PyLockStatus) PyThread_acquire_lock_timed_with_retries( + PyThread_type_lock, + PY_TIMEOUT_T microseconds); + + #ifdef __cplusplus } #endif diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 86bd560b92ba6b..7620511dd1d6eb 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -3,7 +3,6 @@ /* Interface to Sjoerd's portable C thread library */ #include "Python.h" -#include "pycore_ceval.h" // _PyEval_MakePendingCalls() #include "pycore_dict.h" // _PyDict_Pop() #include "pycore_interp.h" // _PyInterpreterState.threads.count #include "pycore_moduleobject.h" // _PyModule_GetState() @@ -76,57 +75,10 @@ lock_dealloc(lockobject *self) Py_DECREF(tp); } -/* Helper to acquire an interruptible lock with a timeout. If the lock acquire - * is interrupted, signal handlers are run, and if they raise an exception, - * PY_LOCK_INTR is returned. Otherwise, PY_LOCK_ACQUIRED or PY_LOCK_FAILURE - * are returned, depending on whether the lock can be acquired within the - * timeout. - */ -static PyLockStatus +static inline PyLockStatus acquire_timed(PyThread_type_lock lock, _PyTime_t timeout) { - PyThreadState *tstate = _PyThreadState_GET(); - _PyTime_t endtime = 0; - if (timeout > 0) { - endtime = _PyDeadline_Init(timeout); - } - - PyLockStatus r; - do { - _PyTime_t microseconds; - microseconds = _PyTime_AsMicroseconds(timeout, _PyTime_ROUND_CEILING); - - /* first a simple non-blocking try without releasing the GIL */ - r = PyThread_acquire_lock_timed(lock, 0, 0); - if (r == PY_LOCK_FAILURE && microseconds != 0) { - Py_BEGIN_ALLOW_THREADS - r = PyThread_acquire_lock_timed(lock, microseconds, 1); - Py_END_ALLOW_THREADS - } - - if (r == PY_LOCK_INTR) { - /* Run signal handlers if we were interrupted. Propagate - * exceptions from signal handlers, such as KeyboardInterrupt, by - * passing up PY_LOCK_INTR. */ - if (_PyEval_MakePendingCalls(tstate) < 0) { - return PY_LOCK_INTR; - } - - /* If we're using a timeout, recompute the timeout after processing - * signals, since those can take time. */ - if (timeout > 0) { - timeout = _PyDeadline_Get(endtime); - - /* Check for negative values, since those mean block forever. - */ - if (timeout < 0) { - r = PY_LOCK_FAILURE; - } - } - } - } while (r == PY_LOCK_INTR); /* Retry if we were interrupted. */ - - return r; + return PyThread_acquire_lock_timed_with_retries(lock, timeout); } static int diff --git a/Python/thread.c b/Python/thread.c index bf207cecb90505..7185dd43d965b9 100644 --- a/Python/thread.c +++ b/Python/thread.c @@ -6,6 +6,7 @@ Stuff shared by all thread_*.h files is collected here. */ #include "Python.h" +#include "pycore_ceval.h" // _PyEval_MakePendingCalls() #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_structseq.h" // _PyStructSequence_FiniBuiltin() #include "pycore_pythread.h" // _POSIX_THREADS @@ -92,6 +93,55 @@ PyThread_set_stacksize(size_t size) } +PyLockStatus +PyThread_acquire_lock_timed_with_retries(PyThread_type_lock lock, + PY_TIMEOUT_T timeout) +{ + PyThreadState *tstate = _PyThreadState_GET(); + _PyTime_t endtime = 0; + if (timeout > 0) { + endtime = _PyDeadline_Init(timeout); + } + + PyLockStatus r; + do { + _PyTime_t microseconds; + microseconds = _PyTime_AsMicroseconds(timeout, _PyTime_ROUND_CEILING); + + /* first a simple non-blocking try without releasing the GIL */ + r = PyThread_acquire_lock_timed(lock, 0, 0); + if (r == PY_LOCK_FAILURE && microseconds != 0) { + Py_BEGIN_ALLOW_THREADS + r = PyThread_acquire_lock_timed(lock, microseconds, 1); + Py_END_ALLOW_THREADS + } + + if (r == PY_LOCK_INTR) { + /* Run signal handlers if we were interrupted. Propagate + * exceptions from signal handlers, such as KeyboardInterrupt, by + * passing up PY_LOCK_INTR. */ + if (_PyEval_MakePendingCalls(tstate) < 0) { + return PY_LOCK_INTR; + } + + /* If we're using a timeout, recompute the timeout after processing + * signals, since those can take time. */ + if (timeout > 0) { + timeout = _PyDeadline_Get(endtime); + + /* Check for negative values, since those mean block forever. + */ + if (timeout < 0) { + r = PY_LOCK_FAILURE; + } + } + } + } while (r == PY_LOCK_INTR); /* Retry if we were interrupted. */ + + return r; +} + + /* Thread Specific Storage (TSS) API Cross-platform components of TSS API implementation. From 46c422f9948427b636735dd4a192858a52ac17bd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 18 Sep 2023 16:05:17 -0600 Subject: [PATCH 2/2] Factor out PyThread_ParseTimeoutArg(). --- Include/internal/pycore_pythread.h | 6 ++++++ Modules/_queuemodule.c | 2 ++ Modules/_threadmodule.c | 11 +++++----- Python/thread.c | 34 ++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_pythread.h b/Include/internal/pycore_pythread.h index 607bd22a4a0a42..301d0e2cda678b 100644 --- a/Include/internal/pycore_pythread.h +++ b/Include/internal/pycore_pythread.h @@ -87,6 +87,12 @@ extern int _PyThread_at_fork_reinit(PyThread_type_lock *lock); // unset: -1 seconds, in nanoseconds #define PyThread_UNSET_TIMEOUT ((_PyTime_t)(-1 * 1000 * 1000 * 1000)) +// Exported for the _xxinterpchannels module. +PyAPI_FUNC(int) PyThread_ParseTimeoutArg( + PyObject *arg, + int blocking, + PY_TIMEOUT_T *timeout); + /* Helper to acquire an interruptible lock with a timeout. If the lock acquire * is interrupted, signal handlers are run, and if they raise an exception, * PY_LOCK_INTR is returned. Otherwise, PY_LOCK_ACQUIRED or PY_LOCK_FAILURE diff --git a/Modules/_queuemodule.c b/Modules/_queuemodule.c index b4bafb375c999d..81a06cdb79a4f2 100644 --- a/Modules/_queuemodule.c +++ b/Modules/_queuemodule.c @@ -214,6 +214,8 @@ _queue_SimpleQueue_get_impl(simplequeueobject *self, PyTypeObject *cls, PY_TIMEOUT_T microseconds; PyThreadState *tstate = PyThreadState_Get(); + // XXX Use PyThread_ParseTimeoutArg(). + if (block == 0) { /* Non-blocking */ microseconds = 0; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 7620511dd1d6eb..4d453040503643 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -88,14 +88,15 @@ lock_acquire_parse_args(PyObject *args, PyObject *kwds, char *kwlist[] = {"blocking", "timeout", NULL}; int blocking = 1; PyObject *timeout_obj = NULL; - const _PyTime_t unset_timeout = _PyTime_FromSeconds(-1); - - *timeout = unset_timeout ; - if (!PyArg_ParseTupleAndKeywords(args, kwds, "|pO:acquire", kwlist, &blocking, &timeout_obj)) return -1; + // XXX Use PyThread_ParseTimeoutArg(). + + const _PyTime_t unset_timeout = _PyTime_FromSeconds(-1); + *timeout = unset_timeout; + if (timeout_obj && _PyTime_FromSecondsObject(timeout, timeout_obj, _PyTime_ROUND_TIMEOUT) < 0) @@ -108,7 +109,7 @@ lock_acquire_parse_args(PyObject *args, PyObject *kwds, } if (*timeout < 0 && *timeout != unset_timeout) { PyErr_SetString(PyExc_ValueError, - "timeout value must be positive"); + "timeout value must be a non-negative number"); return -1; } if (!blocking) diff --git a/Python/thread.c b/Python/thread.c index 7185dd43d965b9..fefae8391617f7 100644 --- a/Python/thread.c +++ b/Python/thread.c @@ -93,6 +93,40 @@ PyThread_set_stacksize(size_t size) } +int +PyThread_ParseTimeoutArg(PyObject *arg, int blocking, PY_TIMEOUT_T *timeout_p) +{ + assert(_PyTime_FromSeconds(-1) == PyThread_UNSET_TIMEOUT); + if (arg == NULL || arg == Py_None) { + *timeout_p = blocking ? PyThread_UNSET_TIMEOUT : 0; + return 0; + } + if (!blocking) { + PyErr_SetString(PyExc_ValueError, + "can't specify a timeout for a non-blocking call"); + return -1; + } + + _PyTime_t timeout; + if (_PyTime_FromSecondsObject(&timeout, arg, _PyTime_ROUND_TIMEOUT) < 0) { + return -1; + } + if (timeout < 0) { + PyErr_SetString(PyExc_ValueError, + "timeout value must be a non-negative number"); + return -1; + } + + if (_PyTime_AsMicroseconds(timeout, + _PyTime_ROUND_TIMEOUT) > PY_TIMEOUT_MAX) { + PyErr_SetString(PyExc_OverflowError, + "timeout value is too large"); + return -1; + } + *timeout_p = timeout; + return 0; +} + PyLockStatus PyThread_acquire_lock_timed_with_retries(PyThread_type_lock lock, PY_TIMEOUT_T timeout)