8000 [3.8] bpo-37879: Suppress subtype_dealloc decref when base type is a … · python/cpython@f02c74f · GitHub
[go: up one dir, main page]
Skip to content

Commit f02c74f

Browse files
eduardo-elizondoencukou
eduardo-elizondo
authored and
encukou
committed
[3.8] 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. (cherry picked from commit ff023ed) Co-authored-by: Eddie Elizondo <eduardo.elizondorueda@gmail.com>
1 parent fef5bdc commit f02c74f

File tree

4 files changed

+318
-10
lines changed

4 files changed

+318
-10
lines changed

‎Lib/test/test_capi.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,92 @@ def __del__(self):
383383
del L
384384
self.assertEqual(PyList.num, 0)
385385

386+
def test_subclass_of_heap_gc_ctype_with_tpdealloc_decrefs_once(self):
387+
class HeapGcCTypeSubclass(_testcapi.HeapGcCType):
388+
def __init__(self):
389+
self.value2 = 20
390+
super().__init__()
391+
392+
subclass_instance = HeapGcCTypeSubclass()
393+
type_refcnt = sys.getrefcount(HeapGcCTypeSubclass)
394+
395+
# Test that subclass instance was fully created
396+
self.assertEqual(subclass_instance.value, 10)
397+
self.assertEqual(subclass_instance.value2, 20)
398+
399+
# Test that the type reference count is only decremented once
400+
del subclass_instance
401+
self.assertEqual(type_refcnt - 1, sys.getrefcount(HeapGcCTypeSubclass))
402+
403+
def test_subclass_of_heap_gc_ctype_with_del_modifying_dunder_class_only_decrefs_once(self):
404+
class A(_testcapi.HeapGcCType):
405+
def __init__(self):
406+
self.value2 = 20
407+
super().__init__()
408+
409+
class B(A):
410+
def __init__(self):
411+
super().__init__()
412+
413+
def __del__(self):
414+
self.__class__ = A
415+
A.refcnt_in_del = sys.getrefcount(A)
416+
B.refcnt_in_del = sys.getrefcount(B)
417+
418+
subclass_instance = B()
419+
type_refcnt = sys.getrefcount(B)
420+
new_type_refcnt = sys.getrefcount(A)
421+
422+
# Test that subclass instance was fully created
423+
self.assertEqual(subclass_instance.value, 10)
424+
self.assertEqual(subclass_instance.value2, 20)
425+
426+
del subclass_instance
427+
428+
# Test that setting __class__ modified the reference counts of the types
429+
self.assertEqual(type_refcnt - 1, B.refcnt_in_del)
430+
self.assertEqual(new_type_refcnt + 1, A.refcnt_in_del)
431+
432+
# Test that the original type already has decreased its refcnt
433+
self.assertEqual(type_refcnt - 1, sys.getrefcount(B))
434+
435+
# Test that subtype_dealloc decref the newly assigned __class__ only once
436+
self.assertEqual(new_type_refcnt, sys.getrefcount(A))
437+
438+
def test_c_subclass_of_heap_ctype_with_tpdealloc_decrefs_once(self):
439+
subclass_instance = _testcapi.HeapCTypeSubclass()
440+
type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclass)
441+
442+
# Test that subclass instance was fully created
443+
self.assertEqual(subclass_instance.value, 10)
444+
self.assertEqual(subclass_instance.value2, 20)
445+
446+
# Test that the type reference count is only decremented once
447+
del subclass_instance
448+
self.assertEqual(type_refcnt - 1, sys.getrefcount(_testcapi.HeapCTypeSubclass))
449+
450+
def test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once(self):
451+
subclass_instance = _testcapi.HeapCTypeSubclassWithFinalizer()
452+
type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclassWithFinalizer)
453+
new_type_refcnt = sys.getrefcount(_testcapi.HeapCTypeSubclass)
454+
455+
# Test that subclass instance was fully created
456+
self.assertEqual(subclass_instance.value, 10)
457+
self.assertEqual(subclass_instance.value2, 20)
458+
459+
# The tp_finalize slot will set __class__ to HeapCTypeSubclass
460+
del subclass_instance
461+
462+
# Test that setting __class__ modified the reference counts of the types
463+
self.assertEqual(type_refcnt - 1, _testcapi.HeapCTypeSubclassWithFinalizer.refcnt_in_del)
464+
self.assertEqual(new_type_refcnt + 1, _testcapi.HeapCTypeSubclass.refcnt_in_del)
465+
466+
# Test that the original type already has decreased its refcnt
467+
self.assertEqual(type_refcnt - 1, sys.getrefcount(_testcapi.HeapCTypeSubclassWithFinalizer))
468+
469+
# Test that subtype_dealloc decref the newly assigned __class__ only once
470+
self.assertEqual(new_type_refcnt, sys.getrefcount(_testcapi.HeapCTypeSubclass))
471+
386472

387473
class TestPendingCalls(unittest.TestCase):
388474

‎Misc/NEWS.d/next/C API/2019-08-17-13-50-21.bpo-37879.CZeUem.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix subtype_dealloc to suppress the type decref when the base type is a C
2+
heap type

‎Modules/_testcapimodule.c

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
# error "_testcapi must test the public Python C API, not CPython internal C API"
3333
#endif
3434

35+
static struct PyModuleDef _testcapimodule;
36+
3537
static PyObject *TestError; /* set to exception object in init */
3638

3739
/* Raise TestError with test_name + ": " + msg, and return NULL. */
@@ -5955,6 +5957,189 @@ static PyTypeObject MethodDescriptor2_Type = {
59555957
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | _Py_TPFLAGS_HAVE_VECTORCALL,
59565958
};
59575959

5960+
PyDoc_STRVAR(heapgctype__doc__,
5961+
"A heap type with GC, and with overridden dealloc.\n\n"
5962+
"The 'value' attribute is set to 10 in __init__.");
5963+
5964+
typedef struct {
5965+
PyObject_HEAD
5966+
int value;
5967+
} HeapCTypeObject;
5968+
5969+
static struct PyMemberDef heapctype_members[] = {
5970+
{"value", T_INT, offsetof(HeapCTypeObject, value)},
5971+
{NULL} /* Sentinel */
5972+
};
5973+
5974+
static int
5975+
heapctype_init(PyObject *self, PyObject *args, PyObject *kwargs)
5976+
{
5977+
((HeapCTypeObject *)self)->value = 10;
5978+
return 0;
5979+
}
5980+
5981+
static void
5982+
heapgcctype_dealloc(HeapCTypeObject *self)
5983+
{
5984+
PyTypeObject *tp = Py_TYPE(self);
5985+
PyObject_GC_UnTrack(self);
5986+
PyObject_GC_Del(self);
5987+
Py_DECREF(tp);
5988+
}
5989+
5990+
static PyType_Slot HeapGcCType_slots[] = {
5991+
{Py_tp_init, heapctype_init},
5992+
{Py_tp_members, heapctype_members},
5993+
{Py_tp_dealloc, heapgcctype_dealloc},
5994+
{Py_tp_doc, heapgctype__doc__},
5995+
{0, 0},
5996+
};
5997+
5998+
static PyType_Spec HeapGcCType_spec = {
5999+
"_testcapi.HeapGcCType",
6000+
sizeof(HeapCTypeObject),
6001+
0,
6002+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
6003+
HeapGcCType_slots
6004+
};
6005+
6006+
PyDoc_STRVAR(heapctype__doc__,
6007+
"A heap type without GC, but with overridden dealloc.\n\n"
6008+
"The 'value' attribute is set to 10 in __init__.");
6009+
6010+
static void
6011+
heapctype_dealloc(HeapCTypeObject *self)
6012+
{
6013+
PyTypeObject *tp = Py_TYPE(self);
6014+
PyObject_Del(self);
6015+
Py_DECREF(tp);
6016+
}
6017+
6018+
static PyType_Slot HeapCType_slots[] = {
6019+
{Py_tp_init, heapctype_init},
6020+
{Py_tp_members, heapctype_members},
6021+
{Py_tp_dealloc, heapctype_dealloc},
6022+
{Py_tp_doc, heapctype__doc__},
6023+
{0, 0},
6024+
};
6025+
6026+
static PyType_Spec HeapCType_spec = {
6027+
"_testcapi.HeapCType",
6028+
sizeof(HeapCTypeObject),
6029+
0,
6030+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
6031+
HeapCType_slots
6032+
};
6033+
6034+
PyDoc_STRVAR(heapctypesubclass__doc__,
6035+
"Subclass of HeapCType, without GC.\n\n"
6036+
"__init__ sets the 'value' attribute to 10 and 'value2' to 20.");
6037+
6038+
typedef struct {
6039+
HeapCTypeObject base;
6040+
int value2;
6041+
} HeapCTypeSubclassObject;
6042+
6043+
static int
6044+
heapctypesubclass_init(PyObject *self, PyObject *args, PyObject *kwargs)
6045+
{
6046+
/* Call __init__ of the superclass */
6047+
if (heapctype_init(self, args, kwargs) < 0) {
6048+
return -1;
6049+
}
6050+
/* Initialize additional element */
6051+
((HeapCTypeSubclassObject *)self)->value2 = 20;
6052+
return 0;
6053+
}
6054+
6055+
static struct PyMemberDef heapctypesubclass_members[] = {
6056+
{"value2", T_INT, offsetof(HeapCTypeSubclassObject, value2)},
6057+
{NULL} /* Sentinel */
6058+
};
6059+
6060+
static PyType_Slot HeapCTypeSubclass_slots[] = {
6061+
{Py_tp_init, heapctypesubclass_init},
6062+
{Py_tp_members, heapctypesubclass_members},
6063+
{Py_tp_doc, heapctypesubclass__doc__},
6064+
{0, 0},
6065+
};
6066+
6067+
static PyType_Spec HeapCTypeSubclass_spec = {
6068+
"_testcapi.HeapCTypeSubclass",
6069+
sizeof(HeapCTypeSubclassObject),
6070+
0,
6071+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
6072+
HeapCTypeSubclass_slots
6073+
};
6074+
6075+
PyDoc_STRVAR(heapctypesubclasswithfinalizer__doc__,
6076+
"Subclass of HeapCType with a finalizer that reassigns __class__.\n\n"
6077+
"__class__ is set to plain HeapCTypeSubclass during finalization.\n"
6078+
"__init__ sets the 'value' attribute to 10 and 'value2' to 20.");
6079+
6080+
static int
6081+
heapctypesubclasswithfinalizer_init(PyObject *self, PyObject *args, PyObject *kwargs)
6082+
{
6083+
PyTypeObject *base = (PyTypeObject *)PyType_GetSlot(Py_TYPE(self), Py_tp_base);
6084+
initproc base_init = PyType_GetSlot(base, Py_tp_init);
6085+
base_init(self, args, kwargs);
6086+
return 0;
6087+
}
6088+
6089+
static void
6090+
heapctypesubclasswithfinalizer_finalize(PyObject *self)
6091+
{
6092+
PyObject *error_type, *error_value, *error_traceback, *m, *oldtype, *newtype;
6093+
6094+
/* Save the current exception, if any. */
6095+
PyErr_Fetch(&error_type, &error_value, &error_traceback);
6096+
6097+
m = PyState_FindModule(&_testcapimodule);
6098+
if (m == NULL) {
6099+
goto cleanup_finalize;
6100+
}
6101+
oldtype = PyObject_GetAttrString(m, "HeapCTypeSubclassWithFinalizer");
6102+
newtype = PyObject_GetAttrString(m, "HeapCTypeSubclass");
6103+
if (oldtype == NULL || newtype == NULL) {
6104+
goto cleanup_finalize;
6105+
}
6106+
6107+
if (PyObject_SetAttrString(self, "__class__", newtype) < 0) {
6108+
goto cleanup_finalize;
6109+
}
6110+
if (PyObject_SetAttrString(
6111+
oldtype, "refcnt_in_del", PyLong_FromSsize_t(Py_REFCNT(oldtype))) < 0) {
6112+
goto cleanup_finalize;
6113+
}
6114+
if (PyObject_SetAttrString(
6115+
newtype, "refcnt_in_del", PyLong_FromSsize_t(Py_REFCNT(newtype))) < 0) {
6116+
goto cleanup_finalize;
6117+
}
6118+
6119+
cleanup_finalize:
6120+
Py_XDECREF(oldtype);
6121+
Py_XDECREF(newtype);
6122+
6123+
/* Restore the saved exception. */
6124+
PyErr_Restore(error_type, error_value, error_traceback);
6125+
}
6126+
6127+
static PyType_Slot HeapCTypeSubclassWithFinalizer_slots[] = {
6128+
{Py_tp_init, heapctypesubclasswithfinalizer_init},
6129+
{Py_tp_members, heapctypesubclass_members},
6130+
{Py_tp_finalize, heapctypesubclasswithfinalizer_finalize},
6131+
{Py_tp_doc, heapctypesubclasswithfinalizer__doc__},
6132+
{0, 0},
6133+
};
6134+
6135+
static PyType_Spec HeapCTypeSubclassWithFinalizer_spec = {
6136+
"_testcapi.HeapCTypeSubclassWithFinalizer",
6137+
sizeof(HeapCTypeSubclassObject),
6138+
0,
6139+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_FINALIZE,
6140+
HeapCTypeSubclassWithFinalizer_slots
6141+
};
6142+
59586143

59596144
static struct PyModuleDef _testcapimodule = {
59606145
PyModuleDef_HEAD_INIT,
@@ -6089,5 +6274,40 @@ PyInit__testcapi(void)
60896274
TestError = PyErr_NewException("_testcapi.error", NULL, NULL);
60906275
Py_INCREF(TestError);
60916276
PyModule_AddObject(m, "error", TestError);
6277+
6278+
PyObject *HeapGcCType = PyType_FromSpec(&HeapGcCType_spec);
6279+
if (HeapGcCType == NULL) {
6280+
return NULL;
6281+
}
6282+
PyModule_AddObject(m, "HeapGcCType", HeapGcCType);
6283+
6284+
PyObject *HeapCType = PyType_FromSpec(&HeapCType_spec);
6285+
if (HeapCType == NULL) {
6286+
return NULL;
6287+
}
6288+
PyObject *subclass_bases = PyTuple_Pack(1, HeapCType);
6289+
if (subclass_bases == NULL) {
6290+
return NULL;
6291+
}
6292+
PyObject *HeapCTypeSubclass = PyType_FromSpecWithBases(&HeapCTypeSubclass_spec, subclass_bases);
6293+
if (HeapCTypeSubclass == NULL) {
6294+
return NULL;
6295+
}
6296+
Py_DECREF(subclass_bases);
6297+
PyModule_AddObject(m, "HeapCTypeSubclass", HeapCTypeSubclass);
6298+
6299+
PyObject *subclass_with_finalizer_bases = PyTuple_Pack(1, HeapCTypeSubclass);
6300+
if (subclass_with_finalizer_bases == NULL) {
6301+
return NULL;
6302+
}
6303+
PyObject *HeapCTypeSubclassWithFinalizer = PyType_FromSpecWithBases(
6304+
&HeapCTypeSubclassWithFinalizer_spec, subclass_with_finalizer_bases);
6305+
if (HeapCTypeSubclassWithFinalizer == NULL) {
6306+
return NULL;
6307+
}
6308+
Py_DECREF(subclass_with_finalizer_bases);
6309+
PyModule_AddObject(m, "HeapCTypeSubclassWithFinalizer", HeapCTypeSubclassWithFinalizer);
6310+
6311+
PyState_AddModule(m, &_testcapimodule);
60926312
return m;
60936313
}

‎Objects/typeobject.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,11 +1157,9 @@ subtype_dealloc(PyObject *self)
11571157
/* Test whether the type has GC exactly once */
11581158

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

11661164
/* Maybe call finalizer; exit early if resurrected */
11671165
if (type->tp_finalize) {
@@ -1177,7 +1175,6 @@ subtype_dealloc(PyObject *self)
11771175
/* Find the nearest base with a different tp_dealloc */
11781176
base = type;
11791177
while ((basedealloc = base->tp_dealloc) == subtype_dealloc) {
1180-
assert(Py_SIZE(base) == 0);
11811178
base = base->tp_base;
11821179
assert(base);
11831180
}
@@ -1189,8 +1186,10 @@ subtype_dealloc(PyObject *self)
11891186
assert(basedealloc);
11901187
basedealloc(self);
11911188

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

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

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

12961296
endlabel:

0 commit comments

Comments
 (0)
0