Always close implicit connector, even for keepalive responses. #79

Merged
merged 2 commits into from Jun 22, 2014

3 participants

@asvetlov

If network resource returns keepalive response an implicit connection is not closed now.
That's leads to resource leaks and eventually to OSError: [Errno 24] Too many open files.
I guess to always close implicit connection and process keepalives only if explicit connector has been passed to aiohttp.request.

See http://stackoverflow.com/questions/24298095/connections-arent-closing-with-python3-asyncio-concurrent-http-get-requests/24312441?noredirect=1 as initial report for the issue

@fafhrd91

I think 'force_close' should be TCPConnector parameter. should we also introduce keep-alive timer for each opened connection, similar to what server handler does?

@asvetlov

force_close for TCPConnector only or for BaseConnector also?
I like the second.

About keep-alive timer -- nice to have, but timer can be implemented in another issue.

@fafhrd91

Agree on both

@asvetlov

New patch is ready.
@fafhrd91 please review.

@fafhrd91

Lgtm

@asvetlov asvetlov merged commit e360b1b into master Jun 22, 2014

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
@asvetlov asvetlov deleted the close_implicit_connector branch Jun 22, 2014
@asvetlov

Thanks

@asvetlov

I think the fix worth to be published in 0.8.2 ASAP.
BTW I've got the same in my production code but did not figure out the source of problem.
After looking on stack overflow issue we've changed own code to explicit connector usage last Friday, but maybe some user will catch the same error.

@fafhrd91

ok, i'll make release later today.

@asvetlov

Thank you again.

@GeorgiCodes

@asvetlov and @fafhrd91. Thanks for fixing this! I was wondering what was going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment