Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread

[pablogsal]

BPO	36427
Nosy	@pitrou, @vstinner, @ericsnowcurrently, @pablogsal
PRs	bpo-36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread #12541 [3.7] bpo-36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread (GH-12541) #12820

^{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 2019-04-14.02:49:46.980>
created_at = <Date 2019-03-25.22:14:35.939>
labels = ['interpreter-core', '3.7', '3.8']
title = 'Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread'
updated_at = <Date 2019-04-14.02:49:46.980>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2019-04-14.02:49:46.980>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2019-04-14.02:49:46.980>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2019-03-25.22:14:35.939>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 36427
keywords = ['patch']
message_count = 11.0
messages = ['338826', '338827', '338828', '338830', '338831', '338832', '339153', '339154', '339156', '340167', '340178']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'eric.snow', 'pablogsal']
pr_nums = ['12541', '12820']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue36427'
versions = ['Python 3.7', 'Python 3.8']

[pablogsal]

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.

[vstinner]

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

_Py_IsFinalizing() check is redundant :-)

[vstinner]

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.

[pablogsal]

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).

[vstinner]

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 :-)

[pablogsal]

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

[ericsnowcurrently]

Related:

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

[ericsnowcurrently]

> 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

[ericsnowcurrently]

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 bpo-36479).

[pablogsal]

New changeset fde9b33 by Pablo Galindo in branch 'master':
bpo-36427: Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread (GH-12541)
fde9b33

[pablogsal]

New changeset 7723d05 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)
7723d05