DEV Community

LubuSeb
LubuSeb

Posted on

One Event-Loop Turn, One False Redis Capacity Error

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

redis-py is the Python client for Redis. Its asynchronous cluster implementation maintains a per-node connection pool with an in-use set, a free queue, and optional max_connections capacity.

The bug was not a leaked connection or a deadlock. It was one event-loop turn in which a usable pool slot existed in neither place.

Bug Fix or Performance Improvement

An active async cluster connection can be marked for reconnect while a previous disconnect is still waiting for the socket to close. The disconnect clears the reconnect flag before it suspends, but another error or maintenance notification can mark the connection again during that wait.

By the time ClusterNode.release() receives it, the connection is closed but marked again.

The old release path treated every marked connection the same:

if connection.should_reconnect():
    task = asyncio.create_task(self._disconnect_and_release(connection))
    self._background_tasks.add(task)
    task.add_done_callback(self._background_tasks.discard)
    return
self._free.append(connection)
Enter fullscreen mode Exit fullscreen mode

For an already-closed connection, the second disconnect performs no I/O. Its only useful operation is appending the connection to _free, and that operation is deferred until the background task runs.

At max_connections=1, a concurrent acquire in that gap sees:

_free is empty
len(_connections) == max_connections
Enter fullscreen mode Exit fullscreen mode

and raises MaxConnectionsError. Capacity is about to return, but the caller receives a real application error first.

Code

The fix and deterministic regression test are in redis-py PR #4256, addressing issue #4247.

The repaired branch keeps the background disconnect for connections that are still open. If the connection is already closed, it clears the stale flag and returns the slot immediately:

if connection.should_reconnect():
    if connection.is_connected:
        task = asyncio.create_task(self._disconnect_and_release(connection))
        self._background_tasks.add(task)
        task.add_done_callback(self._background_tasks.discard)
        return
    connection.reset_should_reconnect()
self._free.append(connection)
Enter fullscreen mode Exit fullscreen mode

This preserves the safety rule—never put a marked, connected socket back into circulation—without scheduling a no-op disconnect for a closed one.

My Improvements

Reproduce the race through the real command path

A test that manually set two flags and called release() would prove the branch, but not that production control flow can reach it. The regression test instead drives ClusterNode.execute_command() with a scripted connection and uses asyncio.Event objects to control the interleaving:

  1. Start sending the command.
  2. Mark active connections for reconnect.
  3. Let the response complete so the normal error path starts disconnecting.
  4. Wait until the disconnect has suspended.
  5. Mark the same connection again.
  6. Allow the disconnect to finish.
  7. Acquire the next connection immediately.

The final assertions check the behavior that matters:

assert node.acquire_connection() is connection
assert node._background_tasks == set()
assert connection.should_reconnect() is False
assert connection.disconnect_calls == 1
Enter fullscreen mode Exit fullscreen mode

Before the fix, that acquire can hit the transient false-capacity window and the release path schedules a redundant second disconnect. After the fix, the same connection is available inline.

Prefer the narrow invariant repair

One alternative was to add a waiting acquisition API and turn more of the pool surface asynchronous. That would change public behavior and add coordination around a symptom.

The smaller fix restores the existing pool invariant: once a closed connection is released, it should be reusable immediately. No new public method, retry policy, timeout, or queue is required.

Keep the scope honest

This affects the async cluster pool when a node is at an explicitly configured connection limit and a reconnect mark lands during disconnect. It does not affect the synchronous cluster pool or the standalone pools. It causes a transient spurious MaxConnectionsError, not a permanent client hang.

The focused cluster connection-handling suite passed 13 tests on Windows and Ubuntu WSL. The changed code passed the repository's lint task, Ruff checks, formatting, vulture, and git diff --check. The public PR also passed its automated bug review; the full Redis Cluster integration matrix remains upstream CI territory.

Result

An already-closed, re-marked connection now returns to the free queue in the same release call. The pool no longer reports false exhaustion simply because a no-op background task has not received its event-loop turn yet.

Concurrency bugs are often described as timing problems, but the useful question is usually about ownership: at every suspension point, which structure owns the resource, and can another task observe a state in which nobody can use it? Here, asking that question reduced the fix to six lines and made the regression deterministic.

Top comments (0)