sentinel: broken weakref on destruction

[shahar-lev]

Version: redis-py version 4.5.4 Redis Server version 7.0.11

Platform: Python 3.9.16 on Debian 10 x86_64

Description:
The weakref.proxy reference that the sentinel connection holds to its pool might be dead in some cases, causing reference errors.
This can happen when gc collects a reference cycle and calls a destructor __del__.
If the connection pool is one of the objects that is about to be freed, the weak references to it might be "dead", causing the error.

Running the following:

from redis.sentinel import Sentinel
import gc

class Tester:
    def __init__(self):
        sentinel = Sentinel([('localhost', 26379)])
        self.master = sentinel.master_for('mymaster')
        self.master.set('foo', 'hello')
        self.master.connection_pool.get_connection('_').disconnect() # simulate disconnection
        self.circular = self # circular reference

    def __del__(self):
        self.master.set('foo', 'goodbye')

Tester()
gc.collect(2)

Results in this exception:

Exception ignored in: <function Tester.__del__ at 0x7fadd5c398b0>
Traceback (most recent call last):
  File "/root/tester.py", line 13, in __del__
    self.master.set('foo', 'goodbye')
  File "/usr/local/lib/python3.9/site-packages/redis/commands/core.py", line 2302, in set
    return self.execute_command("SET", *pieces, **options)
  File "/usr/local/lib/python3.9/site-packages/redis/client.py", line 1255, in execute_command
    conn = self.connection or pool.get_connection(command_name, **options)
  File "/usr/local/lib/python3.9/site-packages/redis/connection.py", line 1442, in get_connection
    connection.connect()
  File "/usr/local/lib/python3.9/site-packages/redis/sentinel.py", line 54, in connect
    return self.retry.call_with_retry(self._connect_retry, lambda error: None)
  File "/usr/local/lib/python3.9/site-packages/redis/retry.py", line 46, in call_with_retry
    return do()
  File "/usr/local/lib/python3.9/site-packages/redis/sentinel.py", line 43, in _connect_retry
    if self.connection_pool.is_master:
ReferenceError: weakly-referenced object no longer exists

The above test code was deduced from a real scenario that happens when using Celery with a Redis backend.
The actual traceback we keep getting in the real setup is the following:

Traceback (most recent call last):
  File "/opt/conda/lib/python3.8/site-packages/celery/result.py", line 392, in __del__
    self.backend.remove_pending_result(self)
  File "/opt/conda/lib/python3.8/site-packages/celery/backends/asynchronous.py", line 208, in remove_pending_result
    self.on_result_fulfilled(result)
  File "/opt/conda/lib/python3.8/site-packages/celery/backends/asynchronous.py", line 216, in on_result_fulfilled
    self.result_consumer.cancel_for(result.id)
  File "/opt/conda/lib/python3.8/site-packages/celery/backends/redis.py", line 176, in cancel_for
    self._pubsub.unsubscribe(key)
  File "/opt/conda/lib/python3.8/site-packages/redis/client.py", line 1648, in unsubscribe
    return self.execute_command("UNSUBSCRIBE", *args)
  File "/opt/conda/lib/python3.8/site-packages/redis/client.py", line 1458, in execute_command
    self.connection = self.connection_pool.get_connection(
  File "/opt/conda/lib/python3.8/site-packages/redis/connection.py", line 1442, in get_connection
    connection.connect()
  File "/opt/conda/lib/python3.8/site-packages/redis/sentinel.py", line 54, in connect
    return self.retry.call_with_retry(self._connect_retry, lambda error: None)
  File "/opt/conda/lib/python3.8/site-packages/redis/retry.py", line 46, in call_with_retry
    return do()
  File "/opt/conda/lib/python3.8/site-packages/redis/sentinel.py", line 43, in _connect_retry
    if self.connection_pool.is_master:
ReferenceError: weakly-referenced object no longer exists

It might not be safe to use weakref.proxy without checking if the reference is alive. I suggest using weakref.ref instead and checking it before use.

[Darkheir]

We have exactly the same issue, do you know since which version of redis-py the issue exists ?

We are thinking about downgrading the redis lib, but we need to find to which version :)

[shahar-lev]

@Darkheir I didn't run git bisect, so I can't really tell.

I would have guessed that this bug is a result of the introduction of the weakref proxy in the sentinel code, but this goes way back to this commit, which is as old as version 2.9.0.

[Darkheir]

@shahar-lev Thank you for the answer.

I also did a git blame and saw that it is almost 10 years since the weakref was introduced, but we got this issue in our platform quite recently.

Anyway thank you for the fix you pushed :)

[dvora-h]

Fixed in #2771

[rad-pat]

@dvora-h , can you please look at similar change for Async sentinel? #3205

[dvora-h]

approved. @gerzse can merge it