Skip to content

Commit

Permalink
gh-104341: Adjust tstate_must_exit() to Respect Interpreter Finalizat…
Browse files Browse the repository at this point in the history
…ion (gh-104437)

With the move to a per-interpreter GIL, this check slipped through the cracks.
  • Loading branch information
ericsnowcurrently authored May 15, 2023
1 parent cb88ae6 commit 26baa74
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 10 deletions.
1 change: 1 addition & 0 deletions Include/cpython/pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ PyAPI_FUNC(const char *) _Py_gitidentifier(void);
PyAPI_FUNC(const char *) _Py_gitversion(void);

PyAPI_FUNC(int) _Py_IsFinalizing(void);
PyAPI_FUNC(int) _Py_IsInterpreterFinalizing(PyInterpreterState *interp);

/* Random */
PyAPI_FUNC(int) _PyOS_URandom(void *buffer, Py_ssize_t size);
Expand Down
18 changes: 18 additions & 0 deletions Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ struct _is {
int _initialized;
int finalizing;

/* Set by Py_EndInterpreter().
Use _PyInterpreterState_GetFinalizing()
and _PyInterpreterState_SetFinalizing()
to access it, don't access it directly. */
_Py_atomic_address _finalizing;

struct _obmalloc_state obmalloc;

struct _ceval_state ceval;
Expand Down Expand Up @@ -191,6 +198,17 @@ struct _is {
extern void _PyInterpreterState_Clear(PyThreadState *tstate);


static inline PyThreadState*
_PyInterpreterState_GetFinalizing(PyInterpreterState *interp) {
return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing);
}

static inline void
_PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) {
_Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate);
}


/* cross-interpreter data registry */

/* For now we use a global registry of shareable classes. An
Expand Down
2 changes: 1 addition & 1 deletion Modules/_asynciomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ future_init(FutureObj *fut, PyObject *loop)
if (is_true < 0) {
return -1;
}
if (is_true && !_Py_IsFinalizing()) {
if (is_true && !_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
/* Only try to capture the traceback if the interpreter is not being
finalized. The original motivation to add a `_Py_IsFinalizing()`
call was to prevent SIGSEGV when a Future is created in a __del__
Expand Down
3 changes: 2 additions & 1 deletion Modules/_io/bufferedio.c
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ _enter_buffered_busy(buffered *self)
"reentrant call inside %R", self);
return 0;
}
relax_locking = _Py_IsFinalizing();
PyInterpreterState *interp = PyInterpreterState_Get();
relax_locking = _Py_IsInterpreterFinalizing(interp);
Py_BEGIN_ALLOW_THREADS
if (!relax_locking)
st = PyThread_acquire_lock(self->lock, 1);
Expand Down
2 changes: 1 addition & 1 deletion Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ connection_close(pysqlite_Connection *self)
{
/* If close is implicitly called as a result of interpreter
* tear-down, we must not call back into Python. */
if (_Py_IsFinalizing()) {
if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
remove_callbacks(self->db);
}
(void)connection_exec_stmt(self, "ROLLBACK");
Expand Down
2 changes: 1 addition & 1 deletion Modules/_winapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ overlapped_dealloc(OverlappedObject *self)
{
/* The operation is no longer pending -- nothing to do. */
}
else if (_Py_IsFinalizing())
else if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get()))
{
/* The operation is still pending -- give a warning. This
will probably only happen on Windows XP. */
Expand Down
2 changes: 1 addition & 1 deletion Python/_warnings.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import)
PyObject *warnings_module, *obj;

/* don't try to import after the start of the Python finallization */
if (try_import && !_Py_IsFinalizing()) {
if (try_import && !_Py_IsInterpreterFinalizing(interp)) {
warnings_module = PyImport_Import(&_Py_ID(warnings));
if (warnings_module == NULL) {
/* Fallback to the C implementation if we cannot get
Expand Down
3 changes: 3 additions & 0 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ tstate_must_exit(PyThreadState *tstate)
After Py_Finalize() has been called, tstate can be a dangling pointer:
point to PyThreadState freed memory. */
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
if (finalizing == NULL) {
finalizing = _PyInterpreterState_GetFinalizing(tstate->interp);
}
return (finalizing != NULL && finalizing != tstate);
}

Expand Down
17 changes: 17 additions & 0 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,7 @@ Py_FinalizeEx(void)

/* Remaining daemon threads will automatically exit
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
_PyInterpreterState_SetFinalizing(tstate->interp, tstate);
_PyRuntimeState_SetFinalizing(runtime, tstate);
runtime->initialized = 0;
runtime->core_initialized = 0;
Expand Down Expand Up @@ -2142,6 +2143,10 @@ Py_EndInterpreter(PyThreadState *tstate)
Py_FatalError("not the last thread");
}

/* Remaining daemon threads will automatically exit
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
_PyInterpreterState_SetFinalizing(interp, tstate);

// XXX Call something like _PyImport_Disable() here?

_PyImport_FiniExternal(tstate->interp);
Expand All @@ -2152,6 +2157,18 @@ Py_EndInterpreter(PyThreadState *tstate)
finalize_interp_delete(tstate->interp);
}

int
_Py_IsInterpreterFinalizing(PyInterpreterState *interp)
{
/* We check the runtime first since, in a daemon thread,
interp might be dangling pointer. */
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
if (finalizing == NULL) {
finalizing = _PyInterpreterState_GetFinalizing(interp);
}
return finalizing != NULL;
}

/* Add the __main__ module */

static PyStatus
Expand Down
12 changes: 7 additions & 5 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1436,11 +1436,13 @@ PyThreadState_Clear(PyThreadState *tstate)

if (verbose && tstate->cframe->current_frame != NULL) {
/* bpo-20526: After the main thread calls
_PyRuntimeState_SetFinalizing() in Py_FinalizeEx(), threads must
exit when trying to take the GIL. If a thread exit in the middle of
_PyEval_EvalFrameDefault(), tstate->frame is not reset to its
previous value. It is more likely with daemon threads, but it can
happen with regular threads if threading._shutdown() fails
_PyInterpreterState_SetFinalizing() in Py_FinalizeEx()
(or in Py_EndInterpreter() for subinterpreters),
threads must exit when trying to take the GIL.
If a thread exit in the middle of _PyEval_EvalFrameDefault(),
tstate->frame is not reset to its previous value.
It is more likely with daemon threads, but it can happen
with regular threads if threading._shutdown() fails
(ex: interrupted by CTRL+C). */
fprintf(stderr,
"PyThreadState_Clear: warning: thread still has a frame\n");
Expand Down
4 changes: 4 additions & 0 deletions Python/sysmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ _PySys_ClearAuditHooks(PyThreadState *ts)
}

_PyRuntimeState *runtime = ts->interp->runtime;
/* The hooks are global so we have to check for runtime finalization. */
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
assert(finalizing == ts);
if (finalizing != ts) {
Expand Down Expand Up @@ -2039,6 +2040,9 @@ sys__clear_type_cache_impl(PyObject *module)
Py_RETURN_NONE;
}

/* Note that, for now, we do not have a per-interpreter equivalent
for sys.is_finalizing(). */

/*[clinic input]
sys.is_finalizing
Expand Down

0 comments on commit 26baa74

Please sign in to comment.