Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread

@pitrou

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']

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

bpo-36475: "PyEval_AcquireLock() and PyEval_AcquireThread() do not handle runtime finalization properly."
bpo-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 bpo-36479).

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

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

Navigation Menu

Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread #80608

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Document that PyEval_RestoreThread and PyGILState_Ensure can terminate the calling thread #80608

Description

Activity

pablogsal commented on Mar 25, 2019

vstinner commented on Mar 25, 2019

vstinner commented on Mar 25, 2019

pablogsal commented on Mar 25, 2019

vstinner commented on Mar 25, 2019

pablogsal commented on Mar 25, 2019

ericsnowcurrently commented on Mar 29, 2019

ericsnowcurrently commented on Mar 29, 2019

ericsnowcurrently commented on Mar 29, 2019

pablogsal commented on Apr 13, 2019

pablogsal commented on Apr 14, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions