Skip to content

Return copies from _get_free/in_use_connections and fix async _mock#3967

Merged
petyaslavova merged 2 commits intoredis:masterfrom
bysiber:fix-pool-get-connections-thread-safety
Feb 20, 2026
Merged

Return copies from _get_free/in_use_connections and fix async _mock#3967
petyaslavova merged 2 commits intoredis:masterfrom
bysiber:fix-pool-get-connections-thread-safety

Conversation

@bysiber
Copy link
Copy Markdown
Contributor

@bysiber bysiber commented Feb 20, 2026

Two issues in MaintNotificationsConnectionPoolMixin:

1. _get_free_connections and _get_in_use_connections return direct references to internal collections

The lock is held only during return, but callers iterate the returned collection outside the lock (e.g., in re_auth_callback). Another thread mutating the pool can cause RuntimeError: list/set changed size during iteration. The BlockingConnectionPool already does this correctly by returning copies.

Fixed by returning list(self._available_connections) and set(self._in_use_connections).

2. Sync _mock is async def but called from sync context

re_auth_callback is a sync method that passes _mock as a failure callback to the sync Retry.call_with_retry. Calling an async def from sync context returns an unawaited coroutine. Removed the async keyword since this is intentionally a no-op callback.

Copy link
Copy Markdown
Collaborator

@petyaslavova petyaslavova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bysiber, thank you for your contribution! Great catch :)
Change looks good to me.

@petyaslavova petyaslavova added maintenance Maintenance (CI, Releases, etc) labels Feb 20, 2026
@petyaslavova petyaslavova merged commit 21c5175 into redis:master Feb 20, 2026
120 of 121 checks passed
petyaslavova added a commit that referenced this pull request Feb 25, 2026
…3967)

Co-authored-by: petyaslavova <petya.slavova@redis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Maintenance (CI, Releases, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants