Aiohttp: capacity/limit functionality is seriously broken in some way

[ExplodingCabbage]

Long story short

Requests made using a ClientSession whose connector has reached its connection limit simply never get made; they wait around until they eventually time out.

Steps to reproduce

Run this script:

import aiohttp
import asyncio

loop = asyncio.get_event_loop()
connector = aiohttp.TCPConnector(loop=loop, capacity=2)
client = aiohttp.ClientSession(connector=connector)

async def hit_it():
    await client.get('http://google.com')
    print('Done')

loop.run_until_complete(asyncio.gather(*[hit_it() for i in range(100)]))

Expected behaviour

I should eventually see 100 'Done' messages, with the requests being queued such that there are only ever 2 simultaneous requests at a time.

Actual behaviour

I only see 2 'Done' messages; the other requests eventually start timing out.

Other notes

• The same bug exists with the old limit functionality.
• Creating a single coroutine that awaits the gathered futures has exactly the same behaviour
• If I don't use a custom connector, the bug still appears - just with the default of 20 'Done' messages instead of 2

Your environment

Python 3.6 on Ubuntu, installed with apt-get.

[ExplodingCabbage]

Some print-based debugging reveals that there are really two separate failure modes here. The example I gave connects to http://google.com, which is a redirect, and most of the requests hang somewhere inside the if resp.status in (301, 302, 303, 307) and allow_redirects: in client.py (i.e. the requests all get made but a hang occurs somewhere between receiving the response and actually returning it!). If you replace http://google.com with https://github.com (which is not a redirect), then instead it's the line conn = yield from self._connector.connect(req) in ClientSession._request() that hangs. I'm not sure if these are two separate bugs with a common symptom, or if there's a common cause of both. Anyway, I'm off to bed, so I can't debug any further for you.

[fafhrd91]

sure, you should not do anything for me. thank you.

[fafhrd91]

original script does not release request

import aiohttp
import asyncio

loop = asyncio.get_event_loop()
connector = aiohttp.TCPConnector(loop=loop, capacity=2)
client = aiohttp.ClientSession(connector=connector)

async def hit_it():
    async with client.get('http://google.com') as resp:
        print('Done')

loop.run_until_complete(asyncio.gather(*[hit_it() for i in range(100)]))

[fafhrd91]

everything works as expected

[kxepal]

@fafhrd91
That's seems pretty common trap: I was wonder to know that my coworker spent whole day to find out why after Nth request everything get blocked - eventually, he forgot to release/close a given response.

How do you feel to raise a ResourceWarning/RuntimeWarning/WhateverWarning if connector hits the limit and holds it for a while?

[fafhrd91]

i think it is good idea. also we should release connection earlier, at least when we can detect end of payload.

[kxepal]

Cool. Will try to figure something then.

[ExplodingCabbage]

Apologies from my stupidity. :)

I guess the trap here comes from having an object (the _RequestContextManager instance) which is both awaitable and usable as an asynchronous context manager. If it was only the latter (i.e. __await__ wasn't defined on the _RequestContextManager), there'd be no way to screw this up like I did. I'm not sure if there's a good reason to have __await__ defined there? (Of course, even if there isn't, removing it outright would be a breaking change and need care.)

[fafhrd91]

@kxepal do you have any idea on eta on ResourceWarning that you mentioned?

[kxepal]

@fafhrd91 This weekend I think.

[lock[bot]]

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].