[Python-Dev] Possible GIL/threading issue involving subprocess and PyMem_MALLOC...

[Trent Nelson]

    This seems odd to me so I wanted to see what others think.  The unit
    test Lib/unittest/test/test_runner.py:Test_TextRunner.test_warnings
    will eventually hit subprocess.Popen._communicate.

    The `mswindows` implementation of this method relies on threads to
    buffer stdin/stdout.  That'll eventually result in PyOs_StdioReadline
    being called without the GIL being held.  PyOs_StdioReadline calls
    PyMem_MALLOC, PyMem_FREE and possibly PyMem_REALLOC.

    On a debug build, these macros are redirected to their _PyMem_Debug*
    counterparts.  The call hierarchy for _PyMem_DebugMalloc looks like
    this:

        void *
        _PyMem_DebugMalloc(size_t nbytes)
        {
            return _PyObject_DebugMallocApi(_PYMALLOC_MEM_ID, nbytes);
        }

        /* generic debug memory api, with an "id" to
           identify the API in use */
        void *
        _PyObject_DebugMallocApi(char id, size_t nbytes)
        {
            uchar *p;           /* base address of malloc'ed block */
            uchar *tail;        /* p + 2*SST + nbytes ==
                                   pointer to tail pad bytes */
            size_t total;       /* nbytes + 4*SST */

            bumpserialno();
------------^^^^^^^^^^^^^^^

            total = nbytes + 4*SST;
            if (total < nbytes)
                /* overflow:  can't represent total as a size_t */
                return NULL;

            p = (uchar *)PyObject_Malloc(total);
-------------------------^^^^^^^^^^^^^^^^^^^^^^^
            if (p == NULL)
                return NULL;

            <snip>

    Both bumpserialno() and PyObject_Malloc affect global state.  The latter
    also has a bunch of LOCK() and UNLOCK() statements, but these end up being
    no-ops:

        /*
         * Python's threads are serialized,
         * so object malloc locking is disabled.
         */
        #define SIMPLELOCK_DECL(lock) /* simple lock declaration */
        #define SIMPLELOCK_INIT(lock) /* allocate (if needed) and ... */
        #define SIMPLELOCK_FINI(lock) /* free/destroy an existing */
        #define SIMPLELOCK_LOCK(lock) /* acquire released lock */
        #define SIMPLELOCK_UNLOCK(lock) /* release acquired lock */
        ...
        /*
         * This malloc lock
         */
        SIMPLELOCK_DECL(_malloc_lock)
        #define LOCK()          SIMPLELOCK_LOCK(_malloc_lock)
        #define UNLOCK()        SIMPLELOCK_UNLOCK(_malloc_lock)
        #define LOCK_INIT()     SIMPLELOCK_INIT(_malloc_lock)
        #define LOCK_FINI()     SIMPLELOCK_FINI(_malloc_lock)

    The PyObject_Malloc() one concerns me the most, as it affects huge
    amounts of global state.  Also, I just noticed PyOs_StdioReadline()
    can call PyErr_SetString, which will result in a bunch of other
    calls that should only be made whilst the GIL is held.

    So, like I said, this seems like a bit of a head scratcher.  Legit
    issue or am I missing something?

        Trent.