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
[C API] Detect refcount bugs on True/False in C extensions #89224
Comments
Writing C extensions using directly the C API is error prone. It's easy to add or forget a Py_INCREF or Py_DECREF. Adding Py_DECREF(Py_True) or Py_DECREF(Py_False) by mistake causes a surprising crash at Python exit: Enable tracemalloc to get the memory block allocation traceback Fatal Python error: _PyMem_DebugRawFree: bad ID: Allocated using API '', verified using API 'o' Current thread 0x00007f3f562fa740 (most recent call first): In my case, the bug occurs at Python exit, in code_dealloc(): "Py_XDECREF(co->co_consts);" destroys a tuple which contains True. It calls object_dealloc() which calls PyBool_Type.tp_dealloc(Py_True), but this object is allocated statically, and so its memory must not deallocated by the Python dynamic memory allocator. In debug mode, PyObject_Free() triggers a fatal error. Concrete example of such bug in PySide with Python 3.10 which is now stricter on reference counting (thanks to the work made in bpo-1635741 and for Python subinterpreters): I propose to add a specific deallocator functions on bool to detect such bug, to ease debugging. There is already a similar deallocator for the None singleton. |
To reproduce the bug, apply attached os_uname_refcount_bug.patch and call os.uname(): $ ./python -c 'import os; os.uname()'
(...)
Fatal Python error: _PyMem_DebugRawFree: bad ID: Allocated using API '', verified using API 'o'
(...) |
There was such bug in CPython stdlib, in the _testcapi module, see commit 310e2d2 of bpo-36854: commit 310e2d2
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index baa6907b7e..0908f3457f 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -6721,11 +6721,14 @@ PyInit__testcapi(void)
PyModule_AddObject(m, "instancemethod", (PyObject *)&PyInstanceMethod_Type);
PyModule_AddIntConstant(m, "the_number_three", 3);
+ PyObject *v;
#ifdef WITH_PYMALLOC
- PyModule_AddObject(m, "WITH_PYMALLOC", Py_True);
+ v = Py_True;
#else
- PyModule_AddObject(m, "WITH_PYMALLOC", Py_False);
+ v = Py_False;
#endif
+ Py_INCREF(v);
+ PyModule_AddObject(m, "WITH_PYMALLOC", v);
TestError = PyErr_NewException("_testcapi.error", NULL, NULL);
Py_INCREF(TestError); |
What about an even more flexible solution? |
Ah, that would be really much to store: every object with a refcount |
Christian Tismer: "What about an even more flexible solution? A debug option could memorize always the last object deallocated and give full info (the object's repr) before the crash would happen." Bugs in object_dealloc() are rare. Apart None, True and False, do you know other objects which must not be deleted? Calling repr(obj) can crash during late Python finalization for many reasons. It is slow and can consume significant memory. Not all object types detect loops (a container storing indirectly itself) and so it can fail with a MemoryError: repr() is not "safe". Using gdb, it is easy to dump the object currently being deleted: go the object_dealloc() frame and use "print self" command. Example using attached os_uname_refcount_bug.patch: $ gdb ./python
(...)
(gdb) run
>>> import os; os.uname()
>>> exit() Debug memory block at address p=0x886e60: API '' Enable tracemalloc to get the memory block allocation traceback Fatal Python error: _PyMem_DebugRawFree: bad ID: Allocated using API '', verified using API 'o' Current thread 0x00007ffff7c24740 (most recent call first): Program received signal SIGABRT, Aborted. (...) (gdb) frame 9 (gdb) print self |
Yes, what I can think of is the immutable empty tuple singleton |
I don't want to add a deallocator to bytes and int types to detect when their singleton is destroyed, since it would slow down C extensions which have no refcount bug. Maybe it could be added only if Py_DEBUG macro is defined. But I don't think that Py_DECREF() bugs on these singletons are common enough to justify these. I don't want to make Python debug build slower. IMO detecting refcount bugs on the most common singletons is enough: None, True, False, (), "". I implemented these checks because it was simple and had no major on performance, even on the debug build. I now consider that the issue is fully fixed ;-) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: