Eliminate GC refcounting cycle on _connect exception

redis/connection.py

@@ -1576,6 +1576,14 @@ def _connect(self):
         # we want to mimic what socket.create_connection does to support
         # ipv4/ipv6, but we want to set options prior to calling
         # socket.connect()
+
+        # Last caught connection error.
+        # Re-thrown if we are unable to connect to any of the options returned
+        # by getaddrinfo.
+        # Note that we must clear this variable before returning - otherwise,
+        # a caught err's traceback points to this frame, which points to err.
+        # Clearing this lets refcounting reclaim the exception immediately
+        # without deferring to the python garbage collector.
         err = None
 
         for res in socket.getaddrinfo(
@@ -1602,6 +1610,10 @@ def _connect(self):
 
                 # set the socket_timeout now that we're connected
                 sock.settimeout(self.socket_timeout)
+
+                # If a previous connection attempt failed, clear the error
+                err = None
+
                 return sock
 
             except OSError as _:
@@ -1614,7 +1626,11 @@ def _connect(self):
                     sock.close()
 
         if err is not None:
-            raise err
+            try:
+                raise err
+            finally:
+                # Ensure we clear local references to caught exceptions
+                err = None
         raise OSError("socket.getaddrinfo returned an empty list")
 
     def _host_error(self):

tests/test_connection.py

@@ -5,6 +5,7 @@
 import selectors
 import socket
 import ssl
+import sys
 import threading
 import time
 import types
@@ -574,6 +575,69 @@ def test_disconnect__close_OSError(self):
         mock_sock.close.assert_called_once()
         assert conn._sock is None
 
+    def test_connect_breaks_exception_reference_cycle(self):
+        """
+        On connection failure, _connect must not leave its frame retaining the
+        raised exception.
+        The exception's traceback references the frame, so a retained local
+        would form a cycle only reclaimable by the GC.
+        The finally clause in _connect breaks it by clearing the local.
+        """
+        conn = Connection(host="localhost", port=6379)
+        addr_info = (socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 6379))
+        with (
+            patch.object(socket, "getaddrinfo", return_value=[addr_info]),
+            patch.object(socket, "socket") as socket_factory,
+        ):
+            socket_factory.return_value.connect.side_effect = OSError("refused")
+            with pytest.raises(OSError) as exc_info:
+                conn._connect()
+
+        # Locate the _connect frame in the propagated traceback and confirm its
+        # err local was cleared, proving no exception<->frame cycle survives.
+        connect_frame = None
+        tb = exc_info.value.__traceback__
+        while tb is not None:
+            if tb.tb_frame.f_code.co_name == "_connect":
+                connect_frame = tb.tb_frame
+            tb = tb.tb_next
+        assert connect_frame is not None
+        assert connect_frame.f_locals.get("err") is None
+
+    def test_connect_breaks_reference_cycle_when_a_later_address_succeeds(self):
+        """
+        When an address fails but a later one connects, _connect must not
+        return while still holding the caught exception.
+        The exception's traceback references the frame, so a retained local
+        would form a cycle only reclaimable by the GC.
+        """
+        conn = Connection(host="localhost", port=6379)
+        addr_infos = [
+            (socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 6379)),
+            (socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.2", 6379)),
+        ]
+
+        # _connect calls getaddrinfo, so hook that as a way to peek the caller
+        connect_frames = []
+
+        def capturing_getaddrinfo(*args, **kwargs):
+            connect_frames.append(sys._getframe(1))
+            return addr_infos
+
+        failing_sock, working_sock = MagicMock(), MagicMock()
+        failing_sock.connect.side_effect = OSError("refused")
+
+        with (
+            patch.object(socket, "getaddrinfo", capturing_getaddrinfo),
+            patch.object(socket, "socket", side_effect=[failing_sock, working_sock]),
+        ):
+            assert conn._connect() is working_sock
+
+        # Error should be cleared in the connect frame
+        assert len(connect_frames) == 1
+        (connect_frame,) = connect_frames
+        assert connect_frame.f_locals.get("err") is None
+
     @pytest.mark.parametrize(
         "connection_kwargs",
         [