python/cpython

3.12: "Python memory allocator called without holding the GIL" with `PyMem_Free` as `Py_AtExit`

The-Compiler opened this issue · 4 comments

When importing PyQt6 with Python 3.12.0b2 (as well as on the current git 3.12, 0b305e8), it segfaults on exit.

Program received signal SIGSEGV, Segmentation fault.
0x000055555572563a in _PyInterpreterState_GET () at ./Include/internal/pycore_pystate.h:126
126	   return tstate->interp;
(gdb) bt
#0  0x000055555572563a in _PyInterpreterState_GET () at ./Include/internal/pycore_pystate.h:126
#1  get_state () at Objects/obmalloc.c:866
#2  _PyObject_Free (ctx=<optimized out>, p=0x555555d527a0) at Objects/obmalloc.c:1850
#3  _PyObject_Free (ctx=0x0, p=0x555555d527a0) at Objects/obmalloc.c:1843
#4  0x00007ffff75e5351 in finalise () at sip_core.c:1807
#5  0x0000555555835235 in call_ll_exitfuncs (runtime=0x555555ba6380 <_PyRuntime>) at Python/pylifecycle.c:3002
#6  Py_FinalizeEx () at Python/pylifecycle.c:1966
#7  0x0000555555838075 in Py_FinalizeEx () at Python/pylifecycle.c:1970
#8  0x00005555558693cf in Py_RunMain () at Modules/main.c:691
[...]

A --with-pydebug build instead prints:

Fatal Python error: _PyMem_DebugFree: Python memory allocator called without holding the GIL                                                                                                                                                  
Python runtime state: finalizing (tstate=0x0000555555c76490)

and aborts in:

[...]
#4  0x0000555555847cc2 in fatal_error_exit (status=<optimized out>) at Python/pylifecycle.c:2693
#5  0x000055555584957c in fatal_error (fd=2, header=header@entry=1, prefix=prefix@entry=0x55555591fa10 <__func__.3> "_PyMem_DebugFree", msg=msg@entry=0x5555559202d8 "Python memory allocator called without holding the GIL", 
    status=status@entry=-1) at Python/pylifecycle.c:2874
#6  0x00005555558495e6 in _Py_FatalErrorFunc (func=func@entry=0x55555591fa10 <__func__.3> "_PyMem_DebugFree", msg=msg@entry=0x5555559202d8 "Python memory allocator called without holding the GIL") at Python/pylifecycle.c:2890
#7  0x000055555573f9b3 in _PyMem_DebugCheckGIL (func=func@entry=0x55555591fa10 <__func__.3> "_PyMem_DebugFree") at Objects/obmalloc.c:2271
#8  0x000055555573fa30 in _PyMem_DebugFree (ctx=0x555555c024d0 <_PyRuntime+208>, ptr=0x555555dd67b0) at Objects/obmalloc.c:2295
#9  0x00005555557408e7 in PyMem_Free (ptr=<optimized out>) at Objects/obmalloc.c:732
#10 0x00007ffff74e5296 in sip_api_free (mem=<optimized out>) at sip_core.c:1905
#11 0x00007ffff74f529a in sipOMFinalise (om=om@entry=0x7ffff74ff820 <cppPyMap>) at sip_object_map.c:69
#12 0x00007ffff74e5351 in finalise () at sip_core.c:1807
#13 0x00005555558479d0 in call_ll_exitfuncs (runtime=runtime@entry=0x555555c02400 <_PyRuntime>) at Python/pylifecycle.c:3002
#14 0x0000555555849153 in Py_FinalizeEx () at Python/pylifecycle.c:1966
#15 0x0000555555875aa7 in Py_RunMain () at Modules/main.c:691
[...]

To reproduce, pip install PyQt6 followed by e.g. python3 -c "import PyQt6.QtCore".


It's unclear to me whether this is an intended/expected regression or not, given that the behavior of sip (the C++ bindings behind PyQt) seems somewhat unkosher here. Its maintainer says:

finalise() is registered with Py_AtExit() so that it is called by Py_FinalizeEx()...

https://docs.python.org/3/c-api/sys.html#c.Py_AtExit

The docs say nothing about the state of the GIL when a cleanup function is called. The docs also state that the cleanup handler should not make calls to the Python API, so I suppose the state of the GIL shouldn't matter.

However finalise() does make calls to the Python API - but only to free memory (one of the stated purposes of Py_FinalizeEx()). It seems a little bit contradictory to me to disallow cleanup functions calling Py_MemFree().

If this new behaviour were to remain then sip would have to stop using the Python memory allocator and just use malloc() and free().

So from what I understand, it sounds like this was "if it breaks, you got to keep both pieces" territory to begin with?

However, I was able to bisect this to 6036c3e as part of #101161 ("Clarify GILState-related Code"), which in the commit message claims to be a refactor only:

The objective of this change is to help make the GILState-related code easier to understand. This mostly involves moving code around and some semantically equivalent refactors. However, there are a also a small number of slight changes in structure and behavior:

  • tstate_current is moved out of _PyRuntimeState.gilstate
  • autoTSSkey is moved out of _PyRuntimeState.gilstate
  • autoTSSkey is initialized earlier
  • autoTSSkey is re-initialized (after fork) earlier

Thus, I'm erring on the side of caution here, and decided to open an issue anyways - if this is something sip definitely shouldn't be doing that way, feel free to close!

The relevant sip sources can be found on their Mercurial server or in the PyPI sdist (sipbuild/module/source/13/).

cc @philthompson10 @ericsnowcurrently

The failure indicates that the function registered with Py_AtExit() is calling C-API functions. This is a problem because the runtime is already finalized at this point. Per the docs, "Since Python’s internal finalization will have completed before the cleanup function, no Python APIs should be called by func."

It sounds like sip is using PyObject_Malloc(), etc. to allocate non-object data. It should be using PyMem_RawMalloc(), etc. instead.

It's actually calling PyMem_Malloc() - the code was originally written for Python v1.5. There shouldn't be any problem changing this to PyMem_RawMalloc().

Thanks.

Yeah, the "mem" and "object" allocators default to pymalloc, so I had a 50/50 chance. 😄

Are we okay to close this?

We can re-open this if there's more to do.