Skip to content

gh-117142: ctypes: Fix memory leak of StgInfo #118139

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

Closed
wants to merge 14 commits into from
1 change: 1 addition & 0 deletions Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ typedef struct _heaptypeobject {
PyObject *ht_name, *ht_slots, *ht_qualname;
struct _dictkeysobject *ht_cached_keys;
PyObject *ht_module;
PyType_Spec *ht_static_spec;
char *_ht_tpname; // Storage for "tp_name"; see PyType_FromModuleAndSpec
struct _specialization_cache _spec_cache; // For use by the specializer.
/* here are optional user slots, followed by the members. */
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ extern PyObject * _PyType_GetMRO(PyTypeObject *type);
extern PyObject* _PyType_GetSubclasses(PyTypeObject *);
extern int _PyType_HasSubclasses(PyTypeObject *);
PyAPI_FUNC(PyObject *) _PyType_GetModuleByDef2(PyTypeObject *, PyTypeObject *, PyModuleDef *);
PyAPI_FUNC(PyTypeObject *) _PyType_GetBaseBySpec(PyTypeObject *, PyType_Spec *);

// PyType_Ready() must be called if _PyType_IsReady() is false.
// See also the Py_TPFLAGS_READY flag.
Expand Down
4 changes: 4 additions & 0 deletions Include/typeslots.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,7 @@
/* New in 3.10 */
#define Py_am_send 81
#endif
#if !defined(Py_LIMITED_API)
/* New in 3.13 */
#define Py_id_static_spec 82
#endif
2 changes: 1 addition & 1 deletion Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ def delx(self): del self.__x
'3P' # PyMappingMethods
'10P' # PySequenceMethods
'2P' # PyBufferProcs
'6P'
'7P'
'1PIP' # Specializer cache
)
class newstyleclass(object): pass
Expand Down
93 changes: 55 additions & 38 deletions Modules/_ctypes/_ctypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -451,23 +451,44 @@ class _ctypes.CType_Type "PyObject *" "clinic_state()->CType_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=8389fc5b74a84f2a]*/


static PyType_Spec pyctype_type_spec;


static inline int
PyStgInfo_FromSpec(PyObject *type, StgInfo **result)
{
*result = NULL;
PyTypeObject *PyCType_Type = _PyType_GetBaseBySpec(Py_TYPE(type),
&pyctype_type_spec);
if (PyCType_Type == NULL) {
// not a ctypes class.
return -1;
}
StgInfo *info = PyObject_GetTypeData(type, PyCType_Type);
assert(info != NULL);
if (!info->initialized) {
// StgInfo is not initialized. This happens in abstract classes.
return 0;
}
*result = info;
return 1;
}

static int
CType_Type_traverse(PyObject *self, visitproc visit, void *arg)
{
ctypes_state *st = get_module_state_by_def_final(Py_TYPE(self));
if (st && st->PyCType_Type) {
StgInfo *info;
if (PyStgInfo_FromType(st, self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
Py_VISIT(info->proto);
Py_VISIT(info->argtypes);
Py_VISIT(info->converters);
Py_VISIT(info->restype);
Py_VISIT(info->checker);
Py_VISIT(info->module);
}
StgInfo *info;
if (PyStgInfo_FromSpec(self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
Py_VISIT(info->proto);
Py_VISIT(info->argtypes);
Py_VISIT(info->converters);
Py_VISIT(info->restype);
Py_VISIT(info->checker);
Py_VISIT(info->module);
}
Py_VISIT(Py_TYPE(self));
return PyType_Type.tp_traverse(self, visit, arg);
Expand All @@ -488,37 +509,31 @@ ctype_clear_stginfo(StgInfo *info)
static int
CType_Type_clear(PyObject *self)
{
ctypes_state *st = get_module_state_by_def_final(Py_TYPE(self));
if (st && st->PyCType_Type) {
StgInfo *info;
if (PyStgInfo_FromType(st, self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
ctype_clear_stginfo(info);
}
StgInfo *info;
if (PyStgInfo_FromSpec(self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
ctype_clear_stginfo(info);
}
return PyType_Type.tp_clear(self);
}

static void
CType_Type_dealloc(PyObject *self)
{
ctypes_state *st = get_module_state_by_def_final(Py_TYPE(self));
if (st && st->PyCType_Type) {
StgInfo *info;
if (PyStgInfo_FromType(st, self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
PyMem_Free(info->ffi_type_pointer.elements);
info->ffi_type_pointer.elements = NULL;
PyMem_Free(info->format);
info->format = NULL;
PyMem_Free(info->shape);
info->shape = NULL;
ctype_clear_stginfo(info);
}
StgInfo *info;
if (PyStgInfo_FromSpec(self, &info) < 0) {
PyErr_WriteUnraisable(self);
}
if (info) {
PyMem_Free(info->ffi_type_pointer.elements);
info->ffi_type_pointer.elements = NULL;
PyMem_Free(info->format);
info->format = NULL;
PyMem_Free(info->shape);
info->shape = NULL;
ctype_clear_stginfo(info);
}
PyTypeObject *tp = Py_TYPE(self);
PyType_Type.tp_dealloc(self);
Expand Down Expand Up @@ -574,6 +589,8 @@ static PyType_Slot ctype_type_slots[] = {
{Py_tp_methods, ctype_methods},
// Sequence protocol.
{Py_sq_repeat, CType_Type_repeat},
// Memory layout ID.
{Py_id_static_spec, &pyctype_type_spec},
{0, NULL},
};

Expand Down
14 changes: 0 additions & 14 deletions Modules/_ctypes/ctypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,6 @@ get_module_state_by_def(PyTypeObject *cls)
return get_module_state(mod);
}

static inline ctypes_state *
get_module_state_by_def_final(PyTypeObject *cls)
{
if (cls->tp_mro == NULL) {
return NULL;
}
PyObject *mod = PyType_GetModuleByDef(cls, &_ctypesmodule);
if (mod == NULL) {
PyErr_Clear();
return NULL;
}
return get_module_state(mod);
}


extern PyType_Spec carg_spec;
extern PyType_Spec cfield_spec;
Expand Down
73 changes: 73 additions & 0 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3635,6 +3635,7 @@ type_new_alloc(type_new_ctx *ctx)
et->ht_name = Py_NewRef(ctx->name);
et->ht_module = NULL;
et->_ht_tpname = NULL;
et->ht_static_spec = NULL;

_PyObject_SetDeferredRefcount((PyObject *)et);

Expand Down Expand Up @@ -4661,6 +4662,17 @@ _PyType_FromMetaclass_impl(
}
}
break;
case Py_id_static_spec:
{
if (slot->pfunc && slot->pfunc != spec) {
PyErr_Format(PyExc_TypeError,
"Py_id_static_spec slot does not match "
"the type spec of '%s'", type->tp_name);
goto finally;
}
res->ht_static_spec = (PyType_Spec *)slot->pfunc;
}
break;
default:
{
/* Copy other slots directly */
Expand Down Expand Up @@ -4823,6 +4835,9 @@ PyType_GetSlot(PyTypeObject *type, int slot)
PyErr_BadInternalCall();
return NULL;
}
if (slot == Py_id_static_spec) {
return NULL;
}

parent_slot = *(void**)((char*)type + pyslot_offsets[slot].slot_offset);
if (parent_slot == NULL) {
Expand Down Expand Up @@ -4953,6 +4968,63 @@ _PyType_GetModuleByDef2(PyTypeObject *left, PyTypeObject *right,
return module;
}

static PyTypeObject *
get_base_by_spec_recursive(PyTypeObject *type, PyType_Spec *spec)
{
PyObject *bases = type->tp_bases;
Py_ssize_t n = PyTuple_GET_SIZE(bases);
for (Py_ssize_t i = 0; i < n; i++) {
PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(bases, i);
if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) {
continue;
}
if (((PyHeapTypeObject*)base)->ht_static_spec == spec) {
return base;
}
base = get_base_by_spec_recursive(base, spec);
if (base) {
return base;
}
}
return NULL;
}

PyTypeObject *
_PyType_GetBaseBySpec(PyTypeObject *type, PyType_Spec *spec)
{
assert(spec);
assert(PyType_Check(type));
PyTypeObject *res = NULL;

BEGIN_TYPE_LOCK()
PyObject *mro = lookup_tp_mro(type);
if (mro == NULL) {
res = get_base_by_spec_recursive(type, spec);
}
else {
assert(PyTuple_Check(mro));
for (Py_ssize_t i = PyTuple_GET_SIZE(mro) - 1; i >= 0; i--) {
PyObject *super = PyTuple_GET_ITEM(mro, i);
if(!_PyType_HasFeature((PyTypeObject *)super, Py_TPFLAGS_HEAPTYPE)) {
continue;
}
if (((PyHeapTypeObject*)super)->ht_static_spec == spec) {
res = _PyType_CAST(super);
break;
}
}
}
END_TYPE_LOCK()

if (res == NULL) {
PyErr_Format(
PyExc_TypeError,
"PyType_GetBaseBySpec: No superclass of '%s' has the given spec",
type->tp_name);
}
return res;
}

void *
PyObject_GetTypeData(PyObject *obj, PyTypeObject *cls)
{
Expand Down Expand Up @@ -5633,6 +5705,7 @@ type_dealloc(PyObject *self)
}
Py_XDECREF(et->ht_module);
PyMem_Free(et->_ht_tpname);
et->ht_static_spec = NULL;
Py_TYPE(type)->tp_free((PyObject *)type);
}

Expand Down
1 change: 1 addition & 0 deletions Objects/typeslots.inc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Objects/typeslots.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ def generate_typeslots(out=sys.stdout):
elif member.startswith("bf_"):
member = (f'{{offsetof(PyBufferProcs, {member}),'+
' offsetof(PyTypeObject, tp_as_buffer)}')
elif member == "id_static_spec":
member = '{-1, offsetof(PyHeapTypeObject, ht_static_spec)}'
res[int(m.group(2))] = member

M = max(res.keys())+1
Expand Down
Loading