diff --git a/redis/connection.py b/redis/connection.py index d59a9b069b..48c32ea5f8 100644 --- a/redis/connection.py +++ b/redis/connection.py @@ -1532,7 +1532,7 @@ def release(self, connection: "Connection") -> None: except KeyError: # Gracefully fail when a connection is returned to this pool # that the pool doesn't actually own - pass + return if self.owns_connection(connection): self._available_connections.append(connection) @@ -1540,10 +1540,10 @@ def release(self, connection: "Connection") -> None: AfterConnectionReleasedEvent(connection) ) else: - # pool doesn't own this connection. do not add it back - # to the pool and decrement the count so that another - # connection can take its place if needed - self._created_connections -= 1 + # Pool doesn't own this connection, do not add it back + # to the pool. + # The created connections count shouls not be changed, + # because the connection was not created by the pool. connection.disconnect() return diff --git a/tests/test_connection_pool.py b/tests/test_connection_pool.py index 387a0f4565..65f42923fe 100644 --- a/tests/test_connection_pool.py +++ b/tests/test_connection_pool.py @@ -91,6 +91,21 @@ def test_reuse_previously_released_connection(self, master_host): c2 = pool.get_connection() assert c1 == c2 + def test_release_not_owned_connection(self, master_host): + connection_kwargs = {"host": master_host[0], "port": master_host[1]} + pool1 = self.get_pool(connection_kwargs=connection_kwargs) + c1 = pool1.get_connection("_") + pool2 = self.get_pool( + connection_kwargs={"host": master_host[0], "port": master_host[1]} + ) + c2 = pool2.get_connection("_") + pool2.release(c2) + + assert len(pool2._available_connections) == 1 + + pool2.release(c1) + assert len(pool2._available_connections) == 1 + def test_repr_contains_db_info_tcp(self): connection_kwargs = { "host": "localhost", diff --git a/tests/test_multiprocessing.py b/tests/test_multiprocessing.py index 0e8e8958c5..8b9e9fb90b 100644 --- a/tests/test_multiprocessing.py +++ b/tests/test_multiprocessing.py @@ -84,6 +84,40 @@ def target(conn, ev): proc.join(3) assert proc.exitcode == 0 + @pytest.mark.parametrize("max_connections", [2, None]) + def test_release_parent_connection_from_pool_in_child_process( + self, max_connections, master_host + ): + """ + A connection owned by a parent should not decrease the _created_connections + counter in child when released - when the child process starts to use the + pool it resets all the counters that have been set in the parent process. + """ + + pool = ConnectionPool.from_url( + f"redis://{master_host[0]}:{master_host[1]}", + max_connections=max_connections, + ) + + parent_conn = pool.get_connection("ping") + + def target(pool, parent_conn): + with exit_callback(pool.disconnect): + child_conn = pool.get_connection("ping") + assert child_conn.pid != parent_conn.pid + pool.release(child_conn) + assert pool._created_connections == 1 + assert child_conn in pool._available_connections + pool.release(parent_conn) + assert pool._created_connections == 1 + assert child_conn in pool._available_connections + assert parent_conn not in pool._available_connections + + proc = multiprocessing.Process(target=target, args=(pool, parent_conn)) + proc.start() + proc.join(3) + assert proc.exitcode == 0 + @pytest.mark.parametrize("max_connections", [1, 2, None]) def test_pool(self, max_connections, master_host): """