Remove decreasing of created connections count when releasing not owned by connection pool connection(fixes issue redis#2832). (redis#3514)

* Removing decreasing of created connections count when releasing not owned by connection pool connection(redis#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

Author: petyaslavova

redis/connection.py

@@ -1532,18 +1532,18 @@ 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)
                 self._event_dispatcher.dispatch(
                     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 should not be changed,
+                # because the connection was not created by the pool.
                 connection.disconnect()
                 return
 

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",

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):
         """