[New-bugs-announce] [issue13156] _PyGILState_Reinit assumes auto thread state will always exist which is not true.

[Graham Dumpleton]

New submission from Graham Dumpleton <Graham.Dumpleton at gmail.com>:

This is a followup bug report to fix wrong implementation of _PyGILState_Reinit() introduced by http://bugs.python.org/issue10517.

I don't have a standalone test case yet. Problem occurs under mod_wsgi with Python 2.7.2 and thus similarly 3.2 where _PyGILState_Reinit() was also added.

The Python code part which triggers the problem is:

    pid = os.fork()
    if pid:
       sys.stderr.write('Fork succeeded (PID=%s)\n' % pid)
    else:
       sys.stderr.write('Fork succeeded (child PID=%s)\n' % os.getpid())
       time.sleep(60.0)
       os._exit(0)

To trigger the problem requires this code be executed from a thread originally created outside of Python and then calling into a sub interpreter.

This thread would have created its own thread state object for the sub interpreter call since auto thread states don't work for sub interpreters. Further, it would never have called into the main interpreter so auto thread state simply doesn't exist for main interpreter.

When this thread has a fork() occur and _PyGILState_Reinit() is called, the call of PyGILState_GetThisThreadState() returns NULL because auto thread state for main interpreter was never initialised for this thread. When it then calls into PyThread_set_key_value() it is value of NULL and rather than set it, it thinks internally in find_key() you are doing a get which results in PyThread_set_key_value() returning -1 and so the fatal error.

So _PyGILState_Reinit() is broken because it assumes that an auto thread state will always exist for the thread for it to reinit, which will not always be the case.

The simple fix may be that if PyGILState_GetThisThreadState() returns NULL then don't do any reinit. Making that change does seem to fix the problem. Code that works then is:

void
_PyGILState_Reinit(void)
{
    PyThreadState *tstate = PyGILState_GetThisThreadState();

    if (tstate) {
        PyThread_delete_key(autoTLSkey);
        if ((autoTLSkey = PyThread_create_key()) == -1)
            Py_FatalError("Could not allocate TLS entry");

        /* re-associate the current thread state with the new key */
        if (PyThread_set_key_value(autoTLSkey, (void *)tstate) < 0)
            Py_FatalError("Couldn't create autoTLSkey mapping");
    }
}

Diff file also attached.

----------
components: Extension Modules
files: pystate.c.diff
keywords: patch
messages: 145383
nosy: grahamd, neologix
priority: normal
severity: normal
status: open
title: _PyGILState_Reinit assumes auto thread state will always exist which is not true.
type: crash
versions: Python 2.7, Python 3.2
Added file: http://bugs.python.org/file23385/pystate.c.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue13156>
_______________________________________