Skip to content

Commit b928f97

Browse files
committed
Remove decreasing of created connections count when releasing not owned by connection pool connection(fixes issue #2832). (#3514)
* Removing decreasing of created connections count when releasing not owned by connection pool connection(#2832). * Fixed another issue that was allowing adding connections to a pool owned by other pools. Adding unit tests. * Fixing a typo in a comment
1 parent 35ca102 commit b928f97

File tree

3 files changed

+21
-16
lines changed

3 files changed

+21
-16
lines changed

redis/connection.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,18 +1532,18 @@ def release(self, connection: "Connection") -> None:
15321532
except KeyError:
15331533
# Gracefully fail when a connection is returned to this pool
15341534
# that the pool doesn't actually own
1535-
pass
1535+
return
15361536

15371537
if self.owns_connection(connection):
15381538
self._available_connections.append(connection)
15391539
self._event_dispatcher.dispatch(
15401540
AfterConnectionReleasedEvent(connection)
15411541
)
15421542
else:
1543-
# pool doesn't own this connection. do not add it back
1544-
# to the pool and decrement the count so that another
1545-
# connection can take its place if needed
1546-
self._created_connections -= 1
1543+
# Pool doesn't own this connection, do not add it back
1544+
# to the pool.
1545+
# The created connections count should not be changed,
1546+
# because the connection was not created by the pool.
15471547
connection.disconnect()
15481548
return
15491549

tests/test_connection_pool.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,21 @@ def test_reuse_previously_released_connection(self, master_host):
9191
c2 = pool.get_connection()
9292
assert c1 == c2
9393

94+
def test_release_not_owned_connection(self, master_host):
95+
connection_kwargs = {"host": master_host[0], "port": master_host[1]}
96+
pool1 = self.get_pool(connection_kwargs=connection_kwargs)
97+
c1 = pool1.get_connection("_")
98+
pool2 = self.get_pool(
99+
connection_kwargs={"host": master_host[0], "port": master_host[1]}
100+
)
101+
c2 = pool2.get_connection("_")
102+
pool2.release(c2)
103+
104+
assert len(pool2._available_connections) == 1
105+
106+
pool2.release(c1)
107+
assert len(pool2._available_connections) == 1
108+
94109
def test_repr_contains_db_info_tcp(self):
95110
connection_kwargs = {
96111
"host": "localhost",

tests/test_multiprocessing.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,6 @@ def exit_callback(callback, *args):
2222

2323

2424
class TestMultiprocessing:
25-
# On macOS and newly non-macOS POSIX systems (since Python 3.14),
26-
# the default method has been changed to forkserver.
27-
# The code in this module does not work with it,
28-
# hence the explicit change to 'fork'
29-
# See https://github.com/python/cpython/issues/125714
30-
if multiprocessing.get_start_method() in ["forkserver", "spawn"]:
31-
_mp_context = multiprocessing.get_context(method="fork")
32-
else:
33-
_mp_context = multiprocessing.get_context()
34-
3525
# Test connection sharing between forks.
3626
# See issue #1085 for details.
3727

@@ -123,7 +113,7 @@ def target(pool, parent_conn):
123113
assert child_conn in pool._available_connections
124114
assert parent_conn not in pool._available_connections
125115

126-
proc = self._mp_context.Process(target=target, args=(pool, parent_conn))
116+
proc = multiprocessing.Process(target=target, args=(pool, parent_conn))
127117
proc.start()
128118
proc.join(3)
129119
assert proc.exitcode == 0

0 commit comments

Comments
 (0)