_RequestContextManager will not close connection if ClientConnectorError has happened

[asnelzin]

Long story short

When using _RequestContextManager and try to get a host which can not be connected to, connection will not be released to the pool.

Expected behaviour

Raises an Exception and connection releases.

Actual behaviour

Raises an Exception.

Steps to reproduce

import asyncio

import aiohttp


async def get_image(session, url):
    async with session.get(url) as response:
        if response.status != 200:
            print('Something went wrong!')
            return None

        return await response.read()


async def worker():
    conn = aiohttp.TCPConnector(limit=1)
    async with aiohttp.ClientSession(connector=conn) as session:
        await asyncio.gather(
            asyncio.ensure_future(get_image(session, 'http://unavailable:9090/test.jpg')),
            asyncio.ensure_future(get_image(session, 'http://unavailable:9090/test.jpg'))
        )


if __name__ == '__main__':
    loop = asyncio.get_event_loop()

    loop.run_until_complete(worker())
    loop.close()

Your environment

macOS 10.12.6
Python 3.6.1
aiohttp 2.2.5

[matthiasblondeel]

I think I recently hit the same issue and believe I found the root cause, which resides in the BaseConnector class (in connect.py)

Below is the way I currently understand the issue:
Let us assume we have two connection requests coming in as is the case in the above repro.

The first connection passes the limit, and goes into the _create_connection method
The second connection, does not pass the limit, and so gets appended to the list of waiters, waiting to be notified to start up again.
Normally this notification happens when a previous Connection object gets created and released (in the release of close call).

However if the previous _create_connection raises, no connection object gets created. The self._acquired gets cleaned up correctly in this case, but noone notifies that a new connection is now available, and so the waiter never gets freed.

This has the observable outcome that a waiter will always be stuck waiting, and that your connection pool size is effectively reduced by one.

Hopefully this is helpful in fixing the bug.

[asnelzin]

@matthiasblondeel great explanation! Do you have any ideas how to fix that?

[fafhrd91]

I will look into this problem during this weekend

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new