Skip to content

Commit f18988d

Browse files
committed
Fixed another issue that was allowing adding connections to a pool owned by other pools. Adding unit tests.
1 parent 761b7e3 commit f18988d

File tree

3 files changed

+50
-1
lines changed

3 files changed

+50
-1
lines changed

redis/connection.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,7 @@ def release(self, connection: "Connection") -> None:
15251525
except KeyError:
15261526
# Gracefully fail when a connection is returned to this pool
15271527
# that the pool doesn't actually own
1528-
pass
1528+
return
15291529

15301530
if self.owns_connection(connection):
15311531
self._available_connections.append(connection)

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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,40 @@ def target(conn, ev):
8484
proc.join(3)
8585
assert proc.exitcode == 0
8686

87+
@pytest.mark.parametrize("max_connections", [2, None])
88+
def test_release_parent_connection_from_pool_in_child_process(
89+
self, max_connections, master_host
90+
):
91+
"""
92+
A connection owned by a parent should not decrease the _created_connections
93+
counter in child when released - when the child process starts to use the
94+
pool it resets all the counters that have been set in the parent process.
95+
"""
96+
97+
pool = ConnectionPool.from_url(
98+
f"redis://{master_host[0]}:{master_host[1]}",
99+
max_connections=max_connections,
100+
)
101+
102+
parent_conn = pool.get_connection("ping")
103+
104+
def target(pool, parent_conn):
105+
with exit_callback(pool.disconnect):
106+
child_conn = pool.get_connection("ping")
107+
assert child_conn.pid != parent_conn.pid
108+
pool.release(child_conn)
109+
assert pool._created_connections == 1
110+
assert child_conn in pool._available_connections
111+
pool.release(parent_conn)
112+
assert pool._created_connections == 1
113+
assert child_conn in pool._available_connections
114+
assert parent_conn not in pool._available_connections
115+
116+
proc = multiprocessing.Process(target=target, args=(pool, parent_conn))
117+
proc.start()
118+
proc.join(3)
119+
assert proc.exitcode == 0
120+
87121
@pytest.mark.parametrize("max_connections", [1, 2, None])
88122
def test_pool(self, max_connections, master_host):
89123
"""

0 commit comments

Comments
 (0)