Skip to content

Commit

Permalink
Fix possible pipeline connections leak (#3104)
Browse files Browse the repository at this point in the history
* Update cluster.py

When Executing "n.write()" may generate some unknown errors(e.g. DataError), which could result in the connection not being released.

* Update cluster.py

* Update cluster.py

release connection move to "try...finally"

* Update cluster.py

 fix the linters

* fix problems of code review
  • Loading branch information
ING-XIAOJIAN authored and dvora-h committed Feb 25, 2024
1 parent 316667e commit 4528726
Showing 1 changed file with 28 additions and 26 deletions.
54 changes: 28 additions & 26 deletions redis/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -2153,32 +2153,34 @@ def _send_cluster_commands(
# we dont' multiplex on the sockets as they come available,
# but that shouldn't make too much difference.
node_commands = nodes.values()
for n in node_commands:
n.write()

for n in node_commands:
n.read()

# release all of the redis connections we allocated earlier
# back into the connection pool.
# we used to do this step as part of a try/finally block,
# but it is really dangerous to
# release connections back into the pool if for some
# reason the socket has data still left in it
# from a previous operation. The write and
# read operations already have try/catch around them for
# all known types of errors including connection
# and socket level errors.
# So if we hit an exception, something really bad
# happened and putting any oF
# these connections back into the pool is a very bad idea.
# the socket might have unread buffer still sitting in it,
# and then the next time we read from it we pass the
# buffered result back from a previous command and
# every single request after to that connection will always get
# a mismatched result.
for n in nodes.values():
n.connection_pool.release(n.connection)
try:
node_commands = nodes.values()
for n in node_commands:
n.write()

for n in node_commands:
n.read()
finally:
# release all of the redis connections we allocated earlier
# back into the connection pool.
# we used to do this step as part of a try/finally block,
# but it is really dangerous to
# release connections back into the pool if for some
# reason the socket has data still left in it
# from a previous operation. The write and
# read operations already have try/catch around them for
# all known types of errors including connection
# and socket level errors.
# So if we hit an exception, something really bad
# happened and putting any oF
# these connections back into the pool is a very bad idea.
# the socket might have unread buffer still sitting in it,
# and then the next time we read from it we pass the
# buffered result back from a previous command and
# every single request after to that connection will always get
# a mismatched result.
for n in nodes.values():
n.connection_pool.release(n.connection)

# if the response isn't an exception it is a
# valid response from the node
Expand Down

0 comments on commit 4528726

Please sign in to comment.