Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
The-Compiler opened this issue Jun 12, 2023 · 4 comments
Closed

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

The-Compiler opened this issue Jun 12, 2023 · 4 comments
Labels
3.12 bugs and security fixes 3.13 new features, bugs and security fixes type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@The-Compiler
Copy link
Contributor

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-Compiler The-Compiler added the type-crash A hard crash of the interpreter, possibly with a core dump label Jun 12, 2023
@AlexWaygood AlexWaygood added 3.12 bugs and security fixes 3.13 new features, bugs and security fixes labels Jun 12, 2023
@ericsnowcurrently ericsnowcurrently mentioned this issue Jun 12, 2023
Open
@The-Compiler The-Compiler mentioned this issue Jun 12, 2023
Closed
@ericsnowcurrently
Copy link
Member

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.

@philthompson10
Copy link

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.

@ericsnowcurrently
Copy link
Member

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

Are we okay to close this?

@ericsnowcurrently
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.12 bugs and security fixes 3.13 new features, bugs and security fixes type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@The-Compiler @ericsnowcurrently @philthompson10 @AlexWaygood