From 675bec56b9cb29fc9c0dcd16c9d3dafd45b1be82 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Sun, 30 Mar 2025 20:13:29 +0300 Subject: [PATCH 01/30] gh-92810: Avoid O(n^2) complexity in ABCMeta.__subclasscheck__ Signed-off-by: Martynov Maxim --- Lib/_py_abc.py | 37 ++++++++-- Lib/test/test_abc.py | 30 +++++++- Lib/test/test_abstract_numbers.py | 30 ++++---- Modules/_abc.c | 113 +++++++++++++++++++++++------- 4 files changed, 162 insertions(+), 48 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index c870ae9048b4f1..3a707696d87fcf 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -1,5 +1,7 @@ from _weakrefset import WeakSet +_UNSET = object() + def get_cache_token(): """Returns the current ABC cache token. @@ -65,8 +67,23 @@ def register(cls, subclass): if issubclass(cls, subclass): # This would create a cycle, which is bad for the algorithm below raise RuntimeError("Refusing to create an inheritance cycle") + + # Actual registration cls._abc_registry.add(subclass) - ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache + + # Recursively register the subclass in all ABC bases, to avoid recursive lookups. + # >>> class Ancestor1(ABC): pass + # >>> class Ancestor2(Ancestor1): pass + # >>> class Other: pass + # >>> Ancestor2.register(Other) # same result for Ancestor1.register(Other) + # >>> issubclass(Other, Ancestor2) is True + # >>> issubclass(Other, Ancestor1) is True + for pcls in cls.__mro__: + if hasattr(pcls, "_abc_registry"): + pcls._abc_registry.add(subclass) + + # Invalidate negative cache + ABCMeta._abc_invalidation_counter += 1 return subclass def _dump_registry(cls, file=None): @@ -137,11 +154,19 @@ def __subclasscheck__(cls, subclass): if issubclass(subclass, rcls): cls._abc_cache.add(subclass) return True - # Check if it's a subclass of a subclass (recursive) - for scls in cls.__subclasses__(): - if issubclass(subclass, scls): - cls._abc_cache.add(subclass) - return True + + # Check if it's a subclass of a subclass (recursive). + # >>> class Ancestor: __subclasses__ = lambda: [Other] + # >>> class Other: pass + # >>> isinstance(Other, Ancestor) is True + # Do not iterate over cls.__subclasses__() because it returns the entire class tree, + # not just direct children, which leads to O(n^2) lookup. + original_subclasses = getattr(cls, "__dict__", {}).get("__subclasses__", _UNSET) + if original_subclasses is not _UNSET: + for scls in original_subclasses(): + if issubclass(subclass, scls): + cls._abc_cache.add(subclass) + return True # No dice; update negative cache cls._abc_negative_cache.add(subclass) return False diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 80ee9e0ba56e75..b5604439f69951 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -411,7 +411,35 @@ class MyInt(int): self.assertIsInstance(42, A) self.assertIsInstance(42, (A,)) - def test_issubclass_bad_arguments(self): + def test_subclasses(self): + class A: + pass + + class B: + pass + + class C: + pass + + class Sup(metaclass=abc_ABCMeta): + __subclasses__ = lambda: [A, B] + + self.assertIsSubclass(A, Sup) + self.assertIsSubclass(A, (Sup,)) + self.assertIsInstance(A(), Sup) + self.assertIsInstance(A(), (Sup,)) + + self.assertIsSubclass(B, Sup) + self.assertIsSubclass(B, (Sup,)) + self.assertIsInstance(B(), Sup) + self.assertIsInstance(B(), (Sup,)) + + self.assertNotIsSubclass(C, Sup) + self.assertNotIsSubclass(C, (Sup,)) + self.assertNotIsInstance(C(), Sup) + self.assertNotIsInstance(C(), (Sup,)) + + def test_subclasses_bad_arguments(self): class A(metaclass=abc_ABCMeta): pass diff --git a/Lib/test/test_abstract_numbers.py b/Lib/test/test_abstract_numbers.py index 72232b670cdb89..cf071d2c933dd2 100644 --- a/Lib/test/test_abstract_numbers.py +++ b/Lib/test/test_abstract_numbers.py @@ -24,11 +24,11 @@ def not_implemented(*args, **kwargs): class TestNumbers(unittest.TestCase): def test_int(self): - self.assertTrue(issubclass(int, Integral)) - self.assertTrue(issubclass(int, Rational)) - self.assertTrue(issubclass(int, Real)) - self.assertTrue(issubclass(int, Complex)) - self.assertTrue(issubclass(int, Number)) + self.assertIsSubclass(int, Integral) + self.assertIsSubclass(int, Rational) + self.assertIsSubclass(int, Real) + self.assertIsSubclass(int, Complex) + self.assertIsSubclass(int, Number) self.assertEqual(7, int(7).real) self.assertEqual(0, int(7).imag) @@ -38,11 +38,11 @@ def test_int(self): self.assertEqual(1, int(7).denominator) def test_float(self): - self.assertFalse(issubclass(float, Integral)) - self.assertFalse(issubclass(float, Rational)) - self.assertTrue(issubclass(float, Real)) - self.assertTrue(issubclass(float, Complex)) - self.assertTrue(issubclass(float, Number)) + self.assertNotIsSubclass(float, Integral) + self.assertNotIsSubclass(float, Rational) + self.assertIsSubclass(float, Real) + self.assertIsSubclass(float, Complex) + self.assertIsSubclass(float, Number) self.assertEqual(7.3, float(7.3).real) self.assertEqual(0, float(7.3).imag) @@ -50,11 +50,11 @@ def test_float(self): self.assertEqual(-7.3, float(-7.3).conjugate()) def test_complex(self): - self.assertFalse(issubclass(complex, Integral)) - self.assertFalse(issubclass(complex, Rational)) - self.assertFalse(issubclass(complex, Real)) - self.assertTrue(issubclass(complex, Complex)) - self.assertTrue(issubclass(complex, Number)) + self.assertNotIsSubclass(complex, Integral) + self.assertNotIsSubclass(complex, Rational) + self.assertNotIsSubclass(complex, Real) + self.assertIsSubclass(complex, Complex) + self.assertIsSubclass(complex, Number) c1, c2 = complex(3, 2), complex(4,1) # XXX: This is not ideal, but see the comment in math_trunc(). diff --git a/Modules/_abc.c b/Modules/_abc.c index d6a953b336025d..153f5ab13e008f 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -578,6 +578,8 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) if (result < 0) { return NULL; } + + /* Actual registration */ _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; @@ -588,6 +590,49 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) } Py_DECREF(impl); + /* Recursively register the subclass in all ABC bases, to avoid recursive lookups. + >>> class Ancestor1(ABC): pass + >>> class Ancestor2(Ancestor1): pass + >>> class Other: pass + >>> Ancestor2.register(Other) # same result for Ancestor1.register(Other) + >>> issubclass(Other, Ancestor2) is True + >>> issubclass(Other, Ancestor1) is True + */ + PyObject *mro = PyObject_GetAttrString(self, "__mro__"); + if (mro == NULL) { + return NULL; + } + + if (!PyTuple_Check(mro)) { + PyErr_SetString(PyExc_TypeError, "__mro__ is not tuple"); + goto error; + } + + for (Py_ssize_t pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) { + PyObject *base_class = PyTuple_GET_ITEM(mro, pos); // borrowed + PyObject *base_class_data; + + if (PyObject_GetOptionalAttr(base_class, &_Py_ID(_abc_impl), + &base_class_data) < 0) { + goto error; + } + + if (PyErr_Occurred()) { + goto error; + } + + if (base_class_data == NULL) { + // not ABC class + continue; + } + + _abc_data *base_class_state = _abc_data_CAST(base_class_data); + if (_add_to_weak_set(base_class_state, &base_class_state->_abc_registry, subclass) < 0) { + Py_DECREF(base_class_data); + goto error; + } + } + /* Invalidate negative cache */ increment_invalidation_counter(get_abc_state(module)); @@ -602,6 +647,10 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) } } return Py_NewRef(subclass); + +error: + Py_XDECREF(mro); + return NULL; } @@ -710,6 +759,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, PyErr_SetString(PyExc_TypeError, "issubclass() arg 1 must be a class"); return NULL; } + PyTypeObject *cls = (PyTypeObject *)self; PyObject *ok, *subclasses = NULL, *result = NULL; _abcmodule_state *state = NULL; @@ -800,32 +850,43 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } - /* 6. Check if it's a subclass of a subclass (recursive). */ - subclasses = PyObject_CallMethod(self, "__subclasses__", NULL); - if (subclasses == NULL) { - goto end; - } - if (!PyList_Check(subclasses)) { - PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list"); - goto end; - } - for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) { - PyObject *scls = PyList_GetItemRef(subclasses, pos); - if (scls == NULL) { - goto end; - } - int r = PyObject_IsSubclass(subclass, scls); - Py_DECREF(scls); - if (r > 0) { - if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { - goto end; - } - result = Py_True; - goto end; - } - if (r < 0) { - goto end; - } + /* 6. Check if it's a subclass of a subclass (recursive). + >>> class Ancestor: __subclasses__ = lambda: [Other] + >>> class Other: pass + >>> isinstance(Other, Ancestor) is True + + Do not iterate over cls.__subclasses__() because it returns the entire class tree, + not just direct children, which leads to O(n^2) lookup. + */ + PyObject *dict = _PyType_GetDict(cls); // borrowed + PyObject *subclasses_own_method = PyDict_GetItemString(dict, "__subclasses__"); // borrowed + if (subclasses_own_method) { + subclasses = PyObject_CallNoArgs(subclasses_own_method); + if (subclasses == NULL) { + goto end; + } + if (!PyList_Check(subclasses)) { + PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list"); + goto end; + } + for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) { + PyObject *scls = PyList_GetItemRef(subclasses, pos); + if (scls == NULL) { + goto end; + } + int r = PyObject_IsSubclass(subclass, scls); + Py_DECREF(scls); + if (r > 0) { + if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { + goto end; + } + result = Py_True; + goto end; + } + if (r < 0) { + goto end; + } + } } /* No dice; update negative cache. */ From 701ecc9aad99c87d108e3c0ae3cdfe2882694f85 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 31 Mar 2025 14:21:46 +0300 Subject: [PATCH 02/30] gh-92810: Apply fixes Signed-off-by: Martynov Maxim --- Lib/_py_abc.py | 14 +-------- Lib/test/test_abc.py | 59 ----------------------------------- Lib/test/test_isinstance.py | 22 +++++++++++++ Modules/_abc.c | 61 +++++++------------------------------ 4 files changed, 34 insertions(+), 122 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 3a707696d87fcf..ceff0467df2876 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -51,6 +51,7 @@ def __new__(mcls, name, bases, namespace, /, **kwargs): cls._abc_cache = WeakSet() cls._abc_negative_cache = WeakSet() cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter + cls._prevent_recursion = 0 return cls def register(cls, subclass): @@ -154,19 +155,6 @@ def __subclasscheck__(cls, subclass): if issubclass(subclass, rcls): cls._abc_cache.add(subclass) return True - - # Check if it's a subclass of a subclass (recursive). - # >>> class Ancestor: __subclasses__ = lambda: [Other] - # >>> class Other: pass - # >>> isinstance(Other, Ancestor) is True - # Do not iterate over cls.__subclasses__() because it returns the entire class tree, - # not just direct children, which leads to O(n^2) lookup. - original_subclasses = getattr(cls, "__dict__", {}).get("__subclasses__", _UNSET) - if original_subclasses is not _UNSET: - for scls in original_subclasses(): - if issubclass(subclass, scls): - cls._abc_cache.add(subclass) - return True # No dice; update negative cache cls._abc_negative_cache.add(subclass) return False diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index b5604439f69951..50d8da6f132ec3 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -411,34 +411,6 @@ class MyInt(int): self.assertIsInstance(42, A) self.assertIsInstance(42, (A,)) - def test_subclasses(self): - class A: - pass - - class B: - pass - - class C: - pass - - class Sup(metaclass=abc_ABCMeta): - __subclasses__ = lambda: [A, B] - - self.assertIsSubclass(A, Sup) - self.assertIsSubclass(A, (Sup,)) - self.assertIsInstance(A(), Sup) - self.assertIsInstance(A(), (Sup,)) - - self.assertIsSubclass(B, Sup) - self.assertIsSubclass(B, (Sup,)) - self.assertIsInstance(B(), Sup) - self.assertIsInstance(B(), (Sup,)) - - self.assertNotIsSubclass(C, Sup) - self.assertNotIsSubclass(C, (Sup,)) - self.assertNotIsInstance(C(), Sup) - self.assertNotIsInstance(C(), (Sup,)) - def test_subclasses_bad_arguments(self): class A(metaclass=abc_ABCMeta): pass @@ -457,37 +429,6 @@ class C: with self.assertRaises(TypeError): issubclass(C(), A) - # bpo-34441: Check that issubclass() doesn't crash on bogus - # classes. - bogus_subclasses = [ - None, - lambda x: [], - lambda: 42, - lambda: [42], - ] - - for i, func in enumerate(bogus_subclasses): - class S(metaclass=abc_ABCMeta): - __subclasses__ = func - - with self.subTest(i=i): - with self.assertRaises(TypeError): - issubclass(int, S) - - # Also check that issubclass() propagates exceptions raised by - # __subclasses__. - class CustomError(Exception): ... - exc_msg = "exception from __subclasses__" - - def raise_exc(): - raise CustomError(exc_msg) - - class S(metaclass=abc_ABCMeta): - __subclasses__ = raise_exc - - with self.assertRaisesRegex(CustomError, exc_msg): - issubclass(int, S) - def test_subclasshook(self): class A(metaclass=abc.ABCMeta): @classmethod diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index daad00e86432d0..5aac482aa72007 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -351,6 +351,28 @@ class B: with support.infinite_recursion(25): self.assertRaises(RecursionError, issubclass, X(), int) + def test_override_subclasses(self): + class A: pass + class B: pass + + class Parent1: + @classmethod + def __subclasses__(self): + return [A, B] + + class Parent2: + __subclasses__ = lambda: [A, B] + + self.assertNotIsInstance(A(), Parent1) + self.assertNotIsInstance(B(), Parent1) + self.assertNotIsSubclass(A, Parent1) + self.assertNotIsSubclass(B, Parent1) + + self.assertNotIsInstance(A(), Parent2) + self.assertNotIsInstance(B(), Parent2) + self.assertNotIsSubclass(A, Parent2) + self.assertNotIsSubclass(B, Parent2) + def blowstack(fxn, arg, compare_to): # Make sure that calling isinstance with a deeply nested tuple for its diff --git a/Modules/_abc.c b/Modules/_abc.c index 153f5ab13e008f..28ebed195ab656 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -603,8 +603,8 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) return NULL; } - if (!PyTuple_Check(mro)) { - PyErr_SetString(PyExc_TypeError, "__mro__ is not tuple"); + if (!PyTuple_CheckExact(mro)) { + PyErr_SetString(PyExc_TypeError, "__mro__ must be an exact tuple"); goto error; } @@ -612,12 +612,10 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) PyObject *base_class = PyTuple_GET_ITEM(mro, pos); // borrowed PyObject *base_class_data; - if (PyObject_GetOptionalAttr(base_class, &_Py_ID(_abc_impl), - &base_class_data) < 0) { - goto error; - } - - if (PyErr_Occurred()) { + if (PyObject_GetOptionalAttr(base_class, + &_Py_ID(_abc_impl), + &base_class_data) < 0) + { goto error; } @@ -627,8 +625,11 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) } _abc_data *base_class_state = _abc_data_CAST(base_class_data); - if (_add_to_weak_set(base_class_state, &base_class_state->_abc_registry, subclass) < 0) { - Py_DECREF(base_class_data); + int res = _add_to_weak_set(base_class_state, + &base_class_state->_abc_registry, + subclass); + Py_DECREF(base_class_data); + if (res < 0) { goto error; } } @@ -763,7 +764,6 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, PyObject *ok, *subclasses = NULL, *result = NULL; _abcmodule_state *state = NULL; - Py_ssize_t pos; int incache; _abc_data *impl = _get_impl(module, self); if (impl == NULL) { @@ -850,45 +850,6 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } - /* 6. Check if it's a subclass of a subclass (recursive). - >>> class Ancestor: __subclasses__ = lambda: [Other] - >>> class Other: pass - >>> isinstance(Other, Ancestor) is True - - Do not iterate over cls.__subclasses__() because it returns the entire class tree, - not just direct children, which leads to O(n^2) lookup. - */ - PyObject *dict = _PyType_GetDict(cls); // borrowed - PyObject *subclasses_own_method = PyDict_GetItemString(dict, "__subclasses__"); // borrowed - if (subclasses_own_method) { - subclasses = PyObject_CallNoArgs(subclasses_own_method); - if (subclasses == NULL) { - goto end; - } - if (!PyList_Check(subclasses)) { - PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list"); - goto end; - } - for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) { - PyObject *scls = PyList_GetItemRef(subclasses, pos); - if (scls == NULL) { - goto end; - } - int r = PyObject_IsSubclass(subclass, scls); - Py_DECREF(scls); - if (r > 0) { - if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { - goto end; - } - result = Py_True; - goto end; - } - if (r < 0) { - goto end; - } - } - } - /* No dice; update negative cache. */ if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) { goto end; From 041f1096af50a64d32fa0d0815284daa39ddd0d2 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 31 Mar 2025 14:25:21 +0300 Subject: [PATCH 03/30] gh-92810: Apply fixes Signed-off-by: Martynov Maxim --- Lib/_py_abc.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index ceff0467df2876..5e51475beb669d 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -1,7 +1,5 @@ from _weakrefset import WeakSet -_UNSET = object() - def get_cache_token(): """Returns the current ABC cache token. From 9bc43855c1b64390b3d3de07cd2ce0a2179f2f5c Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 31 Mar 2025 14:25:43 +0300 Subject: [PATCH 04/30] gh-92810: Apply fixes Signed-off-by: Martynov Maxim --- Lib/_py_abc.py | 1 - Lib/test/test_abstract_numbers.py | 30 +++++++++++++++--------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 5e51475beb669d..00d217ba09c9f6 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -49,7 +49,6 @@ def __new__(mcls, name, bases, namespace, /, **kwargs): cls._abc_cache = WeakSet() cls._abc_negative_cache = WeakSet() cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter - cls._prevent_recursion = 0 return cls def register(cls, subclass): diff --git a/Lib/test/test_abstract_numbers.py b/Lib/test/test_abstract_numbers.py index cf071d2c933dd2..72232b670cdb89 100644 --- a/Lib/test/test_abstract_numbers.py +++ b/Lib/test/test_abstract_numbers.py @@ -24,11 +24,11 @@ def not_implemented(*args, **kwargs): class TestNumbers(unittest.TestCase): def test_int(self): - self.assertIsSubclass(int, Integral) - self.assertIsSubclass(int, Rational) - self.assertIsSubclass(int, Real) - self.assertIsSubclass(int, Complex) - self.assertIsSubclass(int, Number) + self.assertTrue(issubclass(int, Integral)) + self.assertTrue(issubclass(int, Rational)) + self.assertTrue(issubclass(int, Real)) + self.assertTrue(issubclass(int, Complex)) + self.assertTrue(issubclass(int, Number)) self.assertEqual(7, int(7).real) self.assertEqual(0, int(7).imag) @@ -38,11 +38,11 @@ def test_int(self): self.assertEqual(1, int(7).denominator) def test_float(self): - self.assertNotIsSubclass(float, Integral) - self.assertNotIsSubclass(float, Rational) - self.assertIsSubclass(float, Real) - self.assertIsSubclass(float, Complex) - self.assertIsSubclass(float, Number) + self.assertFalse(issubclass(float, Integral)) + self.assertFalse(issubclass(float, Rational)) + self.assertTrue(issubclass(float, Real)) + self.assertTrue(issubclass(float, Complex)) + self.assertTrue(issubclass(float, Number)) self.assertEqual(7.3, float(7.3).real) self.assertEqual(0, float(7.3).imag) @@ -50,11 +50,11 @@ def test_float(self): self.assertEqual(-7.3, float(-7.3).conjugate()) def test_complex(self): - self.assertNotIsSubclass(complex, Integral) - self.assertNotIsSubclass(complex, Rational) - self.assertNotIsSubclass(complex, Real) - self.assertIsSubclass(complex, Complex) - self.assertIsSubclass(complex, Number) + self.assertFalse(issubclass(complex, Integral)) + self.assertFalse(issubclass(complex, Rational)) + self.assertFalse(issubclass(complex, Real)) + self.assertTrue(issubclass(complex, Complex)) + self.assertTrue(issubclass(complex, Number)) c1, c2 = complex(3, 2), complex(4,1) # XXX: This is not ideal, but see the comment in math_trunc(). From 3d80b1e46d4eb1480abbc0bd363cd22cfe393b0c Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 31 Mar 2025 14:25:43 +0300 Subject: [PATCH 05/30] gh-92810: Apply fixes Signed-off-by: Martynov Maxim --- Modules/_abc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 28ebed195ab656..55e620fb3ffbbd 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -760,7 +760,6 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, PyErr_SetString(PyExc_TypeError, "issubclass() arg 1 must be a class"); return NULL; } - PyTypeObject *cls = (PyTypeObject *)self; PyObject *ok, *subclasses = NULL, *result = NULL; _abcmodule_state *state = NULL; From b7603e0c120cfd681a50f5822564722d6e620d05 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 21 Apr 2025 13:57:23 +0300 Subject: [PATCH 06/30] gh-92810: Return __subclasses__clause back --- Lib/_py_abc.py | 40 +++++--- Lib/test/test_abc.py | 197 ++++++++++++++++++++++++++++++------ Lib/test/test_isinstance.py | 4 +- Modules/_abc.c | 31 +++++- 4 files changed, 222 insertions(+), 50 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 00d217ba09c9f6..60300a4f11375b 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -1,4 +1,5 @@ from _weakrefset import WeakSet +from weakref import WeakKeyDictionary def get_cache_token(): @@ -31,6 +32,7 @@ class ABCMeta(type): # Note: this counter is private. Use `abc.get_cache_token()` for # external code. _abc_invalidation_counter = 0 + _abc_issubclass_context = WeakKeyDictionary() def __new__(mcls, name, bases, namespace, /, **kwargs): cls = super().__new__(mcls, name, bases, namespace, **kwargs) @@ -65,21 +67,7 @@ def register(cls, subclass): if issubclass(cls, subclass): # This would create a cycle, which is bad for the algorithm below raise RuntimeError("Refusing to create an inheritance cycle") - - # Actual registration cls._abc_registry.add(subclass) - - # Recursively register the subclass in all ABC bases, to avoid recursive lookups. - # >>> class Ancestor1(ABC): pass - # >>> class Ancestor2(Ancestor1): pass - # >>> class Other: pass - # >>> Ancestor2.register(Other) # same result for Ancestor1.register(Other) - # >>> issubclass(Other, Ancestor2) is True - # >>> issubclass(Other, Ancestor1) is True - for pcls in cls.__mro__: - if hasattr(pcls, "_abc_registry"): - pcls._abc_registry.add(subclass) - # Invalidate negative cache ABCMeta._abc_invalidation_counter += 1 return subclass @@ -152,6 +140,26 @@ def __subclasscheck__(cls, subclass): if issubclass(subclass, rcls): cls._abc_cache.add(subclass) return True - # No dice; update negative cache - cls._abc_negative_cache.add(subclass) + + # Check if it's a subclass of a subclass (recursive) + for scls in cls.__subclasses__(): + # If inside recursive issubclass check, avoid adding classes to any cache because this + # may drastically increase memory usage. + # Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, + # so using global context within ABCMeta. + # This is done only on first method call, others will use cached result. + scls_context = ABCMeta._abc_issubclass_context.setdefault(scls, WeakSet()) + try: + scls_context.add(cls) + result = issubclass(subclass, scls) + finally: + scls_context.remove(cls) + + if result: + if not ABCMeta._abc_issubclass_context.get(cls, None): + cls._abc_cache.add(subclass) + return True + + if not ABCMeta._abc_issubclass_context.get(cls, None): + cls._abc_negative_cache.add(subclass) return False diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 50d8da6f132ec3..94b99ce0494608 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -270,29 +270,100 @@ def x(self): class C(metaclass=meta): pass + def test_isinstance_direct_inheritance(self): + class A(metaclass=abc_ABCMeta): + pass + class B(A): + pass + class C(A): + pass + a = A() + b = B() + c = C() + # trigger caching + for _ in range(2): + self.assertIsInstance(a, A) + self.assertIsInstance(a, (A,)) + self.assertNotIsInstance(a, B) + self.assertNotIsInstance(a, (B,)) + self.assertNotIsInstance(a, C) + self.assertNotIsInstance(a, (C,)) + + self.assertIsInstance(b, B) + self.assertIsInstance(b, (B,)) + self.assertIsInstance(b, A) + self.assertIsInstance(b, (A,)) + self.assertNotIsInstance(b, C) + self.assertNotIsInstance(b, (C,)) + + self.assertIsInstance(c, C) + self.assertIsInstance(c, (C,)) + self.assertIsInstance(c, A) + self.assertIsInstance(c, (A,)) + self.assertNotIsInstance(c, B) + self.assertNotIsInstance(c, (B,)) + + self.assertIsSubclass(B, A) + self.assertIsSubclass(B, (A,)) + self.assertIsSubclass(C, A) + self.assertIsSubclass(C, (A,)) + self.assertNotIsSubclass(B, C) + self.assertNotIsSubclass(B, (C,)) + self.assertNotIsSubclass(C, B) + self.assertNotIsSubclass(C, (B,)) + self.assertNotIsSubclass(A, B) + self.assertNotIsSubclass(A, (B,)) + self.assertNotIsSubclass(A, C) + self.assertNotIsSubclass(A, (C,)) + def test_registration_basics(self): class A(metaclass=abc_ABCMeta): pass class B(object): pass + a = A() b = B() - self.assertNotIsSubclass(B, A) - self.assertNotIsSubclass(B, (A,)) - self.assertNotIsInstance(b, A) - self.assertNotIsInstance(b, (A,)) + + # trigger caching + for _ in range(2): + self.assertNotIsSubclass(B, A) + self.assertNotIsSubclass(B, (A,)) + self.assertNotIsInstance(b, A) + self.assertNotIsInstance(b, (A,)) + + self.assertNotIsSubclass(A, B) + self.assertNotIsSubclass(A, (B,)) + self.assertNotIsInstance(a, B) + self.assertNotIsInstance(a, (B,)) + B1 = A.register(B) - self.assertIsSubclass(B, A) - self.assertIsSubclass(B, (A,)) - self.assertIsInstance(b, A) - self.assertIsInstance(b, (A,)) - self.assertIs(B1, B) + # trigger caching + for _ in range(2): + self.assertIsSubclass(B, A) + self.assertIsSubclass(B, (A,)) + self.assertIsInstance(b, A) + self.assertIsInstance(b, (A,)) + self.assertIs(B1, B) + + self.assertNotIsSubclass(A, B) + self.assertNotIsSubclass(A, (B,)) + self.assertNotIsInstance(a, B) + self.assertNotIsInstance(a, (B,)) + class C(B): pass c = C() - self.assertIsSubclass(C, A) - self.assertIsSubclass(C, (A,)) - self.assertIsInstance(c, A) - self.assertIsInstance(c, (A,)) + # trigger caching + for _ in range(2): + self.assertIsSubclass(C, A) + self.assertIsSubclass(C, (A,)) + self.assertIsInstance(c, A) + self.assertIsInstance(c, (A,)) + + self.assertNotIsSubclass(A, C) + self.assertNotIsSubclass(A, (C,)) + self.assertNotIsInstance(a, C) + self.assertNotIsInstance(a, (C,)) def test_register_as_class_deco(self): class A(metaclass=abc_ABCMeta): @@ -377,41 +448,75 @@ class A(metaclass=abc_ABCMeta): pass self.assertIsSubclass(A, A) self.assertIsSubclass(A, (A,)) + class B(metaclass=abc_ABCMeta): pass self.assertNotIsSubclass(A, B) self.assertNotIsSubclass(A, (B,)) self.assertNotIsSubclass(B, A) self.assertNotIsSubclass(B, (A,)) + class C(metaclass=abc_ABCMeta): pass A.register(B) class B1(B): pass - self.assertIsSubclass(B1, A) - self.assertIsSubclass(B1, (A,)) + # trigger caching + for _ in range(2): + self.assertIsSubclass(B1, A) + self.assertIsSubclass(B1, (A,)) + class C1(C): pass B1.register(C1) - self.assertNotIsSubclass(C, B) - self.assertNotIsSubclass(C, (B,)) - self.assertNotIsSubclass(C, B1) - self.assertNotIsSubclass(C, (B1,)) - self.assertIsSubclass(C1, A) - self.assertIsSubclass(C1, (A,)) - self.assertIsSubclass(C1, B) - self.assertIsSubclass(C1, (B,)) - self.assertIsSubclass(C1, B1) - self.assertIsSubclass(C1, (B1,)) + # trigger caching + for _ in range(2): + self.assertNotIsSubclass(C, B) + self.assertNotIsSubclass(C, (B,)) + self.assertNotIsSubclass(C, B1) + self.assertNotIsSubclass(C, (B1,)) + self.assertIsSubclass(C1, A) + self.assertIsSubclass(C1, (A,)) + self.assertIsSubclass(C1, B) + self.assertIsSubclass(C1, (B,)) + self.assertIsSubclass(C1, B1) + self.assertIsSubclass(C1, (B1,)) + C1.register(int) class MyInt(int): pass - self.assertIsSubclass(MyInt, A) - self.assertIsSubclass(MyInt, (A,)) - self.assertIsInstance(42, A) - self.assertIsInstance(42, (A,)) + # trigger caching + for _ in range(2): + self.assertIsSubclass(MyInt, A) + self.assertIsSubclass(MyInt, (A,)) + self.assertIsInstance(42, A) + self.assertIsInstance(42, (A,)) - def test_subclasses_bad_arguments(self): + def test_custom_subclasses(self): + class A: pass + class B: pass + + class Parent1(metaclass=abc_ABCMeta): + @classmethod + def __subclasses__(cls): + return [A] + + class Parent2(metaclass=abc_ABCMeta): + __subclasses__ = lambda: [A] + + # trigger caching + for _ in range(2): + self.assertIsInstance(A(), Parent1) + self.assertIsSubclass(A, Parent1) + self.assertNotIsInstance(B(), Parent1) + self.assertNotIsSubclass(B, Parent1) + + self.assertIsInstance(A(), Parent2) + self.assertIsSubclass(A, Parent2) + self.assertNotIsInstance(B(), Parent2) + self.assertNotIsSubclass(B, Parent2) + + def test_issubclass_bad_arguments(self): class A(metaclass=abc_ABCMeta): pass @@ -429,6 +534,37 @@ class C: with self.assertRaises(TypeError): issubclass(C(), A) + # bpo-34441: Check that issubclass() doesn't crash on bogus + # classes. + bogus_subclasses = [ + None, + lambda x: [], + lambda: 42, + lambda: [42], + ] + + for i, func in enumerate(bogus_subclasses): + class S(metaclass=abc_ABCMeta): + __subclasses__ = func + + with self.subTest(i=i): + with self.assertRaises(TypeError): + issubclass(int, S) + + # Also check that issubclass() propagates exceptions raised by + # __subclasses__. + class CustomError(Exception): ... + exc_msg = "exception from __subclasses__" + + def raise_exc(): + raise CustomError(exc_msg) + + class S(metaclass=abc_ABCMeta): + __subclasses__ = raise_exc + + with self.assertRaisesRegex(CustomError, exc_msg): + issubclass(int, S) + def test_subclasshook(self): class A(metaclass=abc.ABCMeta): @classmethod @@ -491,7 +627,6 @@ def foo(self): self.assertEqual(A.__abstractmethods__, set()) A() - def test_update_new_abstractmethods(self): class A(metaclass=abc_ABCMeta): @abc.abstractmethod diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index 5aac482aa72007..153b7cd49fe4b7 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -351,13 +351,13 @@ class B: with support.infinite_recursion(25): self.assertRaises(RecursionError, issubclass, X(), int) - def test_override_subclasses(self): + def test_custom_subclasses_are_ignored(self): class A: pass class B: pass class Parent1: @classmethod - def __subclasses__(self): + def __subclasses__(cls): return [A, B] class Parent2: diff --git a/Modules/_abc.c b/Modules/_abc.c index 55e620fb3ffbbd..bb8aa5241ba05a 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -763,6 +763,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, PyObject *ok, *subclasses = NULL, *result = NULL; _abcmodule_state *state = NULL; + Py_ssize_t pos; int incache; _abc_data *impl = _get_impl(module, self); if (impl == NULL) { @@ -849,7 +850,35 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } - /* No dice; update negative cache. */ + /* 6. Check if it's a subclass of a subclass (recursive). */ + subclasses = PyObject_CallMethod(self, "__subclasses__", NULL); + if (subclasses == NULL) { + goto end; + } + if (!PyList_Check(subclasses)) { + PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list"); + goto end; + } + for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) { + PyObject *scls = PyList_GetItemRef(subclasses, pos); + if (scls == NULL) { + goto end; + } + int r = PyObject_IsSubclass(subclass, scls); + Py_DECREF(scls); + if (r > 0) { + if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { + goto end; + } + result = Py_True; + goto end; + } + if (r < 0) { + goto end; + } + } + + /* Recursive calls lead to uncontrolled negative cache growth, avoid this */ if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) { goto end; } From dd0d18ccf6a4edd67418d0ef918fa791a57d29cb Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 21 Apr 2025 14:05:19 +0300 Subject: [PATCH 07/30] gh-92810: Revert _abc.c changes --- Modules/_abc.c | 50 +------------------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index bb8aa5241ba05a..f5eb35e525213d 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -590,50 +590,6 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) } Py_DECREF(impl); - /* Recursively register the subclass in all ABC bases, to avoid recursive lookups. - >>> class Ancestor1(ABC): pass - >>> class Ancestor2(Ancestor1): pass - >>> class Other: pass - >>> Ancestor2.register(Other) # same result for Ancestor1.register(Other) - >>> issubclass(Other, Ancestor2) is True - >>> issubclass(Other, Ancestor1) is True - */ - PyObject *mro = PyObject_GetAttrString(self, "__mro__"); - if (mro == NULL) { - return NULL; - } - - if (!PyTuple_CheckExact(mro)) { - PyErr_SetString(PyExc_TypeError, "__mro__ must be an exact tuple"); - goto error; - } - - for (Py_ssize_t pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) { - PyObject *base_class = PyTuple_GET_ITEM(mro, pos); // borrowed - PyObject *base_class_data; - - if (PyObject_GetOptionalAttr(base_class, - &_Py_ID(_abc_impl), - &base_class_data) < 0) - { - goto error; - } - - if (base_class_data == NULL) { - // not ABC class - continue; - } - - _abc_data *base_class_state = _abc_data_CAST(base_class_data); - int res = _add_to_weak_set(base_class_state, - &base_class_state->_abc_registry, - subclass); - Py_DECREF(base_class_data); - if (res < 0) { - goto error; - } - } - /* Invalidate negative cache */ increment_invalidation_counter(get_abc_state(module)); @@ -648,10 +604,6 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) } } return Py_NewRef(subclass); - -error: - Py_XDECREF(mro); - return NULL; } @@ -878,7 +830,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, } } - /* Recursive calls lead to uncontrolled negative cache growth, avoid this */ + /* No dice; update negative cache. */ if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) { goto end; } From 8d695fd1cac081f66278548fcfa535e5dd051fbc Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 21 Apr 2025 14:18:03 +0300 Subject: [PATCH 08/30] gh-92810: Fix linter errors --- Lib/_py_abc.py | 2 +- Lib/test/test_abc.py | 2 +- Modules/_abc.c | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 60300a4f11375b..a03a77b3bcb391 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -144,7 +144,7 @@ def __subclasscheck__(cls, subclass): # Check if it's a subclass of a subclass (recursive) for scls in cls.__subclasses__(): # If inside recursive issubclass check, avoid adding classes to any cache because this - # may drastically increase memory usage. + # may drastically increase memory usage. # Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, # so using global context within ABCMeta. # This is done only on first method call, others will use cached result. diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 94b99ce0494608..fd1a895814b1a4 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -503,7 +503,7 @@ def __subclasses__(cls): class Parent2(metaclass=abc_ABCMeta): __subclasses__ = lambda: [A] - + # trigger caching for _ in range(2): self.assertIsInstance(A(), Parent1) diff --git a/Modules/_abc.c b/Modules/_abc.c index f5eb35e525213d..d6a953b336025d 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -578,8 +578,6 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) if (result < 0) { return NULL; } - - /* Actual registration */ _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; From a2650b6edc4b29f080ce08d454490c59b8d21670 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Fri, 13 Jun 2025 15:43:54 +0300 Subject: [PATCH 09/30] gh-92810: Add recursive issubclass check to _abc.c --- Lib/_py_abc.py | 15 ++++++---- Modules/_abc.c | 79 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index a03a77b3bcb391..584f85f6d73314 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -32,7 +32,6 @@ class ABCMeta(type): # Note: this counter is private. Use `abc.get_cache_token()` for # external code. _abc_invalidation_counter = 0 - _abc_issubclass_context = WeakKeyDictionary() def __new__(mcls, name, bases, namespace, /, **kwargs): cls = super().__new__(mcls, name, bases, namespace, **kwargs) @@ -51,6 +50,7 @@ def __new__(mcls, name, bases, namespace, /, **kwargs): cls._abc_cache = WeakSet() cls._abc_negative_cache = WeakSet() cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter + cls._abc_issubclasscheck_recursive = False return cls def register(cls, subclass): @@ -148,18 +148,21 @@ def __subclasscheck__(cls, subclass): # Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, # so using global context within ABCMeta. # This is done only on first method call, others will use cached result. - scls_context = ABCMeta._abc_issubclass_context.setdefault(scls, WeakSet()) + scls_is_abc = hasattr(scls, "_abc_issubclasscheck_recursive") + if scls_is_abc: + scls._abc_issubclasscheck_recursive = True + try: - scls_context.add(cls) result = issubclass(subclass, scls) finally: - scls_context.remove(cls) + if scls_is_abc: + scls._abc_issubclasscheck_recursive = False if result: - if not ABCMeta._abc_issubclass_context.get(cls, None): + if not cls._abc_issubclasscheck_recursive: cls._abc_cache.add(subclass) return True - if not ABCMeta._abc_issubclass_context.get(cls, None): + if not cls._abc_issubclasscheck_recursive: cls._abc_negative_cache.add(subclass) return False diff --git a/Modules/_abc.c b/Modules/_abc.c index d6a953b336025d..9640680cef8409 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -65,6 +65,7 @@ typedef struct { PyObject *_abc_cache; PyObject *_abc_negative_cache; uint64_t _abc_negative_cache_version; + bool _abc_issubclasscheck_recursive; } _abc_data; #define _abc_data_CAST(op) ((_abc_data *)(op)) @@ -89,6 +90,24 @@ set_cache_version(_abc_data *impl, uint64_t version) #endif } +static inline bool +is_issubclasscheck_recursive(_abc_data *impl) +{ + return impl->_abc_issubclasscheck_recursive; +} + +static inline void +set_issubclasscheck_recursive(_abc_data *impl) +{ + impl->_abc_issubclasscheck_recursive = true; +} + +static inline void +unset_issubclasscheck_recursive(_abc_data *impl) +{ + impl->_abc_issubclasscheck_recursive = false; +} + static int abc_data_traverse(PyObject *op, visitproc visit, void *arg) { @@ -139,6 +158,7 @@ abc_data_new(PyTypeObject *type, PyObject *args, PyObject *kwds) self->_abc_cache = NULL; self->_abc_negative_cache = NULL; self->_abc_negative_cache_version = get_invalidation_counter(state); + self->_abc_issubclasscheck_recursive = false; return (PyObject *) self; } @@ -177,6 +197,23 @@ _get_impl(PyObject *module, PyObject *self) return (_abc_data *)impl; } +static _abc_data * +_get_impl_optional(PyObject *module, PyObject *self) +{ + _abcmodule_state *state = get_abc_state(module); + PyObject *impl = NULL; + int res = PyObject_GetOptionalAttr(self, &_Py_ID(_abc_impl), &impl); + if (res <= 0) { + return NULL; + } + if (!Py_IS_TYPE(impl, state->_abc_data_type)) { + PyErr_SetString(PyExc_TypeError, "_abc_impl is set to a wrong type"); + Py_DECREF(impl); + return NULL; + } + return (_abc_data *)impl; +} + static int _in_weak_set(_abc_data *impl, PyObject **pset, PyObject *obj) { @@ -347,11 +384,12 @@ _abc__get_dump(PyObject *module, PyObject *self) } PyObject *res; Py_BEGIN_CRITICAL_SECTION(impl); - res = Py_BuildValue("NNNK", + res = Py_BuildValue("NNNKK", PySet_New(impl->_abc_registry), PySet_New(impl->_abc_cache), PySet_New(impl->_abc_negative_cache), - get_cache_version(impl)); + get_cache_version(impl), + is_issubclasscheck_recursive(impl)); Py_END_CRITICAL_SECTION(); Py_DECREF(impl); return res; @@ -814,23 +852,46 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, if (scls == NULL) { goto end; } + + _abc_data *scls_impl = _get_impl_optional(module, scls); + if (scls_impl != NULL) { + /* + If inside recursive issubclass check, avoid adding classes to any cache because this + may drastically increase memory usage. + Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, + so using global context within ABCMeta. + This is done only on first method call, others will use cached result. + */ + set_issubclasscheck_recursive(scls_impl); + } + int r = PyObject_IsSubclass(subclass, scls); Py_DECREF(scls); + + if (scls_impl != NULL) { + unset_issubclasscheck_recursive(scls_impl); + Py_DECREF(scls_impl); + } + + if (r < 0) { + goto end; + } if (r > 0) { - if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { - goto end; + if (!is_issubclasscheck_recursive(impl)) { + if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { + goto end; + } } result = Py_True; goto end; } - if (r < 0) { - goto end; - } } /* No dice; update negative cache. */ - if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) { - goto end; + if (!is_issubclasscheck_recursive(impl)) { + if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) { + goto end; + } } result = Py_False; From 7afa5ea58c5fb19724308dc97d9cf693f6426a8a Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Fri, 13 Jun 2025 17:14:34 +0300 Subject: [PATCH 10/30] gh-92810: Remove WeakKeyDictionary from _py_abc --- Lib/_py_abc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 584f85f6d73314..84e33139c313df 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -1,5 +1,4 @@ from _weakrefset import WeakSet -from weakref import WeakKeyDictionary def get_cache_token(): From 57980d39e42ece9ecb785fbc5549d2a561970b15 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Fri, 13 Jun 2025 17:22:56 +0300 Subject: [PATCH 11/30] gh-92810: Add news entry --- .../2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst new file mode 100644 index 00000000000000..29d1a5722d26ec --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst @@ -0,0 +1,2 @@ +Reduce memory usage by :meth:`ABCMeta.__subclasscheck__` for large class +trees From bbaf38abb6f30d3c10c3277958b532b0813c3474 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Fri, 13 Jun 2025 17:28:19 +0300 Subject: [PATCH 12/30] gh-92810: Fix news entry --- .../2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst index 29d1a5722d26ec..f78b19ce76d8db 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst @@ -1,2 +1,2 @@ -Reduce memory usage by :meth:`ABCMeta.__subclasscheck__` for large class -trees +Reduce memory usage by :meth:`~type.__subclasscheck__` +for :class:`abc.ABCMeta` and large class trees From 6fc994ddf61416b62f66595328a6333f5dac6bef Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Sun, 22 Jun 2025 19:13:48 +0300 Subject: [PATCH 13/30] gh-92810: Fixes after review --- Lib/_py_abc.py | 18 +++--- Lib/test/test_abc.py | 4 +- ...5-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst | 2 +- Modules/_abc.c | 56 ++++++++++++------- 4 files changed, 48 insertions(+), 32 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 84e33139c313df..0610f2c5bde5ec 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -67,8 +67,7 @@ def register(cls, subclass): # This would create a cycle, which is bad for the algorithm below raise RuntimeError("Refusing to create an inheritance cycle") cls._abc_registry.add(subclass) - # Invalidate negative cache - ABCMeta._abc_invalidation_counter += 1 + ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache return subclass def _dump_registry(cls, file=None): @@ -139,29 +138,26 @@ def __subclasscheck__(cls, subclass): if issubclass(subclass, rcls): cls._abc_cache.add(subclass) return True - # Check if it's a subclass of a subclass (recursive) for scls in cls.__subclasses__(): - # If inside recursive issubclass check, avoid adding classes to any cache because this - # may drastically increase memory usage. - # Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, - # so using global context within ABCMeta. - # This is done only on first method call, others will use cached result. + # If inside recursive issubclass check, avoid adding classes + # to any cache because this may drastically increase memory usage. + # Unfortunately, issubclass/__subclasscheck__ don't accept third + # argument with context, so using global context within ABCMeta. + # This is done only on first method call, next will use cache. scls_is_abc = hasattr(scls, "_abc_issubclasscheck_recursive") if scls_is_abc: scls._abc_issubclasscheck_recursive = True - try: result = issubclass(subclass, scls) finally: if scls_is_abc: scls._abc_issubclasscheck_recursive = False - if result: if not cls._abc_issubclasscheck_recursive: cls._abc_cache.add(subclass) return True - + # No dice; update negative cache if not cls._abc_issubclasscheck_recursive: cls._abc_negative_cache.add(subclass) return False diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index fd1a895814b1a4..700608deb1c40a 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -277,6 +277,7 @@ class B(A): pass class C(A): pass + a = A() b = B() c = C() @@ -321,9 +322,9 @@ class A(metaclass=abc_ABCMeta): pass class B(object): pass + a = A() b = B() - # trigger caching for _ in range(2): self.assertNotIsSubclass(B, A) @@ -352,6 +353,7 @@ class B(object): class C(B): pass + c = C() # trigger caching for _ in range(2): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst index f78b19ce76d8db..13cd06eb821bf4 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst @@ -1,2 +1,2 @@ Reduce memory usage by :meth:`~type.__subclasscheck__` -for :class:`abc.ABCMeta` and large class trees +for :class:`abc.ABCMeta` and large class trees. diff --git a/Modules/_abc.c b/Modules/_abc.c index 9640680cef8409..9f27e17a5617ce 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -65,7 +65,7 @@ typedef struct { PyObject *_abc_cache; PyObject *_abc_negative_cache; uint64_t _abc_negative_cache_version; - bool _abc_issubclasscheck_recursive; + uint8_t _abc_issubclasscheck_recursive; } _abc_data; #define _abc_data_CAST(op) ((_abc_data *)(op)) @@ -90,22 +90,34 @@ set_cache_version(_abc_data *impl, uint64_t version) #endif } -static inline bool +static inline uint8_t is_issubclasscheck_recursive(_abc_data *impl) { +#ifdef Py_GIL_DISABLED + return _Py_atomic_load_uint64(&impl->_abc_issubclasscheck_recursive); +#else return impl->_abc_issubclasscheck_recursive; +#endif } static inline void set_issubclasscheck_recursive(_abc_data *impl) { - impl->_abc_issubclasscheck_recursive = true; +#ifdef Py_GIL_DISABLED + _Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 1); +#else + impl->_abc_issubclasscheck_recursive = 1; +#endif } static inline void unset_issubclasscheck_recursive(_abc_data *impl) { - impl->_abc_issubclasscheck_recursive = false; +#ifdef Py_GIL_DISABLED + _Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 0); +#else + impl->_abc_issubclasscheck_recursive = 0; +#endif } static int @@ -158,7 +170,7 @@ abc_data_new(PyTypeObject *type, PyObject *args, PyObject *kwds) self->_abc_cache = NULL; self->_abc_negative_cache = NULL; self->_abc_negative_cache_version = get_invalidation_counter(state); - self->_abc_issubclasscheck_recursive = false; + self->_abc_issubclasscheck_recursive = 0; return (PyObject *) self; } @@ -197,21 +209,24 @@ _get_impl(PyObject *module, PyObject *self) return (_abc_data *)impl; } -static _abc_data * -_get_impl_optional(PyObject *module, PyObject *self) +static int +_get_impl_optional(PyObject *module, PyObject *self, _abc_data **data) { _abcmodule_state *state = get_abc_state(module); PyObject *impl = NULL; int res = PyObject_GetOptionalAttr(self, &_Py_ID(_abc_impl), &impl); if (res <= 0) { - return NULL; + *data = NULL; + return res; } if (!Py_IS_TYPE(impl, state->_abc_data_type)) { PyErr_SetString(PyExc_TypeError, "_abc_impl is set to a wrong type"); Py_DECREF(impl); - return NULL; + *data = NULL; + return -1; } - return (_abc_data *)impl; + *data = (_abc_data *)impl; + return 1; } static int @@ -853,22 +868,25 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } - _abc_data *scls_impl = _get_impl_optional(module, scls); - if (scls_impl != NULL) { + _abc_data *scls_impl; + int scls_is_abc = _get_impl_optional(module, scls, &scls_impl); + if (scls_is_abc < 0) { + goto end; + } + if (scls_is_abc > 0) { /* - If inside recursive issubclass check, avoid adding classes to any cache because this - may drastically increase memory usage. - Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context, - so using global context within ABCMeta. - This is done only on first method call, others will use cached result. + If inside recursive issubclass check, avoid adding classes + to any cache because this may drastically increase memory usage. + Unfortunately, issubclass/__subclasscheck__ don't accept third + argument with context, so using global context within ABCMeta. + This is done only on first method call, next will use cache. */ set_issubclasscheck_recursive(scls_impl); } int r = PyObject_IsSubclass(subclass, scls); Py_DECREF(scls); - - if (scls_impl != NULL) { + if (scls_is_abc > 0) { unset_issubclasscheck_recursive(scls_impl); Py_DECREF(scls_impl); } From b3b5895b7bfd0d8357ac326982d78ba0d4ea2edf Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Sun, 22 Jun 2025 19:16:28 +0300 Subject: [PATCH 14/30] gh-92810: Fixes after review --- Lib/test/test_abc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 700608deb1c40a..1c045c9ab91717 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -322,7 +322,7 @@ class A(metaclass=abc_ABCMeta): pass class B(object): pass - + a = A() b = B() # trigger caching @@ -353,7 +353,7 @@ class B(object): class C(B): pass - + c = C() # trigger caching for _ in range(2): From 69c503864f12a167cc4a56760f22b66083c61f9c Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 23 Jun 2025 18:53:56 +0300 Subject: [PATCH 15/30] gh-92810: Fixes after review --- Modules/_abc.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 9f27e17a5617ce..948e7ee7d15de4 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -93,31 +93,19 @@ set_cache_version(_abc_data *impl, uint64_t version) static inline uint8_t is_issubclasscheck_recursive(_abc_data *impl) { -#ifdef Py_GIL_DISABLED - return _Py_atomic_load_uint64(&impl->_abc_issubclasscheck_recursive); -#else - return impl->_abc_issubclasscheck_recursive; -#endif + return FT_ATOMIC_LOAD_UINT8_RELAXED(impl->_abc_issubclasscheck_recursive); } static inline void set_issubclasscheck_recursive(_abc_data *impl) { -#ifdef Py_GIL_DISABLED - _Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 1); -#else - impl->_abc_issubclasscheck_recursive = 1; -#endif + FT_ATOMIC_STORE_UINT8_RELAXED(impl->_abc_issubclasscheck_recursive, 1); } static inline void unset_issubclasscheck_recursive(_abc_data *impl) { -#ifdef Py_GIL_DISABLED - _Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 0); -#else - impl->_abc_issubclasscheck_recursive = 0; -#endif + FT_ATOMIC_STORE_UINT8_RELAXED(impl->_abc_issubclasscheck_recursive, 0); } static int From dc1b6d56a24adf2b5a5cd687b935b986a4ad8462 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 23 Jun 2025 20:17:38 +0300 Subject: [PATCH 16/30] gh-92810: Fixes after review --- Modules/_abc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 948e7ee7d15de4..150844aec4b0b2 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -74,7 +74,7 @@ static inline uint64_t get_cache_version(_abc_data *impl) { #ifdef Py_GIL_DISABLED - return _Py_atomic_load_uint64(&impl->_abc_negative_cache_version); + return _Py_atomic_load_uint64_relaxed(&impl->_abc_negative_cache_version); #else return impl->_abc_negative_cache_version; #endif @@ -84,7 +84,7 @@ static inline void set_cache_version(_abc_data *impl, uint64_t version) { #ifdef Py_GIL_DISABLED - _Py_atomic_store_uint64(&impl->_abc_negative_cache_version, version); + _Py_atomic_store_uint64_relaxed(&impl->_abc_negative_cache_version, version); #else impl->_abc_negative_cache_version = version; #endif From cd097aba31ad6cc3429ac15fcaa74880e0aee097 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 23 Jun 2025 20:46:39 +0300 Subject: [PATCH 17/30] gh-92810: Introduce FT wrappers for uint64_t atomics --- Include/internal/pycore_pyatomic_ft_wrappers.h | 6 ++++++ Modules/_abc.c | 12 ++---------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 3e41e2fd1569ca..bf35af513cdb85 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -45,6 +45,8 @@ extern "C" { _Py_atomic_load_uint16_relaxed(&value) #define FT_ATOMIC_LOAD_UINT32_RELAXED(value) \ _Py_atomic_load_uint32_relaxed(&value) +#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) \ + _Py_atomic_load_uint64_relaxed(&value) #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) \ _Py_atomic_load_ulong_relaxed(&value) #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \ @@ -61,6 +63,8 @@ extern "C" { _Py_atomic_store_uint16_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \ _Py_atomic_store_uint32_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_UINT64_RELAXED(value, new_value) \ + _Py_atomic_store_uint64_relaxed(&value, new_value) #define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) \ _Py_atomic_store_char_relaxed(&value, new_value) #define FT_ATOMIC_LOAD_CHAR_RELAXED(value) \ @@ -126,6 +130,7 @@ extern "C" { #define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value #define FT_ATOMIC_LOAD_UINT16_RELAXED(value) value #define FT_ATOMIC_LOAD_UINT32_RELAXED(value) value +#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) value #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) value #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value @@ -134,6 +139,7 @@ extern "C" { #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_STORE_UINT64_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_LOAD_CHAR_RELAXED(value) value #define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) value diff --git a/Modules/_abc.c b/Modules/_abc.c index 150844aec4b0b2..f9303f57cc1273 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -73,21 +73,13 @@ typedef struct { static inline uint64_t get_cache_version(_abc_data *impl) { -#ifdef Py_GIL_DISABLED - return _Py_atomic_load_uint64_relaxed(&impl->_abc_negative_cache_version); -#else - return impl->_abc_negative_cache_version; -#endif + return FT_ATOMIC_LOAD_UINT64_RELAXED(impl->_abc_negative_cache_version); } static inline void set_cache_version(_abc_data *impl, uint64_t version) { -#ifdef Py_GIL_DISABLED - _Py_atomic_store_uint64_relaxed(&impl->_abc_negative_cache_version, version); -#else - impl->_abc_negative_cache_version = version; -#endif + FT_ATOMIC_STORE_UINT64_RELAXED(impl->_abc_negative_cache_version, version); } static inline uint8_t From f3a21a7dd27f6d713af096991fced30dbacc5cb0 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 23 Jun 2025 21:03:11 +0300 Subject: [PATCH 18/30] gh-92810: Use FT atomic wrappers for ABC invalidation counter --- Include/internal/pycore_pyatomic_ft_wrappers.h | 2 ++ Modules/_abc.c | 12 ++---------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index bf35af513cdb85..990134f77c33b3 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -115,6 +115,8 @@ extern "C" { _Py_atomic_load_ullong_relaxed(&value) #define FT_ATOMIC_ADD_SSIZE(value, new_value) \ (void)_Py_atomic_add_ssize(&value, new_value) +#define FT_ATOMIC_ADD_UINT64(value, new_value) \ + (void)_Py_atomic_add_uint64(&value, new_value) #else #define FT_ATOMIC_LOAD_PTR(value) value diff --git a/Modules/_abc.c b/Modules/_abc.c index f9303f57cc1273..d529f39606d900 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -35,21 +35,13 @@ get_abc_state(PyObject *module) static inline uint64_t get_invalidation_counter(_abcmodule_state *state) { -#ifdef Py_GIL_DISABLED - return _Py_atomic_load_uint64(&state->abc_invalidation_counter); -#else - return state->abc_invalidation_counter; -#endif + return FT_ATOMIC_LOAD_UINT64_RELAXED(state->abc_invalidation_counter); } static inline void increment_invalidation_counter(_abcmodule_state *state) { -#ifdef Py_GIL_DISABLED - _Py_atomic_add_uint64(&state->abc_invalidation_counter, 1); -#else - state->abc_invalidation_counter++; -#endif + FT_ATOMIC_ADD_UINT64(state->abc_invalidation_counter, 1); } /* This object stores internal state for ABCs. From e24e81504df9f7057902ecb45ab63010f7a33fe9 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 23 Jun 2025 21:07:21 +0300 Subject: [PATCH 19/30] gh-92810: Fix missing FT wrapper --- Include/internal/pycore_pyatomic_ft_wrappers.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 990134f77c33b3..09540b613d2dbf 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -167,6 +167,7 @@ extern "C" { #define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value #define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_ADD_SSIZE(value, new_value) (void)(value += new_value) +#define FT_ATOMIC_ADD_UINT64(value, new_value) (void)(value += new_value) #endif From b723912b938219fae770ff4526ebd16ef8e7ade8 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 11:35:57 +0300 Subject: [PATCH 20/30] gh-92810: Address review fixes --- Lib/test/test_abc.py | 125 ++++++++---------- ...5-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst | 0 Modules/_abc.c | 20 ++- 3 files changed, 71 insertions(+), 74 deletions(-) rename Misc/NEWS.d/next/{Core_and_Builtins => Library}/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst (100%) diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 1c045c9ab91717..1bc2808056dca6 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -14,7 +14,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): class TestLegacyAPI(unittest.TestCase): - def test_abstractproperty_basics(self): @abc.abstractproperty def foo(self): pass @@ -70,6 +69,25 @@ def foo(): return 4 class TestABC(unittest.TestCase): + def check_isinstance(self, obj, target_class): + self.assertIsInstance(obj, target_class) + self.assertIsInstance(obj, (target_class,)) + self.assertIsInstance(obj, target_class | target_class) + + def check_not_isinstance(self, obj, target_class): + self.assertNotIsInstance(obj, target_class) + self.assertNotIsInstance(obj, (target_class,)) + self.assertNotIsInstance(obj, target_class | target_class) + + def check_issubclass(self, klass, target_class): + self.assertIsSubclass(klass, target_class) + self.assertIsSubclass(klass, (target_class,)) + self.assertIsSubclass(klass, target_class | target_class) + + def check_not_issubclass(self, klass, target_class): + self.assertNotIsSubclass(klass, target_class) + self.assertNotIsSubclass(klass, (target_class,)) + self.assertNotIsSubclass(klass, target_class | target_class) def test_ABC_helper(self): # create an ABC using the helper class and perform basic checks @@ -283,39 +301,24 @@ class C(A): c = C() # trigger caching for _ in range(2): - self.assertIsInstance(a, A) - self.assertIsInstance(a, (A,)) - self.assertNotIsInstance(a, B) - self.assertNotIsInstance(a, (B,)) - self.assertNotIsInstance(a, C) - self.assertNotIsInstance(a, (C,)) - - self.assertIsInstance(b, B) - self.assertIsInstance(b, (B,)) - self.assertIsInstance(b, A) - self.assertIsInstance(b, (A,)) - self.assertNotIsInstance(b, C) - self.assertNotIsInstance(b, (C,)) - - self.assertIsInstance(c, C) - self.assertIsInstance(c, (C,)) - self.assertIsInstance(c, A) - self.assertIsInstance(c, (A,)) - self.assertNotIsInstance(c, B) - self.assertNotIsInstance(c, (B,)) - - self.assertIsSubclass(B, A) - self.assertIsSubclass(B, (A,)) - self.assertIsSubclass(C, A) - self.assertIsSubclass(C, (A,)) - self.assertNotIsSubclass(B, C) - self.assertNotIsSubclass(B, (C,)) - self.assertNotIsSubclass(C, B) - self.assertNotIsSubclass(C, (B,)) - self.assertNotIsSubclass(A, B) - self.assertNotIsSubclass(A, (B,)) - self.assertNotIsSubclass(A, C) - self.assertNotIsSubclass(A, (C,)) + self.check_isinstance(a, A) + self.check_not_isinstance(a, B) + self.check_not_isinstance(a, C) + + self.check_isinstance(b, B) + self.check_isinstance(b, A) + self.check_not_isinstance(b, C) + + self.check_isinstance(c, C) + self.check_isinstance(c, A) + self.check_not_isinstance(c, B) + + self.check_issubclass(B, A) + self.check_issubclass(C, A) + self.check_not_issubclass(B, C) + self.check_not_issubclass(C, B) + self.check_not_issubclass(A, B) + self.check_not_issubclass(A, C) def test_registration_basics(self): class A(metaclass=abc_ABCMeta): @@ -327,29 +330,21 @@ class B(object): b = B() # trigger caching for _ in range(2): - self.assertNotIsSubclass(B, A) - self.assertNotIsSubclass(B, (A,)) - self.assertNotIsInstance(b, A) - self.assertNotIsInstance(b, (A,)) + self.check_not_issubclass(B, A) + self.check_not_isinstance(b, A) - self.assertNotIsSubclass(A, B) - self.assertNotIsSubclass(A, (B,)) - self.assertNotIsInstance(a, B) - self.assertNotIsInstance(a, (B,)) + self.check_not_issubclass(A, B) + self.check_not_isinstance(a, B) B1 = A.register(B) # trigger caching for _ in range(2): - self.assertIsSubclass(B, A) - self.assertIsSubclass(B, (A,)) - self.assertIsInstance(b, A) - self.assertIsInstance(b, (A,)) + self.check_issubclass(B, A) + self.check_isinstance(b, A) self.assertIs(B1, B) - self.assertNotIsSubclass(A, B) - self.assertNotIsSubclass(A, (B,)) - self.assertNotIsInstance(a, B) - self.assertNotIsInstance(a, (B,)) + self.check_not_issubclass(A, B) + self.check_not_isinstance(a, B) class C(B): pass @@ -357,15 +352,11 @@ class C(B): c = C() # trigger caching for _ in range(2): - self.assertIsSubclass(C, A) - self.assertIsSubclass(C, (A,)) - self.assertIsInstance(c, A) - self.assertIsInstance(c, (A,)) + self.check_issubclass(C, A) + self.check_isinstance(c, A) - self.assertNotIsSubclass(A, C) - self.assertNotIsSubclass(A, (C,)) - self.assertNotIsInstance(a, C) - self.assertNotIsInstance(a, (C,)) + self.check_not_issubclass(A, C) + self.check_not_isinstance(a, C) def test_register_as_class_deco(self): class A(metaclass=abc_ABCMeta): @@ -508,15 +499,15 @@ class Parent2(metaclass=abc_ABCMeta): # trigger caching for _ in range(2): - self.assertIsInstance(A(), Parent1) - self.assertIsSubclass(A, Parent1) - self.assertNotIsInstance(B(), Parent1) - self.assertNotIsSubclass(B, Parent1) - - self.assertIsInstance(A(), Parent2) - self.assertIsSubclass(A, Parent2) - self.assertNotIsInstance(B(), Parent2) - self.assertNotIsSubclass(B, Parent2) + self.check_isinstance(A(), Parent1) + self.check_issubclass(A, Parent1) + self.check_not_isinstance(B(), Parent1) + self.check_not_issubclass(B, Parent1) + + self.check_isinstance(A(), Parent2) + self.check_issubclass(A, Parent2) + self.check_not_isinstance(B(), Parent2) + self.check_not_issubclass(B, Parent2) def test_issubclass_bad_arguments(self): class A(metaclass=abc_ABCMeta): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst b/Misc/NEWS.d/next/Library/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst similarity index 100% rename from Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst rename to Misc/NEWS.d/next/Library/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst diff --git a/Modules/_abc.c b/Modules/_abc.c index d529f39606d900..9bb1c780d3dbaa 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -181,10 +181,14 @@ _get_impl(PyObject *module, PyObject *self) return (_abc_data *)impl; } +/* If class is inherited from ABC, set data to point to internal ABC state of class, and return 1. + If object is not inherited from ABC, return 0. + If error is encountered, return -1. + */ static int -_get_impl_optional(PyObject *module, PyObject *self, _abc_data **data) +_get_optional_impl(_abcmodule_state *state, PyObject *self, _abc_data **data) { - _abcmodule_state *state = get_abc_state(module); + assert(data != NULL); PyObject *impl = NULL; int res = PyObject_GetOptionalAttr(self, &_Py_ID(_abc_impl), &impl); if (res <= 0) { @@ -841,7 +845,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, } _abc_data *scls_impl; - int scls_is_abc = _get_impl_optional(module, scls, &scls_impl); + int scls_is_abc = _get_optional_impl(state, scls, &scls_impl); if (scls_is_abc < 0) { goto end; } @@ -858,14 +862,16 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, int r = PyObject_IsSubclass(subclass, scls); Py_DECREF(scls); + if (r < 0) { + Py_XDECREF(scls_impl); + goto end; + } + if (scls_is_abc > 0) { unset_issubclasscheck_recursive(scls_impl); - Py_DECREF(scls_impl); } + Py_XDECREF(scls_impl); - if (r < 0) { - goto end; - } if (r > 0) { if (!is_issubclasscheck_recursive(impl)) { if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { From 16f39bde89f99ae9f4d4b5349fc3a95106496674 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 13:00:12 +0300 Subject: [PATCH 21/30] gh-92810: Address review fixes --- Modules/_abc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_abc.c b/Modules/_abc.c index 9bb1c780d3dbaa..baab01bcfa3bbd 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -847,6 +847,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, _abc_data *scls_impl; int scls_is_abc = _get_optional_impl(state, scls, &scls_impl); if (scls_is_abc < 0) { + Py_DECREF(scls); goto end; } if (scls_is_abc > 0) { From 2dc6453867a1999cab670d8a03b73ace4a0dc322 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 13:01:24 +0300 Subject: [PATCH 22/30] gh-92810: Add What's New entry --- Doc/whatsnew/3.15.rst | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index e716d7bb0f2a5c..4a5d71381d11ad 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -375,10 +375,18 @@ zlib Optimizations ============= -module_name ------------ +abc +--- + +* Reduce memory usage of `issubclass` checks for classes inheriting abstract classes. + + :class:`abc.ABCMeta` hook ``__subclasscheck__`` now includes + a guard which is triggered then the hook is called from a parent class + (``issubclass(cls, RootClass)`` -> ``issubclass(cls, NestedClass)`` -> ...). + This guard prevents adding ``cls`` to ``NestedClass`` positive and negative caches, + preventing memory bloat in some cases (thousands of classes inherited from ABC). -* TODO + (Contributed by Maxim Martynov in :gh:`92810`.) From 968766d49a35bc29998268edea493af2731c33c9 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 13:03:32 +0300 Subject: [PATCH 23/30] gh-92810: Fix What's New entry syntax --- Doc/whatsnew/3.15.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index 4a5d71381d11ad..c5b80e010bab46 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -378,7 +378,7 @@ Optimizations abc --- -* Reduce memory usage of `issubclass` checks for classes inheriting abstract classes. +* Reduce memory usage of :func:`issubclass` checks for classes inheriting abstract classes. :class:`abc.ABCMeta` hook ``__subclasscheck__`` now includes a guard which is triggered then the hook is called from a parent class From a6e44610520d94e66ab9d309372f63205bdd7a99 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 14:34:01 +0300 Subject: [PATCH 24/30] gh-92810: Address review fixes --- Lib/test/test_abc.py | 47 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 1bc2808056dca6..3cf7c7529e51ba 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -14,6 +14,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): class TestLegacyAPI(unittest.TestCase): + def test_abstractproperty_basics(self): @abc.abstractproperty def foo(self): pass @@ -558,8 +559,32 @@ class S(metaclass=abc_ABCMeta): with self.assertRaisesRegex(CustomError, exc_msg): issubclass(int, S) - def test_subclasshook(self): + def test_issubclass_bad_class(self): class A(metaclass=abc.ABCMeta): + pass + + A._abc_impl = 1 + error_msg = "_abc_impl is set to a wrong type" + with self.assertRaisesRegex(TypeError, error_msg): + issubclass(A, A) + + class B(metaclass=_py_abc.ABCMeta): + pass + + B._abc_cache = 1 + error_msg = "argument of type 'int' is not a container or iterable" + with self.assertRaisesRegex(TypeError, error_msg): + issubclass(B, B) + + class C(metaclass=_py_abc.ABCMeta): + pass + + C._abc_negative_cache = 1 + with self.assertRaisesRegex(TypeError, error_msg): + issubclass(C, C) + + def test_subclasshook(self): + class A(metaclass=abc_ABCMeta): @classmethod def __subclasshook__(cls, C): if cls is A: @@ -576,6 +601,26 @@ class C: self.assertNotIsSubclass(C, A) self.assertNotIsSubclass(C, (A,)) + def test_subclasshook_exception(self): + # Check that issubclass() propagates exceptions raised by + # __subclasshook__. + class CustomError(Exception): ... + exc_msg = "exception from __subclasshook__" + class A(metaclass=abc_ABCMeta): + @classmethod + def __subclasshook__(cls, C): + raise CustomError(exc_msg) + with self.assertRaisesRegex(CustomError, exc_msg): + issubclass(A, A) + class B(A): + pass + with self.assertRaisesRegex(CustomError, exc_msg): + issubclass(B, A) + class C: + pass + with self.assertRaisesRegex(CustomError, exc_msg): + issubclass(C, A) + def test_all_new_methods_are_called(self): class A(metaclass=abc_ABCMeta): pass From 80d328165faab5a0a5a862d269b9f93819bf36a8 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 14:35:32 +0300 Subject: [PATCH 25/30] gh-92810: Address review fixes --- Lib/_py_abc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 0610f2c5bde5ec..eeb18f8a60c618 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -67,7 +67,7 @@ def register(cls, subclass): # This would create a cycle, which is bad for the algorithm below raise RuntimeError("Refusing to create an inheritance cycle") cls._abc_registry.add(subclass) - ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache + ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache return subclass def _dump_registry(cls, file=None): From ff38b9ea33441763b536f29c62dbb42c5f19ba29 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Mon, 4 Aug 2025 14:37:06 +0300 Subject: [PATCH 26/30] gh-92810: Properly reset recursion check --- Modules/_abc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index baab01bcfa3bbd..73013ba065c198 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -863,16 +863,16 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, int r = PyObject_IsSubclass(subclass, scls); Py_DECREF(scls); - if (r < 0) { - Py_XDECREF(scls_impl); - goto end; - } if (scls_is_abc > 0) { + // reset recursion guard even if exception was raised in __subclasscheck__ unset_issubclasscheck_recursive(scls_impl); } Py_XDECREF(scls_impl); + if (r < 0) { + goto end; + } if (r > 0) { if (!is_issubclasscheck_recursive(impl)) { if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { From 858d4c0dfe5e7af5040b5d27a15f777d01b9bde1 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Tue, 4 Nov 2025 22:36:01 +0300 Subject: [PATCH 27/30] gh-92810: Fix What's New entry --- Doc/whatsnew/3.15.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index ceab13686c102b..73adceb54ffcc9 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -676,7 +676,7 @@ xml.parsers.expat to :ref:`xmlparser ` objects to tune protections against `billion laughs`_ attacks. (Contributed by Bénédikt Tran in :gh:`90949`.) - + .. _billion laughs: https://en.wikipedia.org/wiki/Billion_laughs_attack abc From 7b6a0dc4aeaf8c740651ef4c78991954810dbce8 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Tue, 4 Nov 2025 22:37:06 +0300 Subject: [PATCH 28/30] gh-92810: Improve nested subclass check performance --- Lib/_py_abc.py | 6 ++++++ Modules/_abc.c | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index eeb18f8a60c618..fb1b8dbde2b3e5 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -140,6 +140,11 @@ def __subclasscheck__(cls, subclass): return True # Check if it's a subclass of a subclass (recursive) for scls in cls.__subclasses__(): + if subclass is scls: + # Fast path + if not cls._abc_issubclasscheck_recursive: + cls._abc_cache.add(subclass) + return True # If inside recursive issubclass check, avoid adding classes # to any cache because this may drastically increase memory usage. # Unfortunately, issubclass/__subclasscheck__ don't accept third @@ -149,6 +154,7 @@ def __subclasscheck__(cls, subclass): if scls_is_abc: scls._abc_issubclasscheck_recursive = True try: + # Perform recursive check result = issubclass(subclass, scls) finally: if scls_is_abc: diff --git a/Modules/_abc.c b/Modules/_abc.c index 72ce78446f5c3b..f591ef9e0070ef 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -848,6 +848,17 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } + if (scls == subclass) { + // Fast path + if (!is_issubclasscheck_recursive(impl)) { + if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { + goto end; + } + } + result = Py_True; + goto end; + } + _abc_data *scls_impl; int scls_is_abc = _get_optional_impl(state, scls, &scls_impl); if (scls_is_abc < 0) { @@ -865,6 +876,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, set_issubclasscheck_recursive(scls_impl); } + // Perform recursive check int r = PyObject_IsSubclass(subclass, scls); Py_DECREF(scls); From 2f8f2b208bcf4f7250b65ad0b1e4cbc6e06964c3 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Tue, 4 Nov 2025 22:38:42 +0300 Subject: [PATCH 29/30] gh-92810: Automatically add ABC registry entries to cache --- Lib/_py_abc.py | 3 +++ Modules/_abc.c | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index fb1b8dbde2b3e5..000eb4232383d0 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -66,7 +66,10 @@ def register(cls, subclass): if issubclass(cls, subclass): # This would create a cycle, which is bad for the algorithm below raise RuntimeError("Refusing to create an inheritance cycle") + # Add registry entry cls._abc_registry.add(subclass) + # Automatically include cache entry + cls._abc_cache.add(subclass) ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache return subclass diff --git a/Modules/_abc.c b/Modules/_abc.c index f591ef9e0070ef..f25de46ad4b4a5 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -613,10 +613,16 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) if (impl == NULL) { return NULL; } + // Add registry entry if (_add_to_weak_set(impl, &impl->_abc_registry, subclass) < 0) { Py_DECREF(impl); return NULL; } + // Automatically include cache entry + if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { + Py_DECREF(impl); + return NULL; + } Py_DECREF(impl); /* Invalidate negative cache */ From b657cd6f2070ca32cbf26f6ae3e475728030a421 Mon Sep 17 00:00:00 2001 From: Martynov Maxim Date: Thu, 6 Nov 2025 18:07:07 +0300 Subject: [PATCH 30/30] gh-92810: Remove false fastpath --- Lib/_py_abc.py | 7 +------ Modules/_abc.c | 13 +------------ 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/Lib/_py_abc.py b/Lib/_py_abc.py index 000eb4232383d0..113adde22944bf 100644 --- a/Lib/_py_abc.py +++ b/Lib/_py_abc.py @@ -143,16 +143,11 @@ def __subclasscheck__(cls, subclass): return True # Check if it's a subclass of a subclass (recursive) for scls in cls.__subclasses__(): - if subclass is scls: - # Fast path - if not cls._abc_issubclasscheck_recursive: - cls._abc_cache.add(subclass) - return True # If inside recursive issubclass check, avoid adding classes # to any cache because this may drastically increase memory usage. # Unfortunately, issubclass/__subclasscheck__ don't accept third # argument with context, so using global context within ABCMeta. - # This is done only on first method call, next will use cache. + # This is done only on first method call, next will use cache anyway. scls_is_abc = hasattr(scls, "_abc_issubclasscheck_recursive") if scls_is_abc: scls._abc_issubclasscheck_recursive = True diff --git a/Modules/_abc.c b/Modules/_abc.c index f25de46ad4b4a5..defa219b98acf0 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -854,17 +854,6 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, goto end; } - if (scls == subclass) { - // Fast path - if (!is_issubclasscheck_recursive(impl)) { - if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) { - goto end; - } - } - result = Py_True; - goto end; - } - _abc_data *scls_impl; int scls_is_abc = _get_optional_impl(state, scls, &scls_impl); if (scls_is_abc < 0) { @@ -925,7 +914,7 @@ static int subclasscheck_check_registry(_abc_data *impl, PyObject *subclass, PyObject **result) { - // Fast path: check subclass is in weakref directly. + // Fast path: check subclass is in weakset directly. int ret = _in_weak_set(impl, &impl->_abc_registry, subclass); if (ret < 0) { *result = NULL;