[issue20891] PyGILState_Ensure on non-Python thread causes fatal error

[STINNER Victor]

STINNER Victor <victor.stinner at gmail.com> added the comment:

The "if (gil_created())" check can be found in the Py_BEGIN_ALLOW_THREADS / Py_END_ALLOW_THREADS macros which call PyEval_SaveThread() / PyEval_RestoreThread():

PyThreadState *
PyEval_SaveThread(void)
{
    PyThreadState *tstate = PyThreadState_Swap(NULL);
    if (tstate == NULL)
        Py_FatalError("PyEval_SaveThread: NULL tstate");
    if (gil_created()) <~~~~ HERE
        drop_gil(tstate);
    return tstate;
}

void
PyEval_RestoreThread(PyThreadState *tstate)
{
    if (tstate == NULL)
        Py_FatalError("PyEval_RestoreThread: NULL tstate");
    if (gil_created()) { <~~~~ HERE
        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);
}


I guess that the intent of dynamically created GIL is to reduce the "overhead" of the GIL when 100% of the code is run in single thread.

I understand that this potential overhead must be measured to decide if it's ok to always create the GIL.

This bug was reported once in 2014, then Marcin Kasperski asked for an update last month: 3 years later. It's not a common bug impacting many users. So if there is an overhead, I'm not sure that it's a good idea to change Python to fix this rare use case.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue20891>
_______________________________________