Skip to content

gh-104341: Adjust tstate_must_exit() to Respect Interpreter Finalization #104437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1798,6 +1798,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 @@ -2152,6 +2153,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 @@ -2162,6 +2167,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