From f8e70b485855dbb709b56ea16a9686ababcefa1d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 13 Jul 2018 16:57:36 -0600 Subject: [PATCH 01/14] Move pending calls from _PyRuntimeState to PyIntepreterState. --- Include/internal/pycore_ceval.h | 10 ++-- Include/internal/pycore_pystate.h | 10 ++++ Python/ceval.c | 77 +++++++++++++++++-------------- Python/pylifecycle.c | 5 +- Python/pystate.c | 11 +++++ 5 files changed, 72 insertions(+), 41 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 2ead96c7abe32a..ef1659212994b3 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -11,7 +11,11 @@ extern "C" { #include "pycore_atomic.h" #include "pythread.h" -PyAPI_FUNC(void) _Py_FinishPendingCalls(void); +struct _is; // See PyInterpreterState in cpython/pystate.h. + +PyAPI_FUNC(int) _Py_AddPendingCall(struct _is*, int (*)(void *), void *); +PyAPI_FUNC(int) _Py_MakePendingCalls(struct _is*); +PyAPI_FUNC(void) _Py_FinishPendingCalls(struct _is*); struct _pending_calls { int finishing; @@ -41,12 +45,8 @@ struct _ceval_runtime_state { c_tracefunc. This speeds up the if statement in PyEval_EvalFrameEx() after fast_next_opcode. */ int tracing_possible; - /* This single variable consolidates all requests to break out of - the fast path in the eval loop. */ - _Py_atomic_int eval_breaker; /* Request for dropping the GIL */ _Py_atomic_int gil_drop_request; - struct _pending_calls pending; /* Request for checking signals. */ _Py_atomic_int signals_pending; struct _gil_runtime_state gil; diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index df3730f8014ae4..a35301972be141 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -12,6 +12,7 @@ extern "C" { #include "pystate.h" #include "pythread.h" +#include "pycore_atomic.h" #include "pycore_ceval.h" #include "pycore_pathconfig.h" #include "pycore_pymem.h" @@ -44,6 +45,15 @@ struct _is { /* Used in Python/sysmodule.c. */ int check_interval; +#ifdef Py_BUILD_CORE + struct _ceval { + /* This single variable consolidates all requests to break out of + the fast path in the eval loop. */ + _Py_atomic_int eval_breaker; + struct _pending_calls pending; + } ceval; +#endif + /* Used in Modules/_threadmodule.c. */ long num_threads; /* Support for runtime thread stack size tuning. diff --git a/Python/ceval.c b/Python/ceval.c index 28e923219d389c..88f92cb4a60779 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -98,16 +98,16 @@ static long dxp[256]; the GIL eventually anyway. */ #define COMPUTE_EVAL_BREAKER() \ _Py_atomic_store_relaxed( \ - &_PyRuntime.ceval.eval_breaker, \ + &PyThreadState_Get()->interp->ceval.eval_breaker, \ GIL_REQUEST | \ _Py_atomic_load_relaxed(&_PyRuntime.ceval.signals_pending) | \ - _Py_atomic_load_relaxed(&_PyRuntime.ceval.pending.calls_to_do) | \ - _PyRuntime.ceval.pending.async_exc) + _Py_atomic_load_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do) | \ + PyThreadState_Get()->interp->ceval.pending.async_exc) #define SET_GIL_DROP_REQUEST() \ do { \ _Py_atomic_store_relaxed(&_PyRuntime.ceval.gil_drop_request, 1); \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.eval_breaker, 1); \ + _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ } while (0) #define RESET_GIL_DROP_REQUEST() \ @@ -119,20 +119,20 @@ static long dxp[256]; /* Pending calls are only modified under pending_lock */ #define SIGNAL_PENDING_CALLS() \ do { \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.pending.calls_to_do, 1); \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.eval_breaker, 1); \ + _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do, 1); \ + _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ } while (0) #define UNSIGNAL_PENDING_CALLS() \ do { \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.pending.calls_to_do, 0); \ + _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do, 0); \ COMPUTE_EVAL_BREAKER(); \ } while (0) #define SIGNAL_PENDING_SIGNALS() \ do { \ _Py_atomic_store_relaxed(&_PyRuntime.ceval.signals_pending, 1); \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.eval_breaker, 1); \ + _Py_atomic_store_relaxed(&_PyRuntime.interpreters.main->ceval.eval_breaker, 1); \ } while (0) #define UNSIGNAL_PENDING_SIGNALS() \ @@ -143,13 +143,13 @@ static long dxp[256]; #define SIGNAL_ASYNC_EXC() \ do { \ - _PyRuntime.ceval.pending.async_exc = 1; \ - _Py_atomic_store_relaxed(&_PyRuntime.ceval.eval_breaker, 1); \ + PyThreadState_Get()->interp->ceval.pending.async_exc = 1; \ + _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ } while (0) #define UNSIGNAL_ASYNC_EXC() \ do { \ - _PyRuntime.ceval.pending.async_exc = 0; \ + PyThreadState_Get()->interp->ceval.pending.async_exc = 0; \ COMPUTE_EVAL_BREAKER(); \ } while (0) @@ -177,10 +177,7 @@ PyEval_InitThreads(void) create_gil(); take_gil(_PyThreadState_GET()); - _PyRuntime.ceval.pending.lock = PyThread_allocate_lock(); - if (_PyRuntime.ceval.pending.lock == NULL) { - Py_FatalError("Can't initialize threads for pending calls"); - } + // The pending calls mutex is initialized in PyInterpreterState_New(). } void @@ -256,8 +253,10 @@ PyEval_ReInitThreads(void) recreate_gil(); take_gil(current_tstate); - _PyRuntime.ceval.pending.lock = PyThread_allocate_lock(); - if (_PyRuntime.ceval.pending.lock == NULL) { + // Only the main interpreter remains, so ignore the rest. + PyInterpreterState *interp = _PyRuntime.interpreters.main; + interp->ceval.pending.lock = PyThread_allocate_lock(); + if (interp->ceval.pending.lock == NULL) { Py_FatalError("Can't initialize threads for pending calls"); } @@ -374,9 +373,9 @@ _pop_pending_call(struct _pending_calls *pending, */ int -Py_AddPendingCall(int (*func)(void *), void *arg) +_Py_AddPendingCall(PyInterpreterState *interp, int (*func)(void *), void *arg) { - struct _pending_calls *pending = &_PyRuntime.ceval.pending; + struct _pending_calls *pending = &interp->ceval.pending; PyThread_acquire_lock(pending->lock, WAIT_LOCK); if (pending->finishing) { @@ -399,6 +398,15 @@ Py_AddPendingCall(int (*func)(void *), void *arg) return result; } +/* Py_AddPendingCall() is a simple wrapper for the sake + of backward-compatibility. */ +int +Py_AddPendingCall(int (*func)(void *), void *arg) +{ + PyInterpreterState *interp = _PyRuntime.interpreters.main; + return _Py_AddPendingCall(interp, func, arg); +} + static int handle_signals(void) { @@ -429,11 +437,6 @@ make_pending_calls(struct _pending_calls* pending) { static int busy = 0; - /* only service pending calls on main thread */ - if (PyThread_get_thread_ident() != _PyRuntime.main_thread) { - return 0; - } - /* don't perform recursive pending calls */ if (busy) { return 0; @@ -474,9 +477,9 @@ make_pending_calls(struct _pending_calls* pending) } void -_Py_FinishPendingCalls(void) +_Py_FinishPendingCalls(PyInterpreterState *interp) { - struct _pending_calls *pending = &_PyRuntime.ceval.pending; + struct _pending_calls *pending = &interp->ceval.pending; assert(PyGILState_Check()); @@ -497,6 +500,14 @@ _Py_FinishPendingCalls(void) } } +int +_Py_MakePendingCalls(PyInterpreterState *interp) +{ + assert(PyGILState_Check()); + + return make_pending_calls(&interp->ceval.pending); +} + /* Py_MakePendingCalls() is a simple wrapper for the sake of backward-compatibility. */ int @@ -511,12 +522,8 @@ Py_MakePendingCalls(void) return res; } - res = make_pending_calls(&_PyRuntime.ceval.pending); - if (res != 0) { - return res; - } - - return 0; + PyInterpreterState *interp = _PyRuntime.interpreters.main; + return make_pending_calls(&interp->ceval.pending); } /* The interpreter's recursion limit */ @@ -638,7 +645,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) PyObject **fastlocals, **freevars; PyObject *retval = NULL; /* Return value */ PyThreadState *tstate = _PyThreadState_GET(); - _Py_atomic_int *eval_breaker = &_PyRuntime.ceval.eval_breaker; + _Py_atomic_int *eval_breaker = &tstate->interp->ceval.eval_breaker; PyCodeObject *co; /* when tracing we set things up so that @@ -1059,9 +1066,9 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) } } if (_Py_atomic_load_relaxed( - &_PyRuntime.ceval.pending.calls_to_do)) + &tstate->interp->ceval.pending.calls_to_do)) { - if (make_pending_calls(&_PyRuntime.ceval.pending) != 0) { + if (make_pending_calls(&tstate->interp->ceval.pending) != 0) { goto error; } } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index ad1447256cc697..44acba2d93e18e 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1146,7 +1146,7 @@ Py_FinalizeEx(void) interp = tstate->interp; // Make any remaining pending calls. - _Py_FinishPendingCalls(); + _Py_FinishPendingCalls(interp); /* The interpreter is still entirely intact at this point, and the * exit funcs may be relying on that. In particular, if some thread @@ -1552,6 +1552,9 @@ Py_EndInterpreter(PyThreadState *tstate) // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(); + // Make any remaining pending calls. + _Py_FinishPendingCalls(interp); + call_py_exitfuncs(interp); if (tstate != interp->tstate_head || tstate->next != NULL) diff --git a/Python/pystate.c b/Python/pystate.c index a2464b6cf5518f..f44d41c26db740 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -173,6 +173,14 @@ PyInterpreterState_New(void) memset(interp, 0, sizeof(*interp)); interp->id_refcount = -1; interp->check_interval = 100; + + interp->ceval.pending.lock = PyThread_allocate_lock(); + if (interp->ceval.pending.lock == NULL) { + PyErr_SetString(PyExc_RuntimeError, + "failed to create interpreter ceval pending mutex"); + return NULL; + } + interp->core_config = _PyCoreConfig_INIT; interp->eval_frame = _PyEval_EvalFrameDefault; #ifdef HAVE_DLOPEN @@ -279,6 +287,9 @@ PyInterpreterState_Delete(PyInterpreterState *interp) if (interp->id_mutex != NULL) { PyThread_free_lock(interp->id_mutex); } + if (interp->ceval.pending.lock != NULL) { + PyThread_free_lock(interp->ceval.pending.lock); + } PyMem_RawFree(interp); } From 991e3fb840e7684a44abf0743dba84f798ca55eb Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 15 Mar 2019 13:46:25 -0600 Subject: [PATCH 02/14] Release the pending calls lock *after* updating the eval breaker flag. --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 88f92cb4a60779..07e6dd72275676 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -391,10 +391,10 @@ _Py_AddPendingCall(PyInterpreterState *interp, int (*func)(void *), void *arg) return -1; } int result = _push_pending_call(pending, func, arg); - PyThread_release_lock(pending->lock); - /* signal main loop */ SIGNAL_PENDING_CALLS(); + PyThread_release_lock(pending->lock); + return result; } From 69b620ea1c4fd19e0743c355b713d3f7c18706d2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 10 Sep 2018 16:41:27 -0600 Subject: [PATCH 03/14] Explicitly pass the interpreter to the stateful ceval macros. --- Include/ceval.h | 2 +- Python/ceval.c | 65 +++++++++++++++++++++++----------------------- Python/ceval_gil.h | 8 +++--- Python/pystate.c | 2 +- 4 files changed, 39 insertions(+), 38 deletions(-) diff --git a/Include/ceval.h b/Include/ceval.h index 11283c0a570b7e..9c6d420bc23449 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -221,7 +221,7 @@ PyAPI_FUNC(Py_ssize_t) _PyEval_RequestCodeExtraIndex(freefunc); #ifndef Py_LIMITED_API PyAPI_FUNC(int) _PyEval_SliceIndex(PyObject *, Py_ssize_t *); PyAPI_FUNC(int) _PyEval_SliceIndexNotNone(PyObject *, Py_ssize_t *); -PyAPI_FUNC(void) _PyEval_SignalAsyncExc(void); +PyAPI_FUNC(void) _PyEval_SignalAsyncExc(PyInterpreterState *); #endif /* Masks and values used by FORMAT_VALUE opcode. */ diff --git a/Python/ceval.c b/Python/ceval.c index 07e6dd72275676..effa8955c83aa7 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -96,37 +96,37 @@ static long dxp[256]; /* This can set eval_breaker to 0 even though gil_drop_request became 1. We believe this is all right because the eval loop will release the GIL eventually anyway. */ -#define COMPUTE_EVAL_BREAKER() \ +#define COMPUTE_EVAL_BREAKER(interp) \ _Py_atomic_store_relaxed( \ - &PyThreadState_Get()->interp->ceval.eval_breaker, \ + &interp->ceval.eval_breaker, \ GIL_REQUEST | \ _Py_atomic_load_relaxed(&_PyRuntime.ceval.signals_pending) | \ - _Py_atomic_load_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do) | \ - PyThreadState_Get()->interp->ceval.pending.async_exc) + _Py_atomic_load_relaxed(&interp->ceval.pending.calls_to_do) | \ + interp->ceval.pending.async_exc) -#define SET_GIL_DROP_REQUEST() \ +#define SET_GIL_DROP_REQUEST(interp) \ do { \ _Py_atomic_store_relaxed(&_PyRuntime.ceval.gil_drop_request, 1); \ - _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ + _Py_atomic_store_relaxed(&interp->ceval.eval_breaker, 1); \ } while (0) -#define RESET_GIL_DROP_REQUEST() \ +#define RESET_GIL_DROP_REQUEST(interp) \ do { \ _Py_atomic_store_relaxed(&_PyRuntime.ceval.gil_drop_request, 0); \ - COMPUTE_EVAL_BREAKER(); \ + COMPUTE_EVAL_BREAKER(interp); \ } while (0) /* Pending calls are only modified under pending_lock */ -#define SIGNAL_PENDING_CALLS() \ +#define SIGNAL_PENDING_CALLS(interp) \ do { \ - _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do, 1); \ - _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ + _Py_atomic_store_relaxed(&interp->ceval.pending.calls_to_do, 1); \ + _Py_atomic_store_relaxed(&interp->ceval.eval_breaker, 1); \ } while (0) -#define UNSIGNAL_PENDING_CALLS() \ +#define UNSIGNAL_PENDING_CALLS(interp) \ do { \ - _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.pending.calls_to_do, 0); \ - COMPUTE_EVAL_BREAKER(); \ + _Py_atomic_store_relaxed(&interp->ceval.pending.calls_to_do, 0); \ + COMPUTE_EVAL_BREAKER(interp); \ } while (0) #define SIGNAL_PENDING_SIGNALS() \ @@ -138,19 +138,19 @@ static long dxp[256]; #define UNSIGNAL_PENDING_SIGNALS() \ do { \ _Py_atomic_store_relaxed(&_PyRuntime.ceval.signals_pending, 0); \ - COMPUTE_EVAL_BREAKER(); \ + COMPUTE_EVAL_BREAKER(_PyRuntime.interpreters.main); \ } while (0) -#define SIGNAL_ASYNC_EXC() \ +#define SIGNAL_ASYNC_EXC(interp) \ do { \ - PyThreadState_Get()->interp->ceval.pending.async_exc = 1; \ - _Py_atomic_store_relaxed(&PyThreadState_Get()->interp->ceval.eval_breaker, 1); \ + interp->ceval.pending.async_exc = 1; \ + _Py_atomic_store_relaxed(&interp->ceval.eval_breaker, 1); \ } while (0) -#define UNSIGNAL_ASYNC_EXC() \ +#define UNSIGNAL_ASYNC_EXC(interp) \ do { \ - PyThreadState_Get()->interp->ceval.pending.async_exc = 0; \ - COMPUTE_EVAL_BREAKER(); \ + interp->ceval.pending.async_exc = 0; \ + COMPUTE_EVAL_BREAKER(interp); \ } while (0) @@ -268,9 +268,9 @@ PyEval_ReInitThreads(void) raised. */ void -_PyEval_SignalAsyncExc(void) +_PyEval_SignalAsyncExc(PyInterpreterState *interp) { - SIGNAL_ASYNC_EXC(); + SIGNAL_ASYNC_EXC(interp); } PyThreadState * @@ -392,7 +392,7 @@ _Py_AddPendingCall(PyInterpreterState *interp, int (*func)(void *), void *arg) } int result = _push_pending_call(pending, func, arg); /* signal main loop */ - SIGNAL_PENDING_CALLS(); + SIGNAL_PENDING_CALLS(interp); PyThread_release_lock(pending->lock); return result; @@ -433,8 +433,9 @@ handle_signals(void) } static int -make_pending_calls(struct _pending_calls* pending) +make_pending_calls(PyInterpreterState *interp) { + struct _pending_calls *pending = &interp->ceval.pending; static int busy = 0; /* don't perform recursive pending calls */ @@ -444,7 +445,7 @@ make_pending_calls(struct _pending_calls* pending) busy = 1; /* unsignal before starting to call callbacks, so that any callback added in-between re-signals */ - UNSIGNAL_PENDING_CALLS(); + UNSIGNAL_PENDING_CALLS(interp); int res = 0; /* perform a bounded number of calls, in case of recursion */ @@ -472,7 +473,7 @@ make_pending_calls(struct _pending_calls* pending) error: busy = 0; - SIGNAL_PENDING_CALLS(); + SIGNAL_PENDING_CALLS(interp); /* We're not done yet */ return res; } @@ -491,7 +492,7 @@ _Py_FinishPendingCalls(PyInterpreterState *interp) return; } - if (make_pending_calls(pending) < 0) { + if (make_pending_calls(interp) < 0) { PyObject *exc, *val, *tb; PyErr_Fetch(&exc, &val, &tb); PyErr_BadInternalCall(); @@ -505,7 +506,7 @@ _Py_MakePendingCalls(PyInterpreterState *interp) { assert(PyGILState_Check()); - return make_pending_calls(&interp->ceval.pending); + return make_pending_calls(interp); } /* Py_MakePendingCalls() is a simple wrapper for the sake @@ -523,7 +524,7 @@ Py_MakePendingCalls(void) } PyInterpreterState *interp = _PyRuntime.interpreters.main; - return make_pending_calls(&interp->ceval.pending); + return make_pending_calls(interp); } /* The interpreter's recursion limit */ @@ -1068,7 +1069,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) if (_Py_atomic_load_relaxed( &tstate->interp->ceval.pending.calls_to_do)) { - if (make_pending_calls(&tstate->interp->ceval.pending) != 0) { + if (make_pending_calls(tstate->interp) != 0) { goto error; } } @@ -1100,7 +1101,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) if (tstate->async_exc != NULL) { PyObject *exc = tstate->async_exc; tstate->async_exc = NULL; - UNSIGNAL_ASYNC_EXC(); + UNSIGNAL_ASYNC_EXC(tstate->interp); PyErr_SetNone(exc); Py_DECREF(exc); goto error; diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index f2d5fdba015364..d9ad3616fa2427 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -176,7 +176,7 @@ static void drop_gil(PyThreadState *tstate) &_PyRuntime.ceval.gil.last_holder) ) == tstate) { - RESET_GIL_DROP_REQUEST(); + RESET_GIL_DROP_REQUEST(tstate->interp); /* NOTE: if COND_WAIT does not atomically start waiting when releasing the mutex, another thread can run through, take the GIL and drop it again, and reset the condition @@ -213,7 +213,7 @@ static void take_gil(PyThreadState *tstate) if (timed_out && _Py_atomic_load_relaxed(&_PyRuntime.ceval.gil.locked) && _PyRuntime.ceval.gil.switch_number == saved_switchnum) { - SET_GIL_DROP_REQUEST(); + SET_GIL_DROP_REQUEST(tstate->interp); } } _ready: @@ -239,10 +239,10 @@ static void take_gil(PyThreadState *tstate) MUTEX_UNLOCK(_PyRuntime.ceval.gil.switch_mutex); #endif if (_Py_atomic_load_relaxed(&_PyRuntime.ceval.gil_drop_request)) { - RESET_GIL_DROP_REQUEST(); + RESET_GIL_DROP_REQUEST(tstate->interp); } if (tstate->async_exc != NULL) { - _PyEval_SignalAsyncExc(); + _PyEval_SignalAsyncExc(tstate->interp); } MUTEX_UNLOCK(_PyRuntime.ceval.gil.mutex); diff --git a/Python/pystate.c b/Python/pystate.c index f44d41c26db740..e606c0957da148 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -939,7 +939,7 @@ PyThreadState_SetAsyncExc(unsigned long id, PyObject *exc) p->async_exc = exc; HEAD_UNLOCK(); Py_XDECREF(old_exc); - _PyEval_SignalAsyncExc(); + _PyEval_SignalAsyncExc(interp); return 1; } } From 8993d237fdf825272d6b5bd7a55078982dd26b32 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Sep 2018 16:35:49 -0700 Subject: [PATCH 04/14] Use the internal function. --- Modules/_testcapimodule.c | 1 + Modules/signalmodule.c | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index c515efe660b5bd..a95b50f061a275 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2445,6 +2445,7 @@ pending_threadfunc(PyObject *self, PyObject *arg) Py_INCREF(callable); Py_BEGIN_ALLOW_THREADS + /* XXX Use the internal _Py_AddPendingCall(). */ r = Py_AddPendingCall(&_pending_callback, callable); Py_END_ALLOW_THREADS diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 4590017c170a52..ce2e7ecde0a2f2 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -19,6 +19,7 @@ #include #endif #endif +#include "internal/pycore_pystate.h" #ifdef HAVE_SIGNAL_H #include @@ -295,8 +296,9 @@ trip_signal(int sig_num) { /* Py_AddPendingCall() isn't signal-safe, but we still use it for this exceptional case. */ - Py_AddPendingCall(report_wakeup_send_error, - (void *)(intptr_t) last_error); + _Py_AddPendingCall(_PyRuntime.interpreters.main, + report_wakeup_send_error, + (void *)(intptr_t) last_error); } } } @@ -313,8 +315,9 @@ trip_signal(int sig_num) { /* Py_AddPendingCall() isn't signal-safe, but we still use it for this exceptional case. */ - Py_AddPendingCall(report_wakeup_write_error, - (void *)(intptr_t)errno); + _Py_AddPendingCall(_PyRuntime.interpreters.main, + report_wakeup_write_error, + (void *)(intptr_t)errno); } } } From 66556769c2d24a438a40913ec8856fe4539773a2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 15 Sep 2018 12:14:04 -0600 Subject: [PATCH 05/14] Add a NEWS entry. --- .../2018-09-15-12-13-46.bpo-33608.avmvVP.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-09-15-12-13-46.bpo-33608.avmvVP.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-15-12-13-46.bpo-33608.avmvVP.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-15-12-13-46.bpo-33608.avmvVP.rst new file mode 100644 index 00000000000000..73a01a1f46bdc1 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-15-12-13-46.bpo-33608.avmvVP.rst @@ -0,0 +1,5 @@ +We added a new internal _Py_AddPendingCall() that operates relative to the +provided interpreter. This allows us to use the existing implementation to +ask another interpreter to do work that cannot be done in the current +interpreter, like decref an object the other interpreter owns. The existing +Py_AddPendingCall() only operates relative to the main interpreter. From d71613160298471fb1e64c19cb60e1c4953f1a5d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 15 Sep 2018 13:16:34 -0600 Subject: [PATCH 06/14] Optionally lock a pending call to a specific thread. --- Include/internal/pycore_ceval.h | 3 ++- Modules/signalmodule.c | 2 ++ Python/ceval.c | 23 +++++++++++++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index ef1659212994b3..a90b288201f834 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -13,7 +13,7 @@ extern "C" { struct _is; // See PyInterpreterState in cpython/pystate.h. -PyAPI_FUNC(int) _Py_AddPendingCall(struct _is*, int (*)(void *), void *); +PyAPI_FUNC(int) _Py_AddPendingCall(struct _is*, unsigned long, int (*)(void *), void *); PyAPI_FUNC(int) _Py_MakePendingCalls(struct _is*); PyAPI_FUNC(void) _Py_FinishPendingCalls(struct _is*); @@ -28,6 +28,7 @@ struct _pending_calls { int async_exc; #define NPENDINGCALLS 32 struct { + unsigned long thread_id; int (*func)(void *); void *arg; } calls[NPENDINGCALLS]; diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index ce2e7ecde0a2f2..962174dda944a6 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -297,6 +297,7 @@ trip_signal(int sig_num) /* Py_AddPendingCall() isn't signal-safe, but we still use it for this exceptional case. */ _Py_AddPendingCall(_PyRuntime.interpreters.main, + main_thread, report_wakeup_send_error, (void *)(intptr_t) last_error); } @@ -316,6 +317,7 @@ trip_signal(int sig_num) /* Py_AddPendingCall() isn't signal-safe, but we still use it for this exceptional case. */ _Py_AddPendingCall(_PyRuntime.interpreters.main, + main_thread, report_wakeup_write_error, (void *)(intptr_t)errno); } diff --git a/Python/ceval.c b/Python/ceval.c index effa8955c83aa7..a2aaf5d0335fdc 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -338,7 +338,7 @@ _PyEval_SignalReceived(void) /* Push one item onto the queue while holding the lock. */ static int -_push_pending_call(struct _pending_calls *pending, +_push_pending_call(struct _pending_calls *pending, unsigned long thread_id, int (*func)(void *), void *arg) { int i = pending->last; @@ -346,6 +346,7 @@ _push_pending_call(struct _pending_calls *pending, if (j == pending->first) { return -1; /* Queue full */ } + pending->calls[i].thread_id = thread_id; pending->calls[i].func = func; pending->calls[i].arg = arg; pending->last = j; @@ -354,7 +355,7 @@ _push_pending_call(struct _pending_calls *pending, /* Pop one item off the queue while holding the lock. */ static void -_pop_pending_call(struct _pending_calls *pending, +_pop_pending_call(struct _pending_calls *pending, unsigned long *thread_id, int (**func)(void *), void **arg) { int i = pending->first; @@ -364,6 +365,7 @@ _pop_pending_call(struct _pending_calls *pending, *func = pending->calls[i].func; *arg = pending->calls[i].arg; + *thread_id = pending->calls[i].thread_id; pending->first = (i + 1) % NPENDINGCALLS; } @@ -373,7 +375,8 @@ _pop_pending_call(struct _pending_calls *pending, */ int -_Py_AddPendingCall(PyInterpreterState *interp, int (*func)(void *), void *arg) +_Py_AddPendingCall(PyInterpreterState *interp, unsigned long thread_id, + int (*func)(void *), void *arg) { struct _pending_calls *pending = &interp->ceval.pending; @@ -390,7 +393,7 @@ _Py_AddPendingCall(PyInterpreterState *interp, int (*func)(void *), void *arg) PyErr_Restore(exc, val, tb); return -1; } - int result = _push_pending_call(pending, func, arg); + int result = _push_pending_call(pending, thread_id, func, arg); /* signal main loop */ SIGNAL_PENDING_CALLS(interp); PyThread_release_lock(pending->lock); @@ -404,7 +407,7 @@ int Py_AddPendingCall(int (*func)(void *), void *arg) { PyInterpreterState *interp = _PyRuntime.interpreters.main; - return _Py_AddPendingCall(interp, func, arg); + return _Py_AddPendingCall(interp, _PyRuntime.main_thread, func, arg); } static int @@ -449,15 +452,23 @@ make_pending_calls(PyInterpreterState *interp) int res = 0; /* perform a bounded number of calls, in case of recursion */ + unsigned long thread_id = 0; for (int i=0; ilock, WAIT_LOCK); - _pop_pending_call(pending, &func, &arg); + _pop_pending_call(pending, &thread_id, &func, &arg); PyThread_release_lock(pending->lock); + if (thread_id && PyThread_get_thread_ident() != thread_id) { + // Thread mismatch, so move it to the end of the list + // and start over. + _Py_AddPendingCall(interp, thread_id, func, arg); + return 0; + } + /* having released the lock, perform the callback */ if (func == NULL) { break; From 1cdc3b20916033111324d687e58b4e6b5f70d299 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 1 Feb 2019 11:49:28 -0700 Subject: [PATCH 07/14] Move core-only field to end of struct. --- Include/internal/pycore_pystate.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index a35301972be141..59039c2d899709 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -45,15 +45,6 @@ struct _is { /* Used in Python/sysmodule.c. */ int check_interval; -#ifdef Py_BUILD_CORE - struct _ceval { - /* This single variable consolidates all requests to break out of - the fast path in the eval loop. */ - _Py_atomic_int eval_breaker; - struct _pending_calls pending; - } ceval; -#endif - /* Used in Modules/_threadmodule.c. */ long num_threads; /* Support for runtime thread stack size tuning. @@ -93,6 +84,13 @@ struct _is { PyObject *pyexitmodule; uint64_t tstate_next_unique_id; + + struct _ceval { + /* This single variable consolidates all requests to break out of + the fast path in the eval loop. */ + _Py_atomic_int eval_breaker; + struct _pending_calls pending; + } ceval; }; PyAPI_FUNC(struct _is*) _PyInterpreterState_LookUpID(PY_INT64_T); From 158151f9a81ea7ad646d2f67c0433b963126aa08 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 1 Feb 2019 12:59:00 -0700 Subject: [PATCH 08/14] Factor out struct _ceval_interpreter_state. --- Include/internal/pycore_ceval.h | 7 +++++++ Include/internal/pycore_pystate.h | 7 +------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index a90b288201f834..1bdcdf527af49d 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -36,6 +36,13 @@ struct _pending_calls { int last; }; +struct _ceval_interpreter_state { + /* This single variable consolidates all requests to break out of + the fast path in the eval loop. */ + _Py_atomic_int eval_breaker; + struct _pending_calls pending; +}; + #include "pycore_gil.h" struct _ceval_runtime_state { diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 59039c2d899709..3ae2e0c60483eb 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -85,12 +85,7 @@ struct _is { uint64_t tstate_next_unique_id; - struct _ceval { - /* This single variable consolidates all requests to break out of - the fast path in the eval loop. */ - _Py_atomic_int eval_breaker; - struct _pending_calls pending; - } ceval; + struct _ceval_interpreter_state ceval; }; PyAPI_FUNC(struct _is*) _PyInterpreterState_LookUpID(PY_INT64_T); From 0c83e6f92129881b039edf1862e15420dd131bde Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 18 Feb 2019 15:24:45 -0700 Subject: [PATCH 09/14] Add a TODO. --- Python/pystate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/pystate.c b/Python/pystate.c index e606c0957da148..621ce5e0800890 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1406,6 +1406,7 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data) } // "Release" the data and/or the object. + // XXX Use _Py_AddPendingCall(). _call_in_interpreter(interp, _release_xidata, data); } From 40ab9ab02d6f98322f514aecbe57820a53d57d36 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Sep 2018 16:37:25 -0700 Subject: [PATCH 10/14] Use _Py_AddPendingCall() for releasing shared data. --- Python/pystate.c | 30 +++--------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 621ce5e0800890..1c39b5032522a3 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1353,7 +1353,7 @@ _PyObject_GetCrossInterpreterData(PyObject *obj, _PyCrossInterpreterData *data) return 0; } -static void +static int _release_xidata(void *arg) { _PyCrossInterpreterData *data = (_PyCrossInterpreterData *)arg; @@ -1361,30 +1361,7 @@ _release_xidata(void *arg) data->free(data->data); } Py_XDECREF(data->obj); -} - -static void -_call_in_interpreter(PyInterpreterState *interp, - void (*func)(void *), void *arg) -{ - /* We would use Py_AddPendingCall() if it weren't specific to the - * main interpreter (see bpo-33608). In the meantime we take a - * naive approach. - */ - PyThreadState *save_tstate = NULL; - if (interp != _PyInterpreterState_Get()) { - // XXX Using the "head" thread isn't strictly correct. - PyThreadState *tstate = PyInterpreterState_ThreadHead(interp); - // XXX Possible GILState issues? - save_tstate = PyThreadState_Swap(tstate); - } - - func(arg); - - // Switch back. - if (save_tstate != NULL) { - PyThreadState_Swap(save_tstate); - } + return 0; } void @@ -1406,8 +1383,7 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data) } // "Release" the data and/or the object. - // XXX Use _Py_AddPendingCall(). - _call_in_interpreter(interp, _release_xidata, data); + _Py_AddPendingCall(interp, 0, _release_xidata, data); } PyObject * From f354a86647857b155fd4df83f2a54b2196a6b694 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Jan 2019 11:58:49 -0700 Subject: [PATCH 11/14] Check the result of adding the pending call (for releasing XID). --- Python/pystate.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 1c39b5032522a3..816a5a4b82cc81 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1372,7 +1372,7 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data) return; } - // Switch to the original interpreter. + // Get the original interpreter. PyInterpreterState *interp = _PyInterpreterState_LookUpID(data->interp); if (interp == NULL) { // The intepreter was already destroyed. @@ -1381,9 +1381,16 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data) } return; } + // XXX There's a slight race here... + if (interp->finalizing) { + // XXX Someone leaked some memory... + return; + } // "Release" the data and/or the object. - _Py_AddPendingCall(interp, 0, _release_xidata, data); + if (_Py_AddPendingCall(interp, 0, _release_xidata, data) != 0) { + // XXX Queue full or couldn't get lock. Try again somehow? + } } PyObject * From 279ff04bf1072d422cbe5b9230ecc558c70f9ec0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Jan 2019 17:21:26 -0700 Subject: [PATCH 12/14] Make a copy of data-to-be-deleted. --- Python/pystate.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 816a5a4b82cc81..fee350100005c7 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1361,6 +1361,7 @@ _release_xidata(void *arg) data->free(data->data); } Py_XDECREF(data->obj); + PyMem_Free(data); return 0; } @@ -1381,14 +1382,22 @@ _PyCrossInterpreterData_Release(_PyCrossInterpreterData *data) } return; } - // XXX There's a slight race here... + // XXX There's an ever-so-slight race here... if (interp->finalizing) { // XXX Someone leaked some memory... return; } // "Release" the data and/or the object. - if (_Py_AddPendingCall(interp, 0, _release_xidata, data) != 0) { + _PyCrossInterpreterData *copied = PyMem_Malloc(sizeof(_PyCrossInterpreterData)); + if (copied == NULL) { + PyErr_SetString(PyExc_MemoryError, + "Not enough memory to preserve cross-interpreter data"); + PyErr_Print(); + return; + } + memcpy(copied, data, sizeof(_PyCrossInterpreterData)); + if (_Py_AddPendingCall(interp, 0, _release_xidata, copied) != 0) { // XXX Queue full or couldn't get lock. Try again somehow? } } From e356cad82502cb0bf5b90c5802ea0ca0d31cd8f7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 5 Apr 2019 12:36:56 -0600 Subject: [PATCH 13/14] Fix the pending-for-other-thread case. --- Lib/test/test_capi.py | 2 +- Python/ceval.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 7c68b2c0fc2c20..9e2fd02e4f0a0d 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -373,7 +373,7 @@ def pendingcalls_wait(self, l, n, context = None): def test_pendingcalls_threaded(self): #do every callback on a separate thread - n = 32 #total callbacks + n = 32 #total callbacks (see NPENDINGCALLS in pycore_ceval.h) threads = [] class foo(object):pass context = foo() diff --git a/Python/ceval.c b/Python/ceval.c index a2aaf5d0335fdc..70a7d9d937a4e8 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -466,7 +466,7 @@ make_pending_calls(PyInterpreterState *interp) // Thread mismatch, so move it to the end of the list // and start over. _Py_AddPendingCall(interp, thread_id, func, arg); - return 0; + goto error; } /* having released the lock, perform the callback */ From 48a23ef33c351a82082a35be958d868551d0297b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 5 Apr 2019 13:17:18 -0600 Subject: [PATCH 14/14] Drop out-of-date code. --- Python/ceval.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 70a7d9d937a4e8..b2fa20d616862c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -189,11 +189,6 @@ _PyEval_FiniThreads(void) destroy_gil(); assert(!gil_created()); - - if (_PyRuntime.ceval.pending.lock != NULL) { - PyThread_free_lock(_PyRuntime.ceval.pending.lock); - _PyRuntime.ceval.pending.lock = NULL; - } } void