From 6bee81323ee7b351224543ea2dd0cd5f128ece74 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 25 Apr 2024 19:58:43 -0600 Subject: [PATCH 1/3] Add a timeout arg to _PyEval_AddPendingCall(). --- Include/internal/pycore_ceval.h | 4 +++- Modules/_testinternalcapi.c | 12 +++++----- Modules/signalmodule.c | 6 +++-- Python/ceval_gil.c | 39 +++++++++++++++++++++++---------- Python/crossinterp.c | 6 +++-- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index cfb88c3f4c8e15..76010a7f66cf14 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -51,13 +51,15 @@ extern void _PyEval_SignalReceived(void); typedef int _Py_add_pending_call_result; #define _Py_ADD_PENDING_SUCCESS 0 #define _Py_ADD_PENDING_FULL -1 +#define _Py_ADD_PENDING_TIMED_OUT -2 // Export for '_testinternalcapi' shared extension PyAPI_FUNC(_Py_add_pending_call_result) _PyEval_AddPendingCall( PyInterpreterState *interp, _Py_pending_call_func func, void *arg, - int flags); + int flags, + PY_TIMEOUT_T timeout); #ifdef HAVE_FORK extern PyStatus _PyEval_ReInitThreads(PyThreadState *tstate); diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index b0bba3422a50a0..4ce8ecfcc7a87e 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1090,13 +1090,16 @@ pending_threadfunc(PyObject *self, PyObject *args, PyObject *kwargs) if (ensure_added) { _Py_add_pending_call_result r; do { - r = _PyEval_AddPendingCall(interp, &_pending_callback, callable, 0); + r = _PyEval_AddPendingCall( + interp, &_pending_callback, callable, 0, 0); assert(r == _Py_ADD_PENDING_SUCCESS || r == _Py_ADD_PENDING_FULL); } while (r == _Py_ADD_PENDING_FULL); } else { - if (_PyEval_AddPendingCall(interp, &_pending_callback, callable, 0) < 0) { + if (_PyEval_AddPendingCall( + interp, &_pending_callback, callable, 0, 0) < 0) + { break; } } @@ -1157,9 +1160,8 @@ pending_identify(PyObject *self, PyObject *args) _Py_add_pending_call_result r; do { Py_BEGIN_ALLOW_THREADS - r = _PyEval_AddPendingCall(interp, - &_pending_identify_callback, (void *)mutex, - 0); + r = _PyEval_AddPendingCall( + interp, &_pending_identify_callback, (void *)mutex, 0, 0); Py_END_ALLOW_THREADS assert(r == _Py_ADD_PENDING_SUCCESS || r == _Py_ADD_PENDING_FULL); diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 08fedeacd96d28..777414d1458a8d 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -316,7 +316,8 @@ trip_signal(int sig_num) _PyEval_AddPendingCall(interp, report_wakeup_send_error, (void *)(intptr_t) last_error, - _Py_PENDING_MAINTHREADONLY); + _Py_PENDING_MAINTHREADONLY, + 0 /* timeout */); } } } @@ -336,7 +337,8 @@ trip_signal(int sig_num) _PyEval_AddPendingCall(interp, report_wakeup_write_error, (void *)(intptr_t)errno, - _Py_PENDING_MAINTHREADONLY); + _Py_PENDING_MAINTHREADONLY, + 0 /* timeout */); } } } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index c3c2c54b199c59..f1a0bac1ba75bf 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -7,6 +7,7 @@ #include "pycore_pylifecycle.h" // _PyErr_Print() #include "pycore_pymem.h" // _PyMem_IsPtrFreed() #include "pycore_pystats.h" // _Py_PrintSpecializationStats() +#include "pycore_time.h" // _PyDeadline_Init() /* Notes about the implementation: @@ -739,7 +740,8 @@ _pop_pending_call(struct _pending_calls *pending, _Py_add_pending_call_result _PyEval_AddPendingCall(PyInterpreterState *interp, - _Py_pending_call_func func, void *arg, int flags) + _Py_pending_call_func func, void *arg, int flags, + PY_TIMEOUT_T timeout) { struct _pending_calls *pending = &interp->ceval.pending; int main_only = (flags & _Py_PENDING_MAINTHREADONLY) != 0; @@ -749,11 +751,22 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, pending = &_PyRuntime.ceval.pending_mainthread; } - PyMutex_Lock(&pending->mutex); - _Py_add_pending_call_result result = - _push_pending_call(pending, func, arg, flags); - PyMutex_Unlock(&pending->mutex); + PyTime_t endtime = 0; + if (timeout > 0) { + endtime = _PyDeadline_Init(timeout); + } + _Py_add_pending_call_result result = -INT32_MIN; + do { + PyMutex_Lock(&pending->mutex); + result = _push_pending_call(pending, func, arg, flags); + PyMutex_Unlock(&pending->mutex); + if (timeout > 0) { + timeout = _PyDeadline_Get(endtime); + } + } while (result == _Py_ADD_PENDING_FULL && timeout > 0); + + // XXX Do not update the eval breaker if not _Py_ADD_PENDING_SUCCESS. if (main_only) { _Py_set_eval_breaker_bit(_PyRuntime.main_tstate, _PY_CALLS_TO_DO_BIT); } @@ -774,14 +787,18 @@ Py_AddPendingCall(_Py_pending_call_func func, void *arg) /* Legacy users of this API will continue to target the main thread (of the main interpreter). */ PyInterpreterState *interp = _PyInterpreterState_Main(); - _Py_add_pending_call_result r = - _PyEval_AddPendingCall(interp, func, arg, _Py_PENDING_MAINTHREADONLY); - if (r == _Py_ADD_PENDING_FULL) { + _Py_add_pending_call_result r = _PyEval_AddPendingCall( + interp, func, arg, _Py_PENDING_MAINTHREADONLY, 0); + switch (r) { + case _Py_ADD_PENDING_TIMED_OUT: // fall through + case _Py_ADD_PENDING_FULL: return -1; - } - else { - assert(r == _Py_ADD_PENDING_SUCCESS); + case _Py_ADD_PENDING_SUCCESS: return 0; + default: + // We added a new result kind but forgot to handle it here. + assert(0); + return -1; } } diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 367e29d40d895a..22214af6ec7041 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -34,7 +34,8 @@ _Py_CallInInterpreter(PyInterpreterState *interp, return func(arg); } // XXX Emit a warning if this fails? - _PyEval_AddPendingCall(interp, (_Py_pending_call_func)func, arg, 0); + _PyEval_AddPendingCall( + interp, (_Py_pending_call_func)func, arg, 0, 0 /* timeout */); return 0; } @@ -48,7 +49,8 @@ _Py_CallInInterpreterAndRawFree(PyInterpreterState *interp, return res; } // XXX Emit a warning if this fails? - _PyEval_AddPendingCall(interp, func, arg, _Py_PENDING_RAWFREE); + _PyEval_AddPendingCall( + interp, func, arg, _Py_PENDING_RAWFREE, 0 /* timeout */); return 0; } From a5f678f7db3178124cf9072d45da52923c4e7035 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 26 Apr 2024 11:18:46 -0600 Subject: [PATCH 2/3] Use a lock to sleep in each loop. --- Python/ceval_gil.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index f1a0bac1ba75bf..42037a8eabe596 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -756,15 +756,32 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, endtime = _PyDeadline_Init(timeout); } - _Py_add_pending_call_result result = -INT32_MIN; - do { - PyMutex_Lock(&pending->mutex); - result = _push_pending_call(pending, func, arg, flags); - PyMutex_Unlock(&pending->mutex); - if (timeout > 0) { + PyMutex_Lock(&pending->mutex); + _Py_add_pending_call_result result = + _push_pending_call(pending, func, arg, flags); + PyMutex_Unlock(&pending->mutex); + + if (timeout > 0) { + /* We use this lock only to sleep. */ + PyMutex timeout_sleeper = (PyMutex){0}; + PyMutex_Lock(&timeout_sleeper); + while (result == _Py_ADD_PENDING_FULL) { + if (timeout <= 0) { + result = _Py_ADD_PENDING_TIMED_OUT; + break; + } +#define SLEEP_NS 1000 /* 1 millisecond */ + (void)_PyMutex_LockTimed( + &timeout_sleeper, SLEEP_NS, _Py_LOCK_DONT_DETACH); + + PyMutex_Lock(&pending->mutex); + result = _push_pending_call(pending, func, arg, flags); + PyMutex_Unlock(&pending->mutex); + timeout = _PyDeadline_Get(endtime); } - } while (result == _Py_ADD_PENDING_FULL && timeout > 0); + PyMutex_Unlock(&timeout_sleeper); + } // XXX Do not update the eval breaker if not _Py_ADD_PENDING_SUCCESS. if (main_only) { From 0ff8221eb9da6e5a8cf7dea4b2306f21026122aa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 26 Apr 2024 11:33:29 -0600 Subject: [PATCH 3/3] Add some TODO comments. --- Python/ceval_gil.c | 2 ++ Python/crossinterp.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 42037a8eabe596..ab41105a1a03ab 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -763,6 +763,7 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, if (timeout > 0) { /* We use this lock only to sleep. */ + // XXX Use time.sleep()? Don't sleep at all? PyMutex timeout_sleeper = (PyMutex){0}; PyMutex_Lock(&timeout_sleeper); while (result == _Py_ADD_PENDING_FULL) { @@ -770,6 +771,7 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, result = _Py_ADD_PENDING_TIMED_OUT; break; } +// XXX Use a smaller sleep interval? #define SLEEP_NS 1000 /* 1 millisecond */ (void)_PyMutex_LockTimed( &timeout_sleeper, SLEEP_NS, _Py_LOCK_DONT_DETACH); diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 22214af6ec7041..8b32451cdab44d 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -34,6 +34,7 @@ _Py_CallInInterpreter(PyInterpreterState *interp, return func(arg); } // XXX Emit a warning if this fails? + // XXX Block until added? _PyEval_AddPendingCall( interp, (_Py_pending_call_func)func, arg, 0, 0 /* timeout */); return 0; @@ -49,6 +50,7 @@ _Py_CallInInterpreterAndRawFree(PyInterpreterState *interp, return res; } // XXX Emit a warning if this fails? + // XXX Block until added? _PyEval_AddPendingCall( interp, func, arg, _Py_PENDING_RAWFREE, 0 /* timeout */); return 0;