From 68b6dbcdeeee17c0f7d93b44c407ee8f25298f62 Mon Sep 17 00:00:00 2001 From: Piotr Sawicki Date: Tue, 20 Oct 2026 12:45:38 +0200 Subject: [PATCH 1/2] [mypyc] Fix native class template and base ownership on failures --- mypyc/lib-rt/misc_ops.c | 43 ++++++--------- mypyc/lib-rt/pythonsupport.c | 2 + mypyc/test-data/run-classes.test | 95 ++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 26 deletions(-) diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index 870cd4682ea7..b41450a2195f 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -230,6 +230,8 @@ PyObject *CPyType_FromTemplate(PyObject *template, if (orig_bases) { bases = update_bases(orig_bases); + if (bases == NULL) + goto error; // update_bases doesn't increment the refcount if nothing changes, // so we do it to make sure we have distinct "references" to both if (bases == orig_bases) @@ -252,17 +254,6 @@ PyObject *CPyType_FromTemplate(PyObject *template, if (!name) goto error; - if (template_->tp_doc) { - // cpython expects tp_doc to be heap-allocated so convert it here to - // avoid segfaults on deallocation. - Py_ssize_t size = strlen(template_->tp_doc) + 1; - char *doc = (char *)PyMem_Malloc(size); - if (!doc) - goto error; - memcpy(doc, template_->tp_doc, size); - template_->tp_doc = doc; - } - // Allocate the type and then copy the main stuff in. t = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, 0); if (!t) @@ -271,6 +262,21 @@ PyObject *CPyType_FromTemplate(PyObject *template, (char *)template_ + sizeof(PyVarObject), sizeof(PyTypeObject) - sizeof(PyVarObject)); + t->ht_type.tp_doc = NULL; + // Unlike the tp_doc slots of most other object, a heap type's tp_doc + // must be heap allocated. + if (template_->tp_doc) { + // Silently truncate the docstring if it contains a null byte + size_t size = strlen(template_->tp_doc) + 1; + char *doc = (char *)PyMem_Malloc(size); + if (doc == NULL) { + PyErr_NoMemory(); + goto error; + } + memcpy(doc, template_->tp_doc, size); + t->ht_type.tp_doc = doc; + } + if (bases != orig_bases) { if (PyObject_SetAttr((PyObject *)t, mypyc_interned_str.__orig_bases__, orig_bases) < 0) goto error; @@ -333,21 +339,6 @@ PyObject *CPyType_FromTemplate(PyObject *template, Py_XDECREF(dummy_class); - // Unlike the tp_doc slots of most other object, a heap type's tp_doc - // must be heap allocated. - if (template_->tp_doc) { - // Silently truncate the docstring if it contains a null byte - Py_ssize_t size = strlen(template_->tp_doc) + 1; - char *tp_doc = (char *)PyMem_Malloc(size); - if (tp_doc == NULL) { - PyErr_NoMemory(); - goto error; - } - - memcpy(tp_doc, template_->tp_doc, size); - t->ht_type.tp_doc = tp_doc; - } - #if PY_MINOR_VERSION == 11 // This is a hack. Python 3.11 doesn't include good public APIs to work with managed // dicts, which are the default for heap types. So we try to opt-out until Python 3.12. diff --git a/mypyc/lib-rt/pythonsupport.c b/mypyc/lib-rt/pythonsupport.c index b10fd0757a60..00f1f770d733 100644 --- a/mypyc/lib-rt/pythonsupport.c +++ b/mypyc/lib-rt/pythonsupport.c @@ -130,6 +130,7 @@ update_bases(PyObject *bases) /* If this is a first successful replacement, create new_bases list and copy previously encountered bases. */ if (!(new_bases = PyList_New(i))) { + Py_DECREF(new_base); goto error; } for (j = 0; j < i; j++) { @@ -140,6 +141,7 @@ update_bases(PyObject *bases) } j = PyList_GET_SIZE(new_bases); if (PyList_SetSlice(new_bases, j, j, new_base) < 0) { + Py_DECREF(new_base); goto error; } Py_DECREF(new_base); diff --git a/mypyc/test-data/run-classes.test b/mypyc/test-data/run-classes.test index 9c50e249ea3b..ae194c97dcda 100644 --- a/mypyc/test-data/run-classes.test +++ b/mypyc/test-data/run-classes.test @@ -10004,3 +10004,98 @@ for _ in range(100): assert owned_child(child) == (item, (item, number)) assert borrowed_child(child) is item assert read_numbers(child, 100) == 300.0 + + +[case testNativeClassDocAfterFailedConstruction] +from hidden import Base + +class C(Base): + pass + +[file hidden.pyi] +class Base: ... + +[file hidden.py] +class Base: + __slots__ = ("attribute",) + +[file driver.py] +import gc +import hidden +from testutil import assertRaises + +for _ in range(3): + with assertRaises(TypeError, "mypyc classes can't have __slots__"): + import native + gc.collect() +# Keep the base layout but permit the next initialization to complete. +hidden.Base.__slots__ = () +import native +# The template's tp_doc contains only the generated constructor signature. +# CPython extracts it into __text_signature__, leaving __doc__ empty. +assert native.C.__doc__ == "" +assert native.C.__text_signature__ == "()" + + +[case testNativeClassResolveBasesErrorRefcounts] +from hidden import Prefix, First, Second + +class C(Prefix, First, Second): + pass + +[file hidden.pyi] +class Prefix: ... +class First: ... +class Second: ... + +[file hidden.py] +class Prefix: + pass + +# Resolution fails before type creation, so a sentinel can stand in for a base. +# Unlike types, its refcount is reliable on free-threaded Python too. +Resolved = object() + +class Proxy: + def __init__(self, result): + self.result = result + self.failure = None + + @property + def __mro_entries__(self): + if self.failure == "lookup": + raise RuntimeError("base lookup failed") + return self.resolve + + def resolve(self, bases): + if self.failure == "call": + raise RuntimeError("base resolution failed") + return self.result + +First = Proxy((Resolved,)) +Second = Proxy([]) + +[file driver.py] +import gc +import sys +import hidden +from testutil import assertRaises + +# First creates a replacement list containing Prefix and Resolved before +# Second fails. Neither that list nor a non-tuple result may leak references. +tracked = (hidden.Resolved, hidden.First, hidden.Second, + hidden.First.result, hidden.Second.result) +for failure, error, message in [ + ("lookup", RuntimeError, "base lookup failed"), + ("call", RuntimeError, "base resolution failed"), + (None, TypeError, "__mro_entries__ must return a tuple"), +]: + hidden.Second.failure = failure + gc.collect() + before = [sys.getrefcount(obj) for obj in tracked] + for _ in range(3): + with assertRaises(error, message): + import native + gc.collect() + after = [sys.getrefcount(obj) for obj in tracked] + assert after == before, (failure, before, after) From 94f304e0b76ac4b1fd540eb15ca7cd4a5d40d676 Mon Sep 17 00:00:00 2001 From: Piotr Sawicki Date: Wed, 21 Oct 2026 13:33:17 +0200 Subject: [PATCH 2/2] Merge test cases with existing test --- mypyc/test-data/run-classes.test | 185 ++++++++++++++----------------- 1 file changed, 81 insertions(+), 104 deletions(-) diff --git a/mypyc/test-data/run-classes.test b/mypyc/test-data/run-classes.test index ae194c97dcda..e7ccfa75d02a 100644 --- a/mypyc/test-data/run-classes.test +++ b/mypyc/test-data/run-classes.test @@ -2272,29 +2272,101 @@ native.test() ok [case testNativeClassHiddenMetaclass] -# The stub hides the metaclass of the base class, so it's only detected at runtime -from hidden import Base +# The stub hides the base classes' metaclass and base resolution behavior. +from hidden import Prefix, Base, Second -class C(Base): +class C(Prefix, Base, Second): pass [file hidden.pyi] +class Prefix: ... class Base: ... +class Second: ... [file hidden.py] +class Prefix: + pass + class Meta(type): pass class Base(metaclass=Meta): pass +class Second: + pass + +class SlotsBase: + __slots__ = ("attribute",) + +# Resolution fails before type creation, so a sentinel can stand in for a base. +# Unlike types, its refcount is reliable on free-threaded Python too. +Resolved = object() + +class Proxy: + def __init__(self, result): + self.result = result + self.failure = None + + @property + def __mro_entries__(self): + if self.failure == "lookup": + raise RuntimeError("base lookup failed") + return self.resolve + + def resolve(self, bases): + if self.failure == "call": + raise RuntimeError("base resolution failed") + return self.result + [file driver.py] -try: - import native -except TypeError as e: - assert str(e) == "mypyc classes can't have a metaclass" -else: - assert False +import gc +import sys +import hidden +from testutil import assertRaises + +for _ in range(3): + with assertRaises(TypeError, "mypyc classes can't have a metaclass"): + import native + gc.collect() + +hidden.Base = hidden.SlotsBase +for _ in range(3): + with assertRaises(TypeError, "mypyc classes can't have __slots__"): + import native + gc.collect() + +second_base = hidden.Second +hidden.Base = hidden.Proxy((hidden.Resolved,)) +hidden.Second = hidden.Proxy([]) +# Base creates a replacement list containing Prefix and Resolved before +# Second fails. Neither that list nor a non-tuple result may leak references. +tracked = (hidden.Resolved, hidden.Base, hidden.Second, + hidden.Base.result, hidden.Second.result) +for failure, error, message in [ + ("lookup", RuntimeError, "base lookup failed"), + ("call", RuntimeError, "base resolution failed"), + (None, TypeError, "__mro_entries__ must return a tuple"), +]: + hidden.Second.failure = failure + gc.collect() + before = [sys.getrefcount(obj) for obj in tracked] + for _ in range(3): + with assertRaises(error, message): + import native + gc.collect() + after = [sys.getrefcount(obj) for obj in tracked] + assert after == before, (failure, before, after) + +# Keep the base layout but permit the next initialization to complete. +hidden.Base = hidden.SlotsBase +hidden.Base.__slots__ = () +hidden.Second = second_base +import native +# The template's tp_doc contains only the generated constructor signature. +# CPython extracts it into __text_signature__, leaving __doc__ empty. +assert native.C.__doc__ == "" +assert native.C.__text_signature__ == "()" [case testMetaclass] from meta import Meta @@ -10004,98 +10076,3 @@ for _ in range(100): assert owned_child(child) == (item, (item, number)) assert borrowed_child(child) is item assert read_numbers(child, 100) == 300.0 - - -[case testNativeClassDocAfterFailedConstruction] -from hidden import Base - -class C(Base): - pass - -[file hidden.pyi] -class Base: ... - -[file hidden.py] -class Base: - __slots__ = ("attribute",) - -[file driver.py] -import gc -import hidden -from testutil import assertRaises - -for _ in range(3): - with assertRaises(TypeError, "mypyc classes can't have __slots__"): - import native - gc.collect() -# Keep the base layout but permit the next initialization to complete. -hidden.Base.__slots__ = () -import native -# The template's tp_doc contains only the generated constructor signature. -# CPython extracts it into __text_signature__, leaving __doc__ empty. -assert native.C.__doc__ == "" -assert native.C.__text_signature__ == "()" - - -[case testNativeClassResolveBasesErrorRefcounts] -from hidden import Prefix, First, Second - -class C(Prefix, First, Second): - pass - -[file hidden.pyi] -class Prefix: ... -class First: ... -class Second: ... - -[file hidden.py] -class Prefix: - pass - -# Resolution fails before type creation, so a sentinel can stand in for a base. -# Unlike types, its refcount is reliable on free-threaded Python too. -Resolved = object() - -class Proxy: - def __init__(self, result): - self.result = result - self.failure = None - - @property - def __mro_entries__(self): - if self.failure == "lookup": - raise RuntimeError("base lookup failed") - return self.resolve - - def resolve(self, bases): - if self.failure == "call": - raise RuntimeError("base resolution failed") - return self.result - -First = Proxy((Resolved,)) -Second = Proxy([]) - -[file driver.py] -import gc -import sys -import hidden -from testutil import assertRaises - -# First creates a replacement list containing Prefix and Resolved before -# Second fails. Neither that list nor a non-tuple result may leak references. -tracked = (hidden.Resolved, hidden.First, hidden.Second, - hidden.First.result, hidden.Second.result) -for failure, error, message in [ - ("lookup", RuntimeError, "base lookup failed"), - ("call", RuntimeError, "base resolution failed"), - (None, TypeError, "__mro_entries__ must return a tuple"), -]: - hidden.Second.failure = failure - gc.collect() - before = [sys.getrefcount(obj) for obj in tracked] - for _ in range(3): - with assertRaises(error, message): - import native - gc.collect() - after = [sys.getrefcount(obj) for obj in tracked] - assert after == before, (failure, before, after)