[Python-checkins] GH-95589: Dont crash when subclassing extension classes with multiple inheritance (GH-96028)
markshannon
webhook-mailer at python.org
Wed Aug 17 07:50:58 EDT 2022
https://github.com/python/cpython/commit/b73e3b6d4a6c505f2869ae4d0a4a93241450cf32
commit: b73e3b6d4a6c505f2869ae4d0a4a93241450cf32
branch: main
author: Mark Shannon <mark at hotpy.org>
committer: markshannon <mark at hotpy.org>
date: 2022-08-17T12:50:53+01:00
summary:
GH-95589: Dont crash when subclassing extension classes with multiple inheritance (GH-96028)
* Treat tp_weakref and tp_dictoffset like other opaque slots for multiple inheritance.
* Document Py_TPFLAGS_MANAGED_DICT and Py_TPFLAGS_MANAGED_WEAKREF in what's new.
files:
A Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst
M Doc/whatsnew/3.12.rst
M Lib/test/test_capi.py
M Objects/typeobject.c
diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst
index 5926205ce5c0..9689d9df9dfc 100644
--- a/Doc/whatsnew/3.12.rst
+++ b/Doc/whatsnew/3.12.rst
@@ -450,6 +450,11 @@ New Features
inherit the ``Py_TPFLAGS_HAVE_VECTORCALL`` flag.
(Contributed by Petr Viktorin in :gh:`93274`.)
+ The :const:`Py_TPFLAGS_MANAGED_DICT` and :const:`Py_TPFLAGS_MANAGED_WEAKREF`
+ flags have been added. This allows extensions classes to support object
+ ``__dict__`` and weakrefs with less bookkeeping,
+ using less memory and with faster access.
+
Porting to Python 3.12
----------------------
@@ -486,6 +491,18 @@ Porting to Python 3.12
:c:func:`PyUnicode_FromFormatV`.
(Contributed by Philip Georgi in :gh:`95504`.)
+* Extension classes wanting to add a ``__dict__`` or weak reference slot
+ should use :const:`Py_TPFLAGS_MANAGED_DICT` and
+ :const:`Py_TPFLAGS_MANAGED_WEAKREF` instead of ``tp_dictoffset`` and
+ ``tp_weaklistoffset``, respectively.
+ The use of ``tp_dictoffset`` and ``tp_weaklistoffset`` is still
+ supported, but does not fully support multiple inheritance
+ (:gh: `95589`), and performance may be worse.
+ Classes declaring :const:`Py_TPFLAGS_MANAGED_DICT` should call
+ :c:func:`_PyObject_VisitManagedDict` and :c:func:`_PyObject_ClearManagedDict`
+ to traverse and clear their instance's dictionaries.
+ To clear weakrefs, call :c:func:`PyObject_ClearWeakRefs`, as before.
+
Deprecated
----------
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index e6516b33aec0..b698e34dec4e 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -677,21 +677,43 @@ def test_heaptype_with_custom_metaclass(self):
def test_multiple_inheritance_ctypes_with_weakref_or_dict(self):
- class Both1(_testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithDict):
+ with self.assertRaises(TypeError):
+ class Both1(_testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithDict):
+ pass
+ with self.assertRaises(TypeError):
+ class Both2(_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref):
+ pass
+
+ def test_multiple_inheritance_ctypes_with_weakref_or_dict_and_other_builtin(self):
+
+ with self.assertRaises(TypeError):
+ class C1(_testcapi.HeapCTypeWithDict, list):
+ pass
+
+ with self.assertRaises(TypeError):
+ class C2(_testcapi.HeapCTypeWithWeakref, list):
+ pass
+
+ class C3(_testcapi.HeapCTypeWithManagedDict, list):
pass
- class Both2(_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref):
+ class C4(_testcapi.HeapCTypeWithManagedWeakref, list):
pass
- for cls in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithDict2,
- _testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithWeakref2):
- for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithDict2,
- _testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithWeakref2):
- if cls is not cls2:
- class S(cls, cls2):
- pass
- class B1(Both1, cls):
+ inst = C3()
+ inst.append(0)
+ str(inst.__dict__)
+
+ inst = C4()
+ inst.append(0)
+ str(inst.__weakref__)
+
+ for cls in (_testcapi.HeapCTypeWithManagedDict, _testcapi.HeapCTypeWithManagedWeakref):
+ for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref):
+ class S(cls, cls2):
+ pass
+ class B1(C3, cls):
pass
- class B2(Both1, cls):
+ class B2(C4, cls):
pass
def test_pytype_fromspec_with_repeated_slots(self):
diff --git a/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst b/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst
new file mode 100644
index 000000000000..696e3c80db62
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst
@@ -0,0 +1,4 @@
+Extensions classes that set ``tp_dictoffset`` and ``tp_weaklistoffset``
+lose the support for multiple inheritance, but are now safe. Extension
+classes should use :const:`Py_TPFLAGS_MANAGED_DICT` and
+:const:`Py_TPFLAGS_MANAGED_WEAKREF` instead.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index f796a91b3c27..e8c36cf15399 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2330,36 +2330,13 @@ best_base(PyObject *bases)
return base;
}
-#define ADDED_FIELD_AT_OFFSET(name, offset) \
- (type->tp_ ## name && (base->tp_ ##name == 0) && \
- type->tp_ ## name + sizeof(PyObject *) == (offset) && \
- type->tp_flags & Py_TPFLAGS_HEAPTYPE)
-
static int
-extra_ivars(PyTypeObject *type, PyTypeObject *base)
+shape_differs(PyTypeObject *t1, PyTypeObject *t2)
{
- size_t t_size = type->tp_basicsize;
- size_t b_size = base->tp_basicsize;
-
- assert(t_size >= b_size); /* Else type smaller than base! */
- if (type->tp_itemsize || base->tp_itemsize) {
- /* If itemsize is involved, stricter rules */
- return t_size != b_size ||
- type->tp_itemsize != base->tp_itemsize;
- }
- /* Check for __dict__ and __weakrefs__ slots in either order */
- if (ADDED_FIELD_AT_OFFSET(weaklistoffset, t_size)) {
- t_size -= sizeof(PyObject *);
- }
- if ((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0 &&
- ADDED_FIELD_AT_OFFSET(dictoffset, t_size)) {
- t_size -= sizeof(PyObject *);
- }
- /* Check __weakrefs__ again, in case it precedes __dict__ */
- if (ADDED_FIELD_AT_OFFSET(weaklistoffset, t_size)) {
- t_size -= sizeof(PyObject *);
- }
- return t_size != b_size;
+ return (
+ t1->tp_basicsize != t2->tp_basicsize ||
+ t1->tp_itemsize != t2->tp_itemsize
+ );
}
static PyTypeObject *
@@ -2367,14 +2344,18 @@ solid_base(PyTypeObject *type)
{
PyTypeObject *base;
- if (type->tp_base)
+ if (type->tp_base) {
base = solid_base(type->tp_base);
- else
+ }
+ else {
base = &PyBaseObject_Type;
- if (extra_ivars(type, base))
+ }
+ if (shape_differs(type, base)) {
return type;
- else
+ }
+ else {
return base;
+ }
}
static void object_dealloc(PyObject *);
More information about the Python-checkins
mailing list