Issue 36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread

Currently PyEval_RestoreThread and its callers (mainly PyGILState_Ensure) can terminate the thread if the interpreter is finalizing:

PyEval_RestoreThread(PyThreadState *tstate)
{
    if (tstate == NULL)
        Py_FatalError("PyEval_RestoreThread: NULL tstate");
    assert(gil_created());

    int err = errno;
    take_gil(tstate);
    /* _Py_Finalizing is protected by the GIL */
    if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate)) {
        drop_gil(tstate);
        PyThread_exit_thread();
        Py_UNREACHABLE();
    }
    errno = err;

    PyThreadState_Swap(tstate);
}

This behaviour that protects against problems due to daemon threads registered with the interpreter can be *very* surprising for C-extensions that are using these functions to implement callbacks that can call into Python. These callbacks threads are not owned by the interpreter and are usually joined by someone else, ending in deadlocks in many situations.

I propose to add a warning to the documentation to inform users about this situation.

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

_Py_IsFinalizing() check is redundant :-)

> This behaviour that protects against problems due to daemon threads registered with the interpreter can be *very* surprising for C-extensions that are using these functions to implement callbacks that can call into Python.

I really dislike the design of daemon threads. If it would be only me, I would prefer this nasty feature causing so much issues at Python shutdown.

> I really dislike the design of daemon threads. If it would be only me, I would prefer this nasty feature causing so much issues at Python shutdown.

Well, given that this happens as well in Python3.7 and before, at least we should document it and we can think about changing it in the future. But for now, but I suggest keeping both PRs separated (in case we really want to change anything).

> Well, given that this happens as well in Python3.7 and before, at least we should document it and we can think about changing it in the future. But for now, but I suggest keeping both PRs separated (in case we really want to change anything).

Right. It was just a general comment :-)

> Right. It was just a general comment :-)
Yep, daemon threads are evil :)

Related:

* #36475: "PyEval_AcquireLock() and PyEval_AcquireThread() do not handle runtime finalization properly."
* #36476: "Runtime finalization assumes all other threads have exited."

> > if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate))
>
> _Py_IsFinalizing() check is redundant :-)

Not really:

* _Py_IsFinalizing() checks if the runtime is finalizing
* _Py_CURRENTLY_FINALIZING checks if the current thread is the one
running finalization

> Currently PyEval_RestoreThread and its callers (mainly PyGILState_Ensure)
> can terminate the thread if the interpreter is finalizing:

s/interpreter/runtime/

Most likely (guaranteed?) it will be in the main interpreter, but it is actually not triggered by interpreter finalization (though it probably should be; I've opened #36479).

New changeset fde9b33dfeedd4a4ed723b12d2330979dc684760 by Pablo Galindo in branch 'master':
bpo-36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread (GH-12541)
https://github.com/python/cpython/commit/fde9b33dfeedd4a4ed723b12d2330979dc684760

New changeset 7723d0545c3369e1b2601b207c250c70ce90b75e by Pablo Galindo in branch '3.7':
[3.7] bpo-36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread (GH-12541) (GH-12820)
https://github.com/python/cpython/commit/7723d0545c3369e1b2601b207c250c70ce90b75e