gh-92810: Return __subclasses__clause back

Author: dolfinus

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

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

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:

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;
     }