From 6db47c52635b50eb64fe6497c795fe8d06c05a3a Mon Sep 17 00:00:00 2001
From: afonsojanu <afonso@typeble.com>
Date: Thu, 3 Sep 2026 18:31:55 +0100
Subject: [PATCH] Look up visit methods on the class instead of the instance

Visitor.visit() caches whichever visit_* method it resolves for a
given node type on self._method_cache. Since the lookup goes through
self, what gets cached is a bound method, and a bound method holds a
reference back to the instance it belongs to. That means every
TreeInterpreter ends up referencing itself through its own cache
(self -> _method_cache -> bound method -> self), so it can only be
freed by a cyclic GC pass instead of the usual refcounting. A
TreeInterpreter gets built fresh on every ParsedResult.search() call,
so under a busy workload this turns into a steady stream of avoidable
GC pressure.

Fetching the method off type(self) instead gives back a plain
function with no bound reference to any instance, so the cache no
longer creates a cycle. Calling it as method(self, node, ...) keeps
dispatch working exactly the same as before, subclass overrides
included.

Added a regression test that spies on TreeInterpreter.__init__ to grab
a weakref, runs a search, and checks the interpreter is already gone
with gc disabled and no explicit collect - which only holds if nothing
is keeping it alive through a cycle.
---
 jmespath/visitor.py   | 11 ++++++--
 tests/test_visitor.py | 63 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 tests/test_visitor.py

diff --git a/jmespath/visitor.py b/jmespath/visitor.py
index 15fb1774..71e34ad5 100644
--- a/jmespath/visitor.py
+++ b/jmespath/visitor.py
@@ -88,10 +88,17 @@ def visit(self, node, *args, **kwargs):
         node_type = node['type']
         method = self._method_cache.get(node_type)
         if method is None:
+            # Looking this up on the class instead of on self means we
+            # cache a plain function rather than a bound method.  A bound
+            # method holds a reference back to the instance it's bound to,
+            # so stashing one in an attribute of that same instance
+            # (self._method_cache) creates a reference cycle, and every
+            # single instance then needs a cyclic GC pass to be collected
+            # instead of going away as soon as its refcount hits zero.
             method = getattr(
-                self, 'visit_%s' % node['type'], self.default_visit)
+                type(self), 'visit_%s' % node['type'], type(self).default_visit)
             self._method_cache[node_type] = method
-        return method(node, *args, **kwargs)
+        return method(self, node, *args, **kwargs)
 
     def default_visit(self, node, *args, **kwargs):
         raise NotImplementedError("default_visit")
diff --git a/tests/test_visitor.py b/tests/test_visitor.py
new file mode 100644
index 00000000..f982defa
--- /dev/null
+++ b/tests/test_visitor.py
@@ -0,0 +1,63 @@
+#!/usr/bin/env python
+import gc
+import weakref
+
+from tests import unittest
+
+import jmespath
+from jmespath import visitor
+
+
+class TestVisitorMethodCache(unittest.TestCase):
+    def test_method_cache_does_not_create_reference_cycle(self):
+        # Visitor.visit() caches the resolved visit_* method on
+        # self._method_cache.  If that cache stores a bound method, the
+        # instance ends up holding a reference to itself
+        # (self -> _method_cache -> bound method -> self), and a whole
+        # TreeInterpreter (one gets built on every ParsedResult.search()
+        # call) can then only be reclaimed by the cyclic garbage
+        # collector instead of going away as soon as its refcount drops
+        # to zero. Under a heavy search workload that adds up to a lot
+        # of avoidable GC churn.
+        gc.disable()
+        try:
+            expression = jmespath.compile('a.b.c')
+            created = []
+            original_init = visitor.TreeInterpreter.__init__
+
+            def spy_init(self, *args, **kwargs):
+                original_init(self, *args, **kwargs)
+                created.append(weakref.ref(self))
+
+            visitor.TreeInterpreter.__init__ = spy_init
+            try:
+                result = expression.search({'a': {'b': {'c': 1}}})
+            finally:
+                visitor.TreeInterpreter.__init__ = original_init
+
+            self.assertEqual(result, 1)
+            self.assertEqual(len(created), 1)
+            # No gc.collect() here on purpose: if there were still a
+            # reference cycle, the interpreter would still be alive at
+            # this point even though nothing outside this test holds a
+            # reference to it any more.
+            self.assertIsNone(
+                created[0](),
+                "TreeInterpreter survived past its last external "
+                "reference without a gc.collect(), which means its "
+                "method cache is holding a reference cycle again.")
+        finally:
+            gc.enable()
+
+    def test_visit_still_dispatches_correctly_after_caching(self):
+        # The fix changes *what* gets stored in _method_cache (a plain
+        # function looked up on the class instead of a bound method on
+        # the instance), so repeat this a few times to make sure
+        # dispatch through the cached entry still reaches the right
+        # subclass-specific visit_* implementation.
+        for _ in range(3):
+            self.assertEqual(
+                jmespath.search('a.b.c', {'a': {'b': {'c': 42}}}), 42)
+        self.assertEqual(
+            jmespath.search('foo[*].bar', {'foo': [{'bar': 1}, {'bar': 2}]}),
+            [1, 2])