Don't remove broken connections multiple times from the pool

Fixes `ValueError: deque.remove(x): x not in deque` when multiple connections
to the same server brake more or less simultaneously.

Author: robsdedude

neo4j/io/__init__.py

@@ -772,13 +772,16 @@ def deactivate(self, address):
                 connections = self.connections[address]
             except KeyError:  # already removed from the connection pool
                 return
-            for conn in list(connections):
-                if not conn.in_use:
-                    connections.remove(conn)
-                    try:
-                        conn.close()
-                    except OSError:
-                        pass
+            closable_connections = [
+                conn for conn in connections if not conn.in_use
+            ]
+            # First remove all connections in question, then try to close them.
+            # If closing of a connection fails, we will end up in this method
+            # again.
+            for conn in closable_connections:
+                connections.remove(conn)
+            for conn in closable_connections:
+                conn.close()
             if not self.connections[address]:
                 del self.connections[address]
 

tests/unit/io/test_neo4j_pool.py

@@ -232,6 +232,36 @@ def test_release_does_not_resets_defunct_connections(opener):
     cx1.reset.asset_not_called()
 
 
+def test_multiple_broken_connections_on_close(opener):
+    def mock_connection_breaks_on_close(cx):
+        def close_side_effect():
+            cx.closed.return_value = True
+            cx.defunct.return_value = True
+            pool.deactivate(READER_ADDRESS)
+
+        cx.attach_mock(Mock(side_effect=close_side_effect), "close")
+
+    # create pool with 2 idle connections
+    pool = Neo4jPool(opener, PoolConfig(), WorkspaceConfig(), ROUTER_ADDRESS)
+    cx1 = pool.acquire(READ_ACCESS, 30, "test_db", None)
+    cx2 = pool.acquire(READ_ACCESS, 30, "test_db", None)
+    pool.release(cx1)
+    pool.release(cx2)
+
+    # both will loose connection
+    mock_connection_breaks_on_close(cx1)
+    mock_connection_breaks_on_close(cx2)
+
+    # force pool to close cx1, which will make it realize that the server is
+    # unreachable
+    cx1.stale.return_value = True
+
+    cx3 = pool.acquire(READ_ACCESS, 30, "test_db", None)
+
+    assert cx3 is not cx1
+    assert cx3 is not cx2
+
+
 def test_failing_opener_leaves_connections_in_use_alone(opener):
     pool = Neo4jPool(opener, PoolConfig(), WorkspaceConfig(), ROUTER_ADDRESS)
     cx1 = pool.acquire(READ_ACCESS, 30, "test_db", None)