Deadlock in PubSub class after update to 5.3.0

[ManelCoutinhoSensei]

Hi again!

I just updated redis-py from 5.2.1 to 5.3.0 and I noticed that while adding the Dispatchers there was deadlock introduced.

Summary

When using a client with Retries enabled and subscribing to a pattern using psubscribe, a deadlock can occur under certain conditions:

• If a punsubscribe is attempted and a disconnection (such as a broken pipe) happens during this process, a reconnection is triggered (via the PubSub._execute error handling).
• During reconnection, the client tries to re-subscribe to the previously subscribed patterns, but this process can result in a deadlock.

Full explanation

There was a new lock introduced before each PubSub command execution on commit 40e5fc1, which is part of the 5.3.0 release:

redis-py/redis/client.py

Lines 874 to 875 in 7130e1a

	with self._lock:
	self._execute(connection, connection.send_command, *args, **kwargs)

When a command execution fails, if Retries are enabled, there will be a reconnection attempt:

redis-py/redis/client.py

Lines 895 to 916 in 7130e1a

	def _reconnect(self, conn) -> None:
	"""
	The supported exceptions are already checked in the
	retry object so we don't need to do it here.
	In this error handler we are trying to reconnect to the server.
	"""
	conn.disconnect()
	conn.connect()
	def _execute(self, conn, command, *args, **kwargs):
	"""
	Connect manually upon disconnection. If the Redis server is down,
	this will fail and raise a ConnectionError as desired.
	After reconnection, the ``on_connect`` callback should have been
	called by the # connection to resubscribe us to any channels and
	patterns we were previously listening to
	"""
	return conn.retry.call_with_retry(
	lambda: command(*args, **kwargs),
	lambda _: self._reconnect(conn),
	)

However, when executing the connect we get to the final lines where the resubscriptions callbacks are triggered for pubsub:

redis-py/redis/connection.py

Lines 406 to 413 in 7130e1a

	# run any user callbacks. right now the only internal callback
	# is for pubsub channel/pattern resubscription
	# first, remove any dead weakrefs
	self._connect_callbacks = [ref for ref in self._connect_callbacks if ref()]
	for ref in self._connect_callbacks:
	callback = ref()
	if callback:
	callback(self)

This will hit the same lock again a cause a deadlock.

---

Let me know if you need any other information. 🙂

[evios]

+1

[petyaslavova]

Hi @ManelCoutinhoSensei, thanks for reporting this! We will have a look at it shortly.

[matejsp]

+1

[Darek07]

I ran into the same issue using django-celery-beat. Below are the logs. Had to downgrade Redis to version 5.2.1 to resolve it.

Exception ignored in: <function AsyncResult.__del__ at 0x7ffbf146b9c0>
 Traceback (most recent call last):
   File "/home/app/.local/lib/python3.11/site-packages/celery/result.py", line 417, in __del__
     self.backend.remove_pending_result(self)
   File "/home/app/.local/lib/python3.11/site-packages/celery/backends/asynchronous.py", line 208, in remove_pending_result
     self.on_result_fulfilled(result)
   File "/home/app/.local/lib/python3.11/site-packages/celery/backends/asynchronous.py", line 216, in on_result_fulfilled
     self.result_consumer.cancel_for(result.id)
   File "/home/app/.local/lib/python3.11/site-packages/celery/backends/redis.py", line 184, in cancel_for
     self._pubsub.unsubscribe(key)
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 1055, in unsubscribe
     return self.execute_command("UNSUBSCRIBE", *args)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 876, in execute_command
     self._execute(connection, connection.send_command, *args, **kwargs)
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 914, in _execute
     return conn.retry.call_with_retry(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/home/app/.local/lib/python3.11/site-packages/redis/retry.py", line 90, in call_with_retry
     fail(error)
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 916, in <lambda>
     lambda _: self._reconnect(conn),
               ^^^^^^^^^^^^^^^^^^^^^
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 904, in _reconnect
     conn.connect()
   File "/home/app/.local/lib/python3.11/site-packages/redis/connection.py", line 379, in connect
     self.connect_check_health(check_health=True)
   File "/home/app/.local/lib/python3.11/site-packages/redis/connection.py", line 413, in connect_check_health
     callback(self)
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 834, in on_connect
     self.subscribe(**channels)
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 1030, in subscribe
     ret_val = self.execute_command("SUBSCRIBE", *new_channels.keys())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/home/app/.local/lib/python3.11/site-packages/redis/client.py", line 875, in execute_command
     with self._lock:
   File "/home/app/.local/lib/python3.11/site-packages/celery/apps/beat.py", line 159, in _sync
     raise SystemExit()
 SystemExit: 

[matejsp]

@Darek07 did you try just released 6.2.0 ? It says in release notes that it fixes one deadlock.

[ManelCoutinhoSensei]

@matejsp I don't know which is the lock that causes the issue on django-celery but the one mentioned in the released 6.2.0 is the cluster one and not the PubSub that i mention here

[Darek07]

@Darek07 did you try just released 6.2.0 ? It says in release notes that it fixes one deadlock.

Unfortunately it doesn't help. With redis 6.2.0 the problem still appears.