select_related() leaks memory

[Jacob Walls]

Calls to select_related() create nested functions that aren't cleaned up until garbage collection runs:

Borrowed testing strategy from #34865:

tests/select_related/tests.py

@@ -242 +242 @@
             FETCH_PEERS,
         )
 
+    def test_memory_leak(self):
+        # todo: factor out a test util
+        import gc
+    
+        # Schedule the restore of the garbage collection settings.
+        self.addCleanup(gc.set_debug, 0)
+        self.addCleanup(gc.enable)
+
+        # Disable automatic garbage collection to control when it's triggered,
+        # then run a full collection cycle to ensure `gc.garbage` is empty.
+        gc.disable()
+        gc.collect()
+
+        # The garbage list isn't automatically populated to avoid CPU overhead,
+        # so debugging needs to be enabled to track all unreachable items and
+        # have them stored in `gc.garbage`.
+        gc.set_debug(gc.DEBUG_SAVEALL)
+
+        list(Species.objects.select_related("genus"))
+
+        # Enforce garbage collection to populate `gc.garbage` for inspection.
+        gc.collect()
+        self.assertEqual(gc.garbage, [])
+
 
 class SelectRelatedValidationTests(SimpleTestCase):
     """

Gives:

AssertionError: Lists differ: [<cell at 0x10630c940: function object at [148 chars]040>] != []

First list contains 3 additional elements.
First extra element 0:
<cell at 0x10630c940: function object at 0x1079b8040>

+ []
- [<cell at 0x10630c940: function object at 0x1079b8040>,
-  (<cell at 0x10630c940: function object at 0x1079b8040>,),
-  <function SQLCompiler.get_select.<locals>.get_select_from_parent at 0x1079b8040>]

At a glance, it looks like we can lift get_select_from_parent to a class method or all the way to module-level without a loss of functionality to address this.

[Simon Charette]

Good catch, looks like it could even be a staticmethod since it only cares about klass_info.

[nzioker]

I'm working on this fix.

[nzioker]

PR created: ​https://github.com/django/django/pull/19977

[Simon Charette]

MR was targeting 4.2.

Also it appears that my analysis was wrong. Since we're recursively calling the function it'd be better if it was a classmethod instead so we can do cls.get_select_from_parent(ki) in the function definition. We should also do self.get_select_from_parent instead of SQLCompiler.get_select_from_parent in .get_select.

[nzioker]

Here's the new PR: ​https://github.com/django/django/pull/19986

[Jacob Walls <jacobtylerwalls@…>]

In 3ff32c50:

Fixed #36674 -- Fixed memory leak in select_related().