Close a connection if an unexpected exception occurs while sending a …

…request (#2828)

.gitignore

@@ -14,6 +14,8 @@
 .cache
 .coverage
 .coverage.*
+.direnv
+.envrc
 .idea
 .installed.cfg
 .noseids
@@ -47,4 +49,5 @@ virtualenv.py
 .gitconfig
 .flake
 .python-version
-.pytest_cache
+.pytest_cache
+.vscode

CHANGES/2827.bugfix

@@ -0,0 +1 @@
+Close a connection if an unexpected exception occurs while sending a request

CONTRIBUTORS.txt

@@ -42,6 +42,7 @@ Boyi Chen
 Brett Cannon
 Brian C. Lane
 Brian Muller
+Bryce Drennan
 Carl George
 Cecile Tonglet
 Chien-Wei Huang

aiohttp/client.py

@@ -326,11 +326,14 @@ async def _request(self, method, url, *,
                     tcp_nodelay(conn.transport, True)
                     tcp_cork(conn.transport, False)
                     try:
-                        resp = req.send(conn)
                         try:
-                            await resp.start(conn, read_until_eof)
+                            resp = req.send(conn)
+                            try:
+                                await resp.start(conn, read_until_eof)
+                            except BaseException:
+                                resp.close()
+                                raise
                         except BaseException:
-                            resp.close()
                             conn.close()
                             raise
                     except ClientError:

tests/test_client_session.py

@@ -369,6 +369,7 @@ async def create_connection(req, traces=None):
         # return self.transport, self.protocol
         return mock.Mock()
     session._connector._create_connection = create_connection
+    session._connector._release = mock.Mock()
 
     with pytest.raises(aiohttp.ClientOSError) as ctx:
         await session.request('get', 'http://example.com')
@@ -377,6 +378,43 @@ async def create_connection(req, traces=None):
     assert e.strerror == err.strerror
 
 
+async def test_close_conn_on_error(create_session):
+    class UnexpectedException(BaseException):
+        pass
+
+    err = UnexpectedException("permission error")
+    req = mock.Mock()
+    req_factory = mock.Mock(return_value=req)
+    req.send = mock.Mock(side_effect=err)
+    session = create_session(request_class=req_factory)
+
+    connections = []
+    original_connect = session._connector.connect
+
+    async def connect(req, traces=None):
+        conn = await original_connect(req, traces=traces)
+        connections.append(conn)
+        return conn
+
+    async def create_connection(req, traces=None):
+        # return self.transport, self.protocol
+        conn = mock.Mock()
+        return conn
+
+    session._connector.connect = connect
+    session._connector._create_connection = create_connection
+    session._connector._release = mock.Mock()
+
+    with pytest.raises(UnexpectedException):
+        async with session.request('get', 'http://example.com') as resp:
+            await resp.text()
+
+    # normally called during garbage collection.  triggers an exception
+    # if the connection wasn't already closed
+    for c in connections:
+        c.__del__()
+
+
 async def test_cookie_jar_usage(loop, aiohttp_client):
     req_url = None