Skip to content

Commit

Permalink
Avoid creating ref cycles (#2932)
Browse files Browse the repository at this point in the history
This won't meaningfully affect most users, so I won't feel bad if this
is closed :-)

It's often important for performance to control when garbage collection
runs and to reduce the need to run garbage collection in the first
place.

During AbstractionConnection.on_connect, it's common to raise and catch
ResponseError, since e.g. CLIENT SETINFO is very new. However, because
response is in a local, it creates ref cycles that keep all locals in
all calling stack frames alive.

The use of a local to store the exception is unfortunate, since
exceptions hold references to their tracebacks, which hold references to
the relevant frames, which holds a reference to the local exception.
See https://peps.python.org/pep-0344/#open-issue-garbage-collection
and https://peps.python.org/pep-3110/#rationale

This breaks the cycle by deleting the local when we raise, so frames are
destroyed by the normal reference counting mechanism.
  • Loading branch information
hauntsaninja committed Sep 14, 2023
1 parent ba186d2 commit 578fb26
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,10 @@ def read_response(
self.next_health_check = time() + self.health_check_interval

if isinstance(response, ResponseError):
raise response
try:
raise response
finally:
del response # avoid creating ref cycles
return response

def pack_command(self, *args):
Expand Down

0 comments on commit 578fb26

Please sign in to comment.