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 file descriptor leak due to stack frame reference #549

merged 2 commits into from Feb 16, 2015


Copy link

Fixes #548

Copy link

So it'd be excellent if there were a test for this, but I don't know how you would begin to test this in the context of urllib3's test suite. That said, @shazow or @Lukasa should definitely be more helpful in that regard. The one thing I'd request is that you leave a comment near the sock = None line explaining the relation to issue #548

Also, I think those test failures are purely Travis' fault but if they continue, we may need to look further into why they're failing.

Copy link
Sponsor Contributor

Lukasa commented Feb 15, 2015

I suspect the way to test this is to test the cause of what we're fixing. Actually demonstrating a reference cycle in a test is a bit tricky, but you can test that the frame local variables don't have the stacktrace. Bit of a tricky test to write, but if we think it's important it's totally do-able.

@shazow shazow changed the title Fix for issue #548 Fix file descriptor leak due to stack frame reference Feb 16, 2015
Copy link

shazow commented Feb 16, 2015

Looks safe to me. :) While a test is always appreciated, I'm not sure exactly how to approach it in a way that scales across Python implementations. Happy to merge it as-is, but if you find a test you'd like to add then it's very welcome!

shazow added a commit that referenced this pull request Feb 16, 2015
Fix file descriptor leak due to stack frame reference.
@shazow shazow merged commit 78bf482 into urllib3:master Feb 16, 2015
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Mar 10, 2015

2.5.3 (2015-02-24)


    Revert changes to our vendored certificate bundle. For more context see (#2455, #2456, and

2.5.2 (2015-02-23)

Features and Improvements

    Add sha256 fingerprint support. (urllib3/urllib3#540)
    Improve the performance of headers. (urllib3/urllib3#544)


    Copy pip’s import machinery. When downstream redistributors remove requests.packages.urllib3 the import machinery will continue to let those same symbols work. Example usage in requests’ documentation and 3rd-party libraries relying on the vendored copies of urllib3 will work without having to fallback to the system urllib3.
    Attempt to quote parts of the URL on redirect if unquoting and then quoting fails. (#2356)
    Fix filename type check for multipart form-data uploads. (#2411)
    Properly handle the case where a server issuing digest authentication challenges provides both auth and auth-int qop-values. (#2408)
    Fix a socket leak. (urllib3/urllib3#549)
    Fix multiple Set-Cookie headers properly. (urllib3/urllib3#534)
    Disable the built-in hostname verification. (urllib3/urllib3#526)
    Fix the behaviour of decoding an exhausted stream. (urllib3/urllib3#535)


    Pulled in an updated cacert.pem.
    Drop RC4 from the default cipher list. (urllib3/urllib3#551)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

File descriptor leak with HTTPS proxies via CONNECT
4 participants