Fix keepalive connections not being closed in time (#4956)

Co-authored-by: Andrew Svetlov <[email protected]>

CHANGES/3296.bugfix

@@ -0,0 +1,10 @@
+Fix keepalive connections not being closed in time
+
+Refactoring in 964921d4e97e7c84bcfda6772ed458549aea0b09 introduced a
+regression so that `_cleanup()` could be called only once or few times.
+`_release()` expects `self._cleanup_handle` to be None to add new
+`weakref_handle`. But when `_cleanup()` called and there are no
+remaining connections, `self._cleanup_handle` will remain as
+`<TimerHandle cancelled when=5853434>`, so `_release()` will not have a
+chance to schedule `_cleanup()` call again.
+

CONTRIBUTORS.txt

@@ -287,6 +287,7 @@ Vincent Maillol
 Vitalik Verhovodov
 Vitaly Haritonsky
 Vitaly Magerya
+Vladimir Kamarzin
 Vladimir Kozlovski
 Vladimir Rutsky
 Vladimir Shulyak

aiohttp/connector.py

@@ -284,6 +284,9 @@ def _cleanup(self) -> None:
         """Cleanup unused transports."""
         if self._cleanup_handle:
             self._cleanup_handle.cancel()
+            # _cleanup_handle should be unset, otherwise _release() will not
+            # recreate it ever!
+            self._cleanup_handle = None
 
         now = self._loop.time()
         timeout = self._keepalive_timeout

tests/test_connector.py

@@ -376,6 +376,7 @@ async def test_release(loop, key) -> None:
 
     conn._release(key, proto)
     assert conn._release_waiter.called
+    assert conn._cleanup_handle is not None
     assert conn._conns[key][0][0] == proto
     assert conn._conns[key][0][1] == pytest.approx(loop.time(), abs=0.1)
     assert not conn._cleanup_closed_transports
@@ -1198,7 +1199,7 @@ async def test_cleanup(key) -> None:
     conn._cleanup()
     assert existing_handle.cancel.called
     assert conn._conns == {}
-    assert conn._cleanup_handle is not None
+    assert conn._cleanup_handle is None
 
 
 async def test_cleanup_close_ssl_transport(loop, ssl_key) -> None: