Clarify issue #253 in the code, add tests

aiohttp/connector.py

@@ -201,6 +201,10 @@ def _release(self, key, req, transport, protocol, *, should_close=False):
         if should_close or (reader.output and not reader.output.at_eof()):
             conns = self._conns.get(key)
             if conns is not None and len(conns) == 0:
+                # Issue #253: An empty array will eventually be
+                # removed by cleanup, but it's better to pop straight
+                # away, because cleanup might not get called (e.g. if
+                # keepalive is False).
                 self._conns.pop(key, None)
             transport.close()
         else:

tests/test_connector.py

@@ -184,6 +184,40 @@ def test_release_close(self):
         self.assertFalse(conn._conns)
         self.assertTrue(tr.close.called)
 
+    def test_release_dont_clean_conns(self):
+        # see issue #253
+        conn = aiohttp.BaseConnector(loop=self.loop)
+        req = unittest.mock.Mock()
+        resp = unittest.mock.Mock()
+        resp.message.should_close = True
+        req.response = resp
+
+        key = ('127.0.0.1', 80, False)
+
+        tr, proto = unittest.mock.Mock(), unittest.mock.Mock()
+        conn._conns[key] = [(tr, proto, 123)]
+
+        conn._release(key, req, tr, proto)
+        self.assertEqual({key: [(tr, proto, 123)]}, conn._conns)
+        self.assertTrue(tr.close.called)
+
+    def test_release_pop_empty_conns(self):
+        # see issue #253
+        conn = aiohttp.BaseConnector(loop=self.loop)
+        req = unittest.mock.Mock()
+        resp = unittest.mock.Mock()
+        resp.message.should_close = True
+        req.response = resp
+
+        key = ('127.0.0.1', 80, False)
+
+        conn._conns[key] = []
+
+        tr, proto = unittest.mock.Mock(), unittest.mock.Mock()
+        conn._release(key, req, tr, proto)
+        self.assertEqual({}, conn._conns)
+        self.assertTrue(tr.close.called)
+
     def test_release_close_do_not_delete_existing_connections(self):
         key = 1
         tr1, proto1 = unittest.mock.Mock(), unittest.mock.Mock()