Skip to content
Permalink
Browse files

bpo-37879: Suppress subtype_dealloc decref when base type is a C heap…

… type (GH-15323)

The instance destructor for a type is responsible for preparing
an instance for deallocation by decrementing the reference counts
of its referents.

If an instance belongs to a heap type, the type object of an instance
has its reference count decremented while for static types, which
are permanently allocated, the type object is unaffected by the
instance destructor.

Previously, the default instance destructor searched the class
hierarchy for an inherited instance destructor and, if present,
would invoke it.

Then, if the instance type is a heap type, it would decrement the
reference count of that heap type.  However, this could result in the
premature destruction of a type because the inherited instance
destructor should have already decremented the reference count
of the type object.

This change avoids the premature destruction of the type object
by suppressing the decrement of its reference count when an
inherited, non-default instance destructor has been invoked.

Finally, an assertion on the Py_SIZE of a type was deleted.  Heap
types have a non zero size, making this into an incorrect assertion.

#15323
  • Loading branch information...
eduardo-elizondo authored and encukou committed Sep 11, 2019
1 parent 7264e92 commit ff023ed36ea260ab64be5895f1f1f087c798987a
@@ -383,6 +383,92 @@ def __del__(self):
del L
self.assertEqual(PyList.num, 0)

def test_subclass_of_heap_gc_ctype_with_tpdealloc_decrefs_once(self):
class HeapGcCTypeSubclass(_testcapi.HeapGcCType):
def __init__(self):
self.value2 = 20
super().__init__()

subclass_instance = HeapGcCTypeSubclass()
type_refcnt = sys.getrefcount(HeapGcCTypeSubclass)

# Test that subclass instance was fully created
self.assertEqual(subclass_instance.value, 10)
self.assertEqual(subclass_instance.value2, 20)

# Test that the type reference count is only decremented once
del subclass_instance
self.assertEqual(type_refcnt - 1, sys.getrefcount(HeapGcCTypeSubclass))

def test_subclass_of_heap_gc_ctype_with_del_modifying_dunder_class_only_decrefs_once(self):
class A(_testcapi.HeapGcCType):
def __init__(self):
self.value2 = 20
super().__init__()

class B(A):
def __init__(self):
super().__init__()

def __del__(self):
self.__class__ = A
A.refcnt_in_del = sys.getrefcount(A)
B.refcnt_in_del = sys.getrefcount(B)

subclass_instance = B()
type_refcnt = sys.getrefcount(B)
new_type_refcnt = sys.getrefcount(A)

# Test that subclass instance was fully created
self.assertEqual(subclass_instance.value, 10)
self.assertEqual(subclass_instance.value2, 20)

del subclass_instance

# Test that setting __class__ modified the reference counts of the types
self.assertEqual(type_refcnt - 1, B.refcnt_in_del)
self.assertEqual(new_type_refcnt + 1, A.refcnt_in_del)

# Test that the original type already has decreased its refcnt
self.assertEqual(type_refcnt - 1, sys.getrefcount(B))

# Test that subtype_dealloc decref the newly assigned __class__ only once
self.assertEqual(new_type_refcnt, sys.getrefcount(A))

def test_c_subclass_of_heap_ctype_with_tpdealloc_decrefs_once(self):
subclass_instance = _testcapi.HeapCTypeSubclass()
type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclass)

# Test that subclass instance was fully created
self.assertEqual(subclass_instance.value, 10)
self.assertEqual(subclass_instance.value2, 20)

# Test that the type reference count is only decremented once
del subclass_instance
self.assertEqual(type_refcnt - 1, sys.getrefcount(_testcapi.HeapCTypeSubclass))

def test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once(self):
subclass_instance = _testcapi.HeapCTypeSubclassWithFinalizer()
type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclassWithFinalizer)
new_type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclass)

# Test that subclass instance was fully created
self.assertEqual(subclass_instance.value, 10)
self.assertEqual(subclass_instance.value2, 20)

# The tp_finalize slot will set __class__ to HeapCTypeSubclass
del subclass_instance

# Test that setting __class__ modified the reference counts of the types
self.assertEqual(type_refcnt - 1, _testcapi.HeapCTypeSubclassWithFinalizer.refcnt_in_del)
self.assertEqual(new_type_refcnt + 1, _testcapi.HeapCTypeSubclass.refcnt_in_del)

# Test that the original type already has decreased its refcnt
self.assertEqual(type_refcnt - 1, sys.getrefcount(_testcapi.HeapCTypeSubclassWithFinalizer))

# Test that subtype_dealloc decref the newly assigned __class__ only once
self.assertEqual(new_type_refcnt, sys.getrefcount(_testcapi.HeapCTypeSubclass))


class TestPendingCalls(unittest.TestCase):

@@ -0,0 +1,2 @@
Fix subtype_dealloc to suppress the type decref when the base type is a C
heap type
@@ -32,6 +32,8 @@
# error "_testcapi must test the public Python C API, not CPython internal C API"
#endif

static struct PyModuleDef _testcapimodule;

static PyObject *TestError; /* set to exception object in init */

/* Raise TestError with test_name + ": " + msg, and return NULL. */
@@ -6169,6 +6171,189 @@ static PyTypeObject MethodDescriptor2_Type = {
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | _Py_TPFLAGS_HAVE_VECTORCALL,
};

PyDoc_STRVAR(heapgctype__doc__,
"A heap type with GC, and with overridden dealloc.\n\n"
"The 'value' attribute is set to 10 in __init__.");

typedef struct {
PyObject_HEAD
int value;
} HeapCTypeObject;

static struct PyMemberDef heapctype_members[] = {
{"value", T_INT, offsetof(HeapCTypeObject, value)},
{NULL} /* Sentinel */
};

static int
heapctype_init(PyObject *self, PyObject *args, PyObject *kwargs)
{
((HeapCTypeObject *)self)->value = 10;
return 0;
}

static void
heapgcctype_dealloc(HeapCTypeObject *self)
{
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);
PyObject_GC_Del(self);
Py_DECREF(tp);
}

static PyType_Slot HeapGcCType_slots[] = {
{Py_tp_init, heapctype_init},
{Py_tp_members, heapctype_members},
{Py_tp_dealloc, heapgcctype_dealloc},
{Py_tp_doc, heapgctype__doc__},
{0, 0},
};

static PyType_Spec HeapGcCType_spec = {
"_testcapi.HeapGcCType",
sizeof(HeapCTypeObject),
0,
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
HeapGcCType_slots
};

PyDoc_STRVAR(heapctype__doc__,
"A heap type without GC, but with overridden dealloc.\n\n"
"The 'value' attribute is set to 10 in __init__.");

static void
heapctype_dealloc(HeapCTypeObject *self)
{
PyTypeObject *tp = Py_TYPE(self);
PyObject_Del(self);
Py_DECREF(tp);
}

static PyType_Slot HeapCType_slots[] = {
{Py_tp_init, heapctype_init},
{Py_tp_members, heapctype_members},
{Py_tp_dealloc, heapctype_dealloc},
{Py_tp_doc, heapctype__doc__},
{0, 0},
};

static PyType_Spec HeapCType_spec = {
"_testcapi.HeapCType",
sizeof(HeapCTypeObject),
0,
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
HeapCType_slots
};

PyDoc_STRVAR(heapctypesubclass__doc__,
"Subclass of HeapCType, without GC.\n\n"
"__init__ sets the 'value' attribute to 10 and 'value2' to 20.");

typedef struct {
HeapCTypeObject base;
int value2;
} HeapCTypeSubclassObject;

static int
heapctypesubclass_init(PyObject *self, PyObject *args, PyObject *kwargs)
{
/* Call __init__ of the superclass */
if (heapctype_init(self, args, kwargs) < 0) {
return -1;
}
/* Initialize additional element */
((HeapCTypeSubclassObject *)self)->value2 = 20;
return 0;
}

static struct PyMemberDef heapctypesubclass_members[] = {
{"value2", T_INT, offsetof(HeapCTypeSubclassObject, value2)},
{NULL} /* Sentinel */
};

static PyType_Slot HeapCTypeSubclass_slots[] = {
{Py_tp_init, heapctypesubclass_init},
{Py_tp_members, heapctypesubclass_members},
{Py_tp_doc, heapctypesubclass__doc__},
{0, 0},
};

static PyType_Spec HeapCTypeSubclass_spec = {
"_testcapi.HeapCTypeSubclass",
sizeof(HeapCTypeSubclassObject),
0,
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
HeapCTypeSubclass_slots
};

PyDoc_STRVAR(heapctypesubclasswithfinalizer__doc__,
"Subclass of HeapCType with a finalizer that reassigns __class__.\n\n"
"__class__ is set to plain HeapCTypeSubclass during finalization.\n"
"__init__ sets the 'value' attribute to 10 and 'value2' to 20.");

static int
heapctypesubclasswithfinalizer_init(PyObject *self, PyObject *args, PyObject *kwargs)
{
PyTypeObject *base = (PyTypeObject *)PyType_GetSlot(Py_TYPE(self), Py_tp_base);
initproc base_init = PyType_GetSlot(base, Py_tp_init);
base_init(self, args, kwargs);
return 0;
}

static void
heapctypesubclasswithfinalizer_finalize(PyObject *self)
{
PyObject *error_type, *error_value, *error_traceback, *m, *oldtype, *newtype;

/* Save the current exception, if any. */
PyErr_Fetch(&error_type, &error_value, &error_traceback);

m = PyState_FindModule(&_testcapimodule);
if (m == NULL) {
goto cleanup_finalize;
}
oldtype = PyObject_GetAttrString(m, "HeapCTypeSubclassWithFinalizer");
newtype = PyObject_GetAttrString(m, "HeapCTypeSubclass");
if (oldtype == NULL || newtype == NULL) {
goto cleanup_finalize;
}

if (PyObject_SetAttrString(self, "__class__", newtype) < 0) {
goto cleanup_finalize;
}
if (PyObject_SetAttrString(
oldtype, "refcnt_in_del", PyLong_FromSsize_t(Py_REFCNT(oldtype))) < 0) {
goto cleanup_finalize;
}
if (PyObject_SetAttrString(
newtype, "refcnt_in_del", PyLong_FromSsize_t(Py_REFCNT(newtype))) < 0) {
goto cleanup_finalize;
}

cleanup_finalize:
Py_XDECREF(oldtype);
Py_XDECREF(newtype);

/* Restore the saved exception. */
PyErr_Restore(error_type, error_value, error_traceback);
}

static PyType_Slot HeapCTypeSubclassWithFinalizer_slots[] = {
{Py_tp_init, heapctypesubclasswithfinalizer_init},
{Py_tp_members, heapctypesubclass_members},
{Py_tp_finalize, heapctypesubclasswithfinalizer_finalize},
{Py_tp_doc, heapctypesubclasswithfinalizer__doc__},
{0, 0},
};

static PyType_Spec HeapCTypeSubclassWithFinalizer_spec = {
"_testcapi.HeapCTypeSubclassWithFinalizer",
sizeof(HeapCTypeSubclassObject),
0,
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_FINALIZE,
HeapCTypeSubclassWithFinalizer_slots
};

static PyMethodDef meth_instance_methods[] = {
{"meth_varargs", meth_varargs, METH_VARARGS},
{"meth_varargs_keywords", (PyCFunction)(void(*)(void))meth_varargs_keywords, METH_VARARGS|METH_KEYWORDS},
@@ -6380,5 +6565,40 @@ PyInit__testcapi(void)
TestError = PyErr_NewException("_testcapi.error", NULL, NULL);
Py_INCREF(TestError);
PyModule_AddObject(m, "error", TestError);

PyObject *HeapGcCType = PyType_FromSpec(&HeapGcCType_spec);
if (HeapGcCType == NULL) {
return NULL;
}
PyModule_AddObject(m, "HeapGcCType", HeapGcCType);

PyObject *HeapCType = PyType_FromSpec(&HeapCType_spec);
if (HeapCType == NULL) {
return NULL;
}
PyObject *subclass_bases = PyTuple_Pack(1, HeapCType);
if (subclass_bases == NULL) {
return NULL;
}
PyObject *HeapCTypeSubclass = PyType_FromSpecWithBases(&HeapCTypeSubclass_spec, subclass_bases);
if (HeapCTypeSubclass == NULL) {
return NULL;
}
Py_DECREF(subclass_bases);
PyModule_AddObject(m, "HeapCTypeSubclass", HeapCTypeSubclass);

PyObject *subclass_with_finalizer_bases = PyTuple_Pack(1, HeapCTypeSubclass);
if (subclass_with_finalizer_bases == NULL) {
return NULL;
}
PyObject *HeapCTypeSubclassWithFinalizer = PyType_FromSpecWithBases(
&HeapCTypeSubclassWithFinalizer_spec, subclass_with_finalizer_bases);
if (HeapCTypeSubclassWithFinalizer == NULL) {
return NULL;
}
Py_DECREF(subclass_with_finalizer_bases);
PyModule_AddObject(m, "HeapCTypeSubclassWithFinalizer", HeapCTypeSubclassWithFinalizer);

PyState_AddModule(m, &_testcapimodule);
return m;
}
@@ -1157,11 +1157,9 @@ subtype_dealloc(PyObject *self)
/* Test whether the type has GC exactly once */

if (!PyType_IS_GC(type)) {
/* It's really rare to find a dynamic type that doesn't have
GC; it can only happen when deriving from 'object' and not
adding any slots or instance variables. This allows
certain simplifications: there's no need to call
clear_slots(), or DECREF the dict, or clear weakrefs. */
/* A non GC dynamic type allows certain simplifications:
there's no need to call clear_slots(), or DECREF the dict,
or clear weakrefs. */

/* Maybe call finalizer; exit early if resurrected */
if (type->tp_finalize) {
@@ -1177,7 +1175,6 @@ subtype_dealloc(PyObject *self)
/* Find the nearest base with a different tp_dealloc */
base = type;
while ((basedealloc = base->tp_dealloc) == subtype_dealloc) {
assert(Py_SIZE(base) == 0);
base = base->tp_base;
assert(base);
}
@@ -1189,8 +1186,10 @@ subtype_dealloc(PyObject *self)
assert(basedealloc);
basedealloc(self);

/* Can't reference self beyond this point */
Py_DECREF(type);
/* Only decref if the base type is not already a heap allocated type.
Otherwise, basedealloc should have decref'd it already */
if (type->tp_flags & Py_TPFLAGS_HEAPTYPE && !(base->tp_flags & Py_TPFLAGS_HEAPTYPE))
Py_DECREF(type);

/* Done */
return;
@@ -1289,8 +1288,9 @@ subtype_dealloc(PyObject *self)

/* Can't reference self beyond this point. It's possible tp_del switched
our type from a HEAPTYPE to a non-HEAPTYPE, so be careful about
reference counting. */
if (type->tp_flags & Py_TPFLAGS_HEAPTYPE)
reference counting. Only decref if the base type is not already a heap
allocated type. Otherwise, basedealloc should have decref'd it already */
if (type->tp_flags & Py_TPFLAGS_HEAPTYPE && !(base->tp_flags & Py_TPFLAGS_HEAPTYPE))
Py_DECREF(type);

endlabel:

0 comments on commit ff023ed

Please sign in to comment.
You can’t perform that action at this time.