Merge pull request #406 from gabtremblay/master

Fixing keep-alive

CONTRIBUTORS.txt

@@ -24,6 +24,7 @@ Elizabeth Leddy
 Erich Healy
 Eugene Chernyshov
 Frederik Gladhorn
+Gabriel Tremblay
 Georges Dubus
 Greg Holt
 Hugo Herter

aiohttp/connector.py

@@ -290,6 +290,13 @@ def connect(self, req):
                         self._conn_timeout, loop=self._loop)
                 else:
                     transport, proto = yield from self._create_connection(req)
+
+                if not self._force_close:
+                    if self._conns.get(key, None) is None:
+                        self._conns[key] = []
+
+                    self._conns[key].append((transport, proto,
+                                            self._loop.time()))
             except asyncio.TimeoutError as exc:
                 raise ClientTimeoutError(
                     'Connection timeout to host %s:%s ssl:%s' % key) from exc
@@ -351,12 +358,13 @@ def _release(self, key, req, transport, protocol, *, should_close=False):
         reader = protocol.reader
         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:
+            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)
+                if not acquired:
+                    self._conns.pop(key, None)
 
             transport.close()
         else:

tests/test_client_functional_newstyle.py

@@ -0,0 +1,121 @@
+"""Http client functional tests against aiohttp.web server"""
+
+import asyncio
+import socket
+import unittest
+
+import aiohttp
+from aiohttp import client, web, log
+
+
+class TestHttpClientFunctionalNewStyle(unittest.TestCase):
+
+    def setUp(self):
+        self.handler = None
+        self.loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(None)
+
+    def tearDown(self):
+        if self.handler:
+            self.loop.run_until_complete(self.handler.finish_connections())
+        self.loop.close()
+
+    def find_unused_port(self):
+        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        s.bind(('127.0.0.1', 0))
+        port = s.getsockname()[1]
+        s.close()
+        return port
+
+    @asyncio.coroutine
+    def create_server(self, method, path, handler=None):
+        app = web.Application(loop=self.loop)
+        if handler:
+            app.router.add_route(method, path, handler)
+
+        port = self.find_unused_port()
+        self.handler = app.make_handler(
+            debug=True, keep_alive_on=False,
+            access_log=log.access_logger)
+        srv = yield from self.loop.create_server(
+            self.handler, '127.0.0.1', port)
+        url = "http://127.0.0.1:{}".format(port) + path
+        self.addCleanup(srv.close)
+        return app, srv, url
+
+    def test_keepalive_two_requests_sucess(self):
+        @asyncio.coroutine
+        def handler(request):
+            body = yield from request.read()
+            self.assertEqual(b'', body)
+            return web.Response(body=b'OK')
+
+        @asyncio.coroutine
+        def go():
+            _, srv, url = yield from self.create_server('GET', '/', handler)
+            connector = aiohttp.TCPConnector(loop=self.loop)
+            r = yield from client.request('GET', url,
+                                          connector=connector, loop=self.loop)
+            yield from r.read()
+
+            r2 = yield from client.request('GET', url,
+                                           connector=connector, loop=self.loop)
+            yield from r2.read()
+            self.assertEqual(1, len(connector._conns))
+            connector.close()
+
+        self.loop.run_until_complete(go())
+
+    def test_keepalive_response_released(self):
+        @asyncio.coroutine
+        def handler(request):
+            body = yield from request.read()
+            self.assertEqual(b'', body)
+            return web.Response(body=b'OK')
+
+        @asyncio.coroutine
+        def go():
+            _, srv, url = yield from self.create_server('GET', '/', handler)
+            connector = aiohttp.TCPConnector(loop=self.loop)
+            r = yield from client.request('GET', url,
+                                          connector=connector, loop=self.loop)
+            yield from r.read()
+            r.release()
+
+            r2 = yield from client.request('GET', url,
+                                           connector=connector, loop=self.loop)
+            yield from r2.read()
+            r2.release()
+            self.assertEqual(1, len(connector._conns))
+            connector.close()
+
+        self.loop.run_until_complete(go())
+
+    def test_keepalive_server_force_close_connection(self):
+        @asyncio.coroutine
+        def handler(request):
+            body = yield from request.read()
+            self.assertEqual(b'', body)
+            response = web.Response(body=b'OK')
+            response.force_close()
+            return response
+
+        @asyncio.coroutine
+        def go():
+            _, srv, url = yield from self.create_server('GET', '/', handler)
+
+            connector = aiohttp.TCPConnector(loop=self.loop)
+
+            r = yield from client.request('GET', url,
+                                          connector=connector, loop=self.loop)
+            yield from r.read()
+            self.assertEqual(0, len(connector._conns))
+
+            r2 = yield from client.request('GET', url,
+                                           connector=connector, loop=self.loop)
+            yield from r2.read()
+
+            self.assertEqual(0, len(connector._conns))
+            connector.close()
+
+        self.loop.run_until_complete(go())

tests/test_connector.py

@@ -309,7 +309,7 @@ def test_release_close_do_not_delete_existing_connections(self):
         req.response = resp
 
         tr, proto = unittest.mock.Mock(), unittest.mock.Mock()
-        conn._acquired[key].append(tr)
+        conn._acquired[key].append(tr1)
         conn._release(key, req, tr, proto)
         self.assertEqual(conn._conns[key], [(tr1, proto1, 1)])
         self.assertTrue(tr.close.called)