Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a potential connection leak on tzdata update #59

Merged
merged 1 commit into from
Nov 23, 2018

Conversation

enerick
Copy link
Contributor

@enerick enerick commented Mar 16, 2018

This PR fixes a potential connection leak on Tzdata.ReleaseUpdater.poll_for_update/1.

In hackney_pool, connections won't be checked in until the response body was read or skipped by calling functions like :hackney.body/1 or :hackney.skip_body/1 (this is expected behavior as of benoitc/hackney#160).

Current implementation possibly cause the connection leak if Tzdata.DataLoader.latest_file_size_by_get/1 (invoked by Tzdata.ReleaseUpdater.poll_for_update/1) returns the status code other than 200.

Tzdata uses :default pool of hackney for HTTP request, so other applications using :default pool can also be affected.

Note

I suspected that #56 is maybe related this fix, since IANA will return 302 with "www.iana.org" (fixed by #51), and that is exactly the situation that connection leak will happen.

@enerick enerick changed the title Fix a potential connection leak on Tzdata.ReleaseUpdater.poll_for_update/1 Fix a potential connection leak on tzdata update Mar 16, 2018
@enerick enerick force-pushed the fix-connection-leak-on-data-loader branch from e3fc97f to 64984e5 Compare March 16, 2018 09:25
@enerick
Copy link
Contributor Author

enerick commented Mar 16, 2018

hmm... test failed in Elixir 1.1.1 (no luck with rebuilding) 😕

I tried building latest master (8d5b195) on my fork, and it failed as well.
https://travis-ci.org/enerick/tzdata/builds/354277392

In hackney_pool, connections won't be checked in until the response body
was read or skipped by calling functions like `:hackney.body/1` or
`:hackney.skip_body/1` (this is expected behavior as of
benoitc/hackney#160).
@enerick enerick force-pushed the fix-connection-leak-on-data-loader branch from 64984e5 to a783cbf Compare June 7, 2018 14:25
@enerick
Copy link
Contributor Author

enerick commented Jun 7, 2018

Hi @lau
I retried CI with the latest master and now it looks fine.
Would you be willing to review this PR?

@lau
Copy link
Owner

lau commented Nov 13, 2018

Hi @enerick
It has been a long time, but I will have some time to look at this soon.

@lau lau merged commit 9a21f65 into lau:master Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants