[Python-checkins] bpo-46417: remove_subclass() clears tp_subclasses (GH-30793)

[vstinner]

https://github.com/python/cpython/commit/2d03b73cc9c0dada3243eab1373a46dbd98d24a0
commit: 2d03b73cc9c0dada3243eab1373a46dbd98d24a0
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2022-01-22T16:53:30+01:00
summary:

bpo-46417: remove_subclass() clears tp_subclasses (GH-30793)

The remove_subclass() function now deletes the dictionary when
removing the last subclass (if the dictionary becomes empty) to save
memory: set PyTypeObject.tp_subclasses to NULL. remove_subclass() is
called when a type is deallocated.

_PyType_GetSubclasses() no longer holds a reference to tp_subclasses:
its loop cannot modify tp_subclasses.

files:
M Lib/test/test_descr.py
M Objects/typeobject.c

diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index 707c93140e251..791cf714d46a3 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -4923,6 +4923,23 @@ def __new__(cls):
                 cls.lst = [2**i for i in range(10000)]
         X.descr
 
+    def test_remove_subclass(self):
+        # bpo-46417: when the last subclass of a type is deleted,
+        # remove_subclass() clears the internal dictionary of subclasses:
+        # set PyTypeObject.tp_subclasses to NULL. remove_subclass() is called
+        # when a type is deallocated.
+        class Parent:
+            pass
+        self.assertEqual(Parent.__subclasses__(), [])
+
+        class Child(Parent):
+            pass
+        self.assertEqual(Parent.__subclasses__(), [Child])
+
+        del Child
+        gc.collect()
+        self.assertEqual(Parent.__subclasses__(), [])
+
 
 class DictProxyTests(unittest.TestCase):
     def setUp(self):
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 2b47afe30e6ec..b3c305e0bf430 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -4137,16 +4137,17 @@ _PyType_GetSubclasses(PyTypeObject *self)
         return NULL;
     }
 
-    // Hold a strong reference to tp_subclasses while iterating on it
-    PyObject *dict = Py_XNewRef(self->tp_subclasses);
-    if (dict == NULL) {
+    PyObject *subclasses = self->tp_subclasses;  // borrowed ref
+    if (subclasses == NULL) {
         return list;
     }
-    assert(PyDict_CheckExact(dict));
+    assert(PyDict_CheckExact(subclasses));
+    // The loop cannot modify tp_subclasses, there is no need
+    // to hold a strong reference (use a borrowed reference).
 
     Py_ssize_t i = 0;
     PyObject *ref;  // borrowed ref
-    while (PyDict_Next(dict, &i, NULL, &ref)) {
+    while (PyDict_Next(subclasses, &i, NULL, &ref)) {
         assert(PyWeakref_CheckRef(ref));
         PyObject *obj = PyWeakref_GET_OBJECT(ref);  // borrowed ref
         if (obj == Py_None) {
@@ -4154,12 +4155,10 @@ _PyType_GetSubclasses(PyTypeObject *self)
         }
         assert(PyType_Check(obj));
         if (PyList_Append(list, obj) < 0) {
-            Py_CLEAR(list);
-            goto done;
+            Py_DECREF(list);
+            return NULL;
         }
     }
-done:
-    Py_DECREF(dict);
     return list;
 }
 
@@ -6568,6 +6567,13 @@ remove_subclass(PyTypeObject *base, PyTypeObject *type)
         PyErr_Clear();
     }
     Py_XDECREF(key);
+
+    if (PyDict_Size(dict) == 0) {
+        // Delete the dictionary to save memory. _PyStaticType_Dealloc()
+        // callers also test if tp_subclasses is NULL to check if a static type
+        // has no subclass.
+        Py_CLEAR(base->tp_subclasses);
+    }
 }
 
 static void