andymccurdy/redis-py — pull request #409 by paulovn (2013-12-10 14:32:46+00:00)
Title: Circular reference in SentinelConnectionPool changed to weak reference, to enable garbage collection

The recommended procedure to work with a Sentinel-enabled Redis

from redis.sentinel import Sentinel
sentinel = Sentinel([('localhost', 26379)], socket_timeout=0.1)
master = sentinel.master_for('mymaster', socket_timeout=0.1)

returns a `master` object, which is typically a `StrictRedis` client. This object is then used to perform operations against the current Redis master. When the object falls out of scope (i.e. its reference count falls to zero) it is freed by Python.

However `master` contains also a `SentinelConnectionPool` object (the one holding the connections to the sentinel). Apparently this object persists even after falling out of scope, and as a result its socket connections to the Redis server remain open. The cause is this self-reference:

self.connection_kwargs['connection_pool'] = self

which does not let the reference count drop to 0. Theoretically the Python GC detects such cycles and is able to remove objects which do not have outside references (given that `SentinelConnectionPool` does not contain a `__del__` method) . But in a long standing process (server-side) I have observed a continuous increase in open connections as `Sentinel` objects get created and drop out of scope.

The use of a weak reference solves this problem:

self.connection_kwargs['connection_pool'] = weakref.proxy(self)

with no apparent side-effects.

PS Of course ideally objects such as `SentinelConnectionPool` should be kept and reused, and this would alleviate the problem. Still, having the safeguard mechanism of a weak reference seems advisable.