From 4000dcd22a7cdd67f0b9c6629efa6aaefa59c923 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 15 Sep 2018 11:57:29 -0600 Subject: [PATCH 1/5] Prevent adding pending calls if finalizing. --- Python/ceval.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index 373cde9a17bba3..a5b2b867d71ca1 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -365,6 +365,17 @@ _pop_pending_call(int (**func)(void *), void **arg) int Py_AddPendingCall(int (*func)(void *), void *arg) { + if (_PyRuntime.finalizing) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + PyErr_SetString(PyExc_SystemError, + "Py_AddPendingCall: cannot add pending calls " + "(Python shutting down)"); + PyErr_Print(); + PyErr_Restore(exc, val, tb); + return -1; + } + PyThread_acquire_lock(_PyRuntime.ceval.pending.lock, WAIT_LOCK); int result = _push_pending_call(func, arg); PyThread_release_lock(_PyRuntime.ceval.pending.lock); From 78a5d3a616026b51710f2956a7692e15bc57bd8e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Mar 2019 15:48:34 -0700 Subject: [PATCH 2/5] Make any remaining pending calls before shutdown. --- Include/internal/pycore_ceval.h | 2 ++ Python/ceval.c | 8 ++++++++ Python/pylifecycle.c | 12 ++++++++++++ 3 files changed, 22 insertions(+) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index c8e09bac074dba..ed67f94f98327a 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -11,6 +11,8 @@ extern "C" { #include "pycore_atomic.h" #include "pythread.h" +PyAPI_FUNC(int) _Py_MakePendingCalls(void); + struct _pending_calls { PyThread_type_lock lock; /* Request for running pending calls. */ diff --git a/Python/ceval.c b/Python/ceval.c index a5b2b867d71ca1..0f75bde2cd04c0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -459,6 +459,14 @@ make_pending_calls(void) return res; } +int +_Py_MakePendingCalls(void) +{ + assert(PyGILState_Check()); + + return make_pending_calls(); +} + /* Py_MakePendingCalls() is a simple wrapper for the sake of backward-compatibility. */ int diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 0902508429a3e7..808fbc676fb84a 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1462,8 +1462,20 @@ Py_EndInterpreter(PyThreadState *tstate) Py_FatalError("Py_EndInterpreter: thread still has a frame"); interp->finalizing = 1; + // Wrap up existing threads. wait_for_thread_shutdown(); + // Make any remaining pending calls. + if (_Py_atomic_load_relaxed(&_PyRuntime.ceval.pending.calls_to_do)) { + if (_Py_MakePendingCalls() < 0) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + PyErr_BadInternalCall(); + _PyErr_ChainExceptions(exc, val, tb); + PyErr_Print(); + } + } + call_py_exitfuncs(interp); if (tstate != interp->tstate_head || tstate->next != NULL) From 8571380db00f12d009edcbc54eea0b6fb7e4911c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 15 Mar 2019 10:26:26 -0600 Subject: [PATCH 3/5] Factor out finish_pending_calls(). --- Python/pylifecycle.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 808fbc676fb84a..a8057d791b738d 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -64,6 +64,7 @@ static _PyInitError initsite(void); static _PyInitError init_sys_streams(PyInterpreterState *interp); static _PyInitError initsigs(void); static void call_py_exitfuncs(PyInterpreterState *); +static void finish_pending_calls(_PyRuntimeState *); static void wait_for_thread_shutdown(void); static void call_ll_exitfuncs(void); @@ -1049,6 +1050,10 @@ Py_FinalizeEx(void) if (!_PyRuntime.initialized) return status; + // Make any remaining pending calls. + finish_pending_calls(&_PyRuntime); + + // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(); /* Get current thread state and interpreter pointer */ @@ -1059,7 +1064,7 @@ Py_FinalizeEx(void) * exit funcs may be relying on that. In particular, if some thread * or exit func is still waiting to do an import, the import machinery * expects Py_IsInitialized() to return true. So don't say the - * interpreter is uninitialized until after the exit funcs have run. + * runtime is uninitialized until after the exit funcs have run. * Note that Threading.py uses an exit func to do a join on all the * threads created thru it, so this also protects pending imports in * the threads created via Threading. @@ -1462,20 +1467,9 @@ Py_EndInterpreter(PyThreadState *tstate) Py_FatalError("Py_EndInterpreter: thread still has a frame"); interp->finalizing = 1; - // Wrap up existing threads. + // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(); - // Make any remaining pending calls. - if (_Py_atomic_load_relaxed(&_PyRuntime.ceval.pending.calls_to_do)) { - if (_Py_MakePendingCalls() < 0) { - PyObject *exc, *val, *tb; - PyErr_Fetch(&exc, &val, &tb); - PyErr_BadInternalCall(); - _PyErr_ChainExceptions(exc, val, tb); - PyErr_Print(); - } - } - call_py_exitfuncs(interp); if (tstate != interp->tstate_head || tstate->next != NULL) @@ -2102,6 +2096,23 @@ call_py_exitfuncs(PyInterpreterState *istate) PyErr_Clear(); } +static void +finish_pending_calls(_PyRuntimeState *runtime) +{ + _Py_atomic_int *calls_to_do = &runtime->ceval.pending.calls_to_do; + if (!_Py_atomic_load_relaxed(calls_to_do)) { + return; + } + + if (_Py_MakePendingCalls() < 0) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + PyErr_BadInternalCall(); + _PyErr_ChainExceptions(exc, val, tb); + PyErr_Print(); + } +} + /* Wait until threading._shutdown completes, provided the threading module was imported in the first place. The shutdown routine will wait until all non-daemon From f0665a3001bb9850060f24f18aaee4b5f0f5b332 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 15 Mar 2019 11:08:54 -0600 Subject: [PATCH 4/5] _Py_MakePendingCalls() -> _Py_FinishPendingCalls(). --- Include/internal/pycore_ceval.h | 3 +- Python/ceval.c | 73 +++++++++++++++++++++------------ Python/pylifecycle.c | 20 +-------- 3 files changed, 50 insertions(+), 46 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index ed67f94f98327a..2ead96c7abe32a 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -11,9 +11,10 @@ extern "C" { #include "pycore_atomic.h" #include "pythread.h" -PyAPI_FUNC(int) _Py_MakePendingCalls(void); +PyAPI_FUNC(void) _Py_FinishPendingCalls(void); struct _pending_calls { + int finishing; PyThread_type_lock lock; /* Request for running pending calls. */ _Py_atomic_int calls_to_do; diff --git a/Python/ceval.c b/Python/ceval.c index 0f75bde2cd04c0..c8453df10d3cd2 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -330,31 +330,33 @@ _PyEval_SignalReceived(void) /* Push one item onto the queue while holding the lock. */ static int -_push_pending_call(int (*func)(void *), void *arg) +_push_pending_call(struct _pending_calls *pending, + int (*func)(void *), void *arg) { - int i = _PyRuntime.ceval.pending.last; + int i = pending->last; int j = (i + 1) % NPENDINGCALLS; - if (j == _PyRuntime.ceval.pending.first) { + if (j == pending->first) { return -1; /* Queue full */ } - _PyRuntime.ceval.pending.calls[i].func = func; - _PyRuntime.ceval.pending.calls[i].arg = arg; - _PyRuntime.ceval.pending.last = j; + pending->calls[i].func = func; + pending->calls[i].arg = arg; + pending->last = j; return 0; } /* Pop one item off the queue while holding the lock. */ static void -_pop_pending_call(int (**func)(void *), void **arg) +_pop_pending_call(struct _pending_calls *pending, + int (**func)(void *), void **arg) { - int i = _PyRuntime.ceval.pending.first; - if (i == _PyRuntime.ceval.pending.last) { + int i = pending->first; + if (i == pending->last) { return; /* Queue empty */ } - *func = _PyRuntime.ceval.pending.calls[i].func; - *arg = _PyRuntime.ceval.pending.calls[i].arg; - _PyRuntime.ceval.pending.first = (i + 1) % NPENDINGCALLS; + *func = pending->calls[i].func; + *arg = pending->calls[i].arg; + pending->first = (i + 1) % NPENDINGCALLS; } /* This implementation is thread-safe. It allows @@ -365,7 +367,12 @@ _pop_pending_call(int (**func)(void *), void **arg) int Py_AddPendingCall(int (*func)(void *), void *arg) { - if (_PyRuntime.finalizing) { + struct _pending_calls *pending = &_PyRuntime.ceval.pending; + + PyThread_acquire_lock(pending->lock, WAIT_LOCK); + if (pending->finishing) { + PyThread_release_lock(pending->lock); + PyObject *exc, *val, *tb; PyErr_Fetch(&exc, &val, &tb); PyErr_SetString(PyExc_SystemError, @@ -375,10 +382,8 @@ Py_AddPendingCall(int (*func)(void *), void *arg) PyErr_Restore(exc, val, tb); return -1; } - - PyThread_acquire_lock(_PyRuntime.ceval.pending.lock, WAIT_LOCK); - int result = _push_pending_call(func, arg); - PyThread_release_lock(_PyRuntime.ceval.pending.lock); + int result = _push_pending_call(pending, func, arg); + PyThread_release_lock(pending->lock); /* signal main loop */ SIGNAL_PENDING_CALLS(); @@ -411,7 +416,7 @@ handle_signals(void) } static int -make_pending_calls(void) +make_pending_calls(struct _pending_calls* pending) { static int busy = 0; @@ -436,9 +441,9 @@ make_pending_calls(void) void *arg = NULL; /* pop one item off the queue while holding the lock */ - PyThread_acquire_lock(_PyRuntime.ceval.pending.lock, WAIT_LOCK); - _pop_pending_call(&func, &arg); - PyThread_release_lock(_PyRuntime.ceval.pending.lock); + PyThread_acquire_lock(pending->lock, WAIT_LOCK); + _pop_pending_call(pending, &func, &arg); + PyThread_release_lock(pending->lock); /* having released the lock, perform the callback */ if (func == NULL) { @@ -459,12 +464,28 @@ make_pending_calls(void) return res; } -int -_Py_MakePendingCalls(void) +void +_Py_FinishPendingCalls(void) { + struct _pending_calls *pending = &_PyRuntime.ceval.pending; + assert(PyGILState_Check()); - return make_pending_calls(); + PyThread_acquire_lock(pending->lock, WAIT_LOCK); + pending->finishing = 1; + PyThread_release_lock(pending->lock); + + if (!_Py_atomic_load_relaxed(&(pending->calls_to_do))) { + return; + } + + if (make_pending_calls(pending) < 0) { + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + PyErr_BadInternalCall(); + _PyErr_ChainExceptions(exc, val, tb); + PyErr_Print(); + } } /* Py_MakePendingCalls() is a simple wrapper for the sake @@ -481,7 +502,7 @@ Py_MakePendingCalls(void) return res; } - res = make_pending_calls(); + res = make_pending_calls(&_PyRuntime.ceval.pending); if (res != 0) { return res; } @@ -1031,7 +1052,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) if (_Py_atomic_load_relaxed( &_PyRuntime.ceval.pending.calls_to_do)) { - if (make_pending_calls() != 0) { + if (make_pending_calls(&_PyRuntime.ceval.pending) != 0) { goto error; } } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index a8057d791b738d..28e455cb2bbd8b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -64,7 +64,6 @@ static _PyInitError initsite(void); static _PyInitError init_sys_streams(PyInterpreterState *interp); static _PyInitError initsigs(void); static void call_py_exitfuncs(PyInterpreterState *); -static void finish_pending_calls(_PyRuntimeState *); static void wait_for_thread_shutdown(void); static void call_ll_exitfuncs(void); @@ -1051,7 +1050,7 @@ Py_FinalizeEx(void) return status; // Make any remaining pending calls. - finish_pending_calls(&_PyRuntime); + _Py_FinishPendingCalls(); // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(); @@ -2096,23 +2095,6 @@ call_py_exitfuncs(PyInterpreterState *istate) PyErr_Clear(); } -static void -finish_pending_calls(_PyRuntimeState *runtime) -{ - _Py_atomic_int *calls_to_do = &runtime->ceval.pending.calls_to_do; - if (!_Py_atomic_load_relaxed(calls_to_do)) { - return; - } - - if (_Py_MakePendingCalls() < 0) { - PyObject *exc, *val, *tb; - PyErr_Fetch(&exc, &val, &tb); - PyErr_BadInternalCall(); - _PyErr_ChainExceptions(exc, val, tb); - PyErr_Print(); - } -} - /* Wait until threading._shutdown completes, provided the threading module was imported in the first place. The shutdown routine will wait until all non-daemon From 6e13394f7fe607989e1f191d09dd6039a76d96ab Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 15 Mar 2019 11:14:27 -0600 Subject: [PATCH 5/5] Finish pending calls after shutting down threads instead of before. --- Python/pylifecycle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 28e455cb2bbd8b..c2d431c912b71e 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1049,9 +1049,6 @@ Py_FinalizeEx(void) if (!_PyRuntime.initialized) return status; - // Make any remaining pending calls. - _Py_FinishPendingCalls(); - // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(); @@ -1059,6 +1056,9 @@ Py_FinalizeEx(void) tstate = _PyThreadState_GET(); interp = tstate->interp; + // Make any remaining pending calls. + _Py_FinishPendingCalls(); + /* The interpreter is still entirely intact at this point, and the * exit funcs may be relying on that. In particular, if some thread * or exit func is still waiting to do an import, the import machinery