[subinterpreters] Exit threads when interpreter is finalizing rather than runtime.

[ericsnowcurrently]

BPO	36479
Nosy	@ncoghlan, @vstinner, @ericsnowcurrently, @pablogsal
PRs	bpo-36479: Exit threads when interpreter is finalizing rather than runtime #12679

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-06-28.23:54:03.593>
created_at = <Date 2019-03-29.21:59:02.182>
labels = ['expert-subinterpreters', 'interpreter-core', 'type-bug', '3.8']
title = '[subinterpreters] Exit threads when interpreter is finalizing rather than runtime.'
updated_at = <Date 2021-06-28.23:54:03.593>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2021-06-28.23:54:03.593>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-28.23:54:03.593>
closer = 'vstinner'
components = ['Interpreter Core', 'Subinterpreters']
creation = <Date 2019-03-29.21:59:02.182>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 36479
keywords = ['patch']
message_count = 6.0
messages = ['339155', '341882', '341884', '342039', '364589', '396680']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'vstinner', 'eric.snow', 'pablogsal']
pr_nums = ['12679']
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36479'
versions = ['Python 3.8']

[ericsnowcurrently]

Currently when a thread acquires the GIL, it subsequently exits if the runtime is finalizing. This helps with some cases like with stopping daemon threads.

This behavior should instead be triggered by the thread's interpreter finalizing rather than the runtime. This implies the following changes:

• in Py_EndInterpreter() move "interp-finalizing = 1;" to right after the call to "wait_for_thread_shutdown()"
• in Py_FinalizeEx() add "interp-finalizing = 1;" right after the call to "wait_for_thread_shutdown()"
• update _PyEval_EvalFrameDefault() (the eval loop) to check "interp->finalizing" instead of the runtime
• likewise update PyEval_RestoreThread() (as well as PyEval_AcquireThread() and PyEval_AcquireLock(); see bpo-36475)

The check will actually need to change from this:

  if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate)) {

to look like this:

  if (interp->finalizing && !_Py_CURRENTLY_FINALIZING(tstate)) {

If we could guarantee that only the main thread will ever call Py_FinalizeEx() then it would look more like this:

  if (interp->finalizing && tstate.id != _PyRuntime.main_thread {

[ncoghlan]

Pablo pointed out a problem with this change at the PyCon sprints: the thread cleanup code doesn't currently distinguish between "Python created daemon thread" and "thread created by the embedding application".

That's already somewhat problematic if an embedding application goes through multiple initialize/finalize cycles, but it's even more critical to make the distinction correctly if we move the daemon thread cleanup to interpreter shutdown.

[ncoghlan]

Another potential issue here is if a thread belonging to another interpreter attempts to access the interpreter being cleaned up.

[vstinner]

IMHO PR 12679 change should wait until Py_EndInterpreter() is reworked to better cleanup its state and its threads.

[vstinner]

I suggest to close this issue, I don't think that it can be implemented because of daemon threads.

--

This behavior should instead be triggered by the thread's interpreter finalizing rather than the runtime.

What is the motivation for that?

--

take_gil() cannot access interp->finalizing since interp memory is freed during Python finalization.

take_gil() can only rely on a global variable like _PyRuntime to check if a daemon thread must exit during the Python finalization.

I modified take_gil() recently to fix 2 or 3 different crashes caused by daemon threads during Python finalization. Extract of ceval.c:

/* Check if a Python thread must exit immediately, rather than taking the GIL
if Py_Finalize() has been called.

When this function is called by a daemon thread after Py_Finalize() has been
called, the GIL does no longer exist.

   tstate must be non-NULL. */
static inline int
tstate_must_exit(PyThreadState *tstate)
{
    /* bpo-39877: Access _PyRuntime directly rather than using
       tstate->interp->runtime to support calls from Python daemon threads.
       After Py_Finalize() has been called, tstate can be a dangling pointer:
       point to PyThreadState freed memory. */
    _PyRuntimeState *runtime = &_PyRuntime;
    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
    return (finalizing != NULL && finalizing != tstate);
}

And simplified take_gil():

static void
take_gil(PyThreadState *tstate)
{
    if (tstate_must_exit(tstate)) {
        /* bpo-39877: If Py_Finalize() has been called and tstate is not the
           thread which called Py_Finalize(), exit immediately the thread.

           This code path can be reached by a daemon thread after Py_Finalize()
           completes. In this case, tstate is a dangling pointer: points to
           PyThreadState freed memory. */
        PyThread_exit_thread();
    }

(... logic to acquire the GIL ...)

    if (must_exit) {
        /* bpo-36475: If Py_Finalize() has been called and tstate is not
           the thread which called Py_Finalize(), exit immediately the
           thread.

           This code path can be reached by a daemon thread which was waiting
           in take_gil() while the main thread called
           wait_for_thread_shutdown() from Py_Finalize(). */
        drop_gil(ceval, ceval2, tstate);
        PyThread_exit_thread();
    }
}

[vstinner]

I close the issue.