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

Already on GitHub? Sign in to your account

ResourceWarning in python 3.2+ #1882

Closed
bboe opened this Issue Jan 25, 2014 · 14 comments

Comments

Projects
None yet
9 participants
Contributor

bboe commented Jan 25, 2014

Requests issues a ResourceWarning in python 3.2+ as sockets are not explicitly closed before garbage collection occurs. While ResourceWarnings are not displayed by default, it can be a distraction to some developers when working with warnings enabled.

File: test.py

import requests

def make_request():
    resp = requests.get('http://google.com')
    resp.close()  # this appears to have no effect, even though the function exists

make_request()
$ python -Wall test.py 
test.py:7: ResourceWarning: unclosed <socket.socket object, fd=4, family=2, type=1, proto=6>
  make_request()
test.py:7: ResourceWarning: unclosed <socket.socket object, fd=3, family=2, type=1, proto=6>
  make_request()

It would be great if there was a way to prevent the ResourceWarning from occurring, without issuing a Connection:close header.

Owner

Lukasa commented Jan 25, 2014

Thanks for this!

This is actually a behaviour in urllib3. I'm going to summon @shazow into this conversation, but I believe this is a deliberate design decision. We can often reuse the socket object to reconnect to the same host without the overhead of recreating the object. This makes this deliberate. I wonder if we can silence the warning.

Contributor

bboe commented Jan 25, 2014

I think the problem might be fixed (in urllib3's end) if when the connection pool is garbage collected, it explicitly closes all of its sockets, rather than using the previously okay (didn't issue warnings in python < 3.2) method of allowing garbage collection to close them.

Contributor

shazow commented Jan 25, 2014

urllib3 has a pool.close() which does this. We decided explicit is better than automatically doing it on garbage collection because you may be manually hanging on to connections/requests outside of the pool (e.g. when streaming).

here commented May 6, 2014

I also ran into this in writing unit tests.

As a workaround, here are docs on temporarily suppressing warnings on a case by case basis.

import warnings

def fxn():
    warnings.warn("deprecated", DeprecationWarning)

with warnings.catch_warnings():
    warnings.simplefilter("ignore")
    fxn()
Owner

sigmavirus24 commented May 6, 2014

Thanks @here

after for some wired reason suppressing warnings didnt work for me (still dont know why)
i came across this:

   req=requests.get('http://google.com')
   req.connection.close()

and no more "ResourceWarning"....
is it an actual solution? dose it work for you too?

mgrandi commented Aug 14, 2014

Is this fixed? I'm using

closing(requests.post("something.com", stream=True)) as r:
    # do stuff

and i'm still getting (while using this within a unittest.TestCase method)

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/unittest/case.py:574: ResourceWarning: unclosed <socket.socket fd=7, family=AddressFamily.AF_INET, type=SocketType.SOCK_STREAM, proto=6, laddr=('someip', 57353), raddr=('someip', 81)> testMethod()

Owner

Lukasa commented Aug 15, 2014

This is not fixed, because we're not entirely confident it's a bug. =) The warning is annoying but it's not symptomatic of a problem.

mgrandi commented Aug 15, 2014

@Lukasa: if this is not a bug, then what exactly is it? It seems to be that its complaining that a connection was left open after the unit tests ended, but how can that be if I'm wrapping the requests.post() call with contextlib.closing() like the documentation mentions here? http://docs.python-requests.org/en/latest/user/advanced/#body-content-workflow am I doing something incorrect?

Owner

Lukasa commented Aug 15, 2014

Nope, you're just running into the magic of requests. =)

We use connection pooling under the covers. This means we attempt to recycle socket objects as much as possible, by leaving them connected. This is an efficiency gain, as creating socket objects is expensive and connecting them is even more expensive.

When you use requests.X, that creates a Session and ConnectionPool under the covers and then throws them away. This behaviour means the socket object owned by the ConnectionPool is cleaned up by the garbage collector, which will close the socket when it does so.

What's important is that response.close() does not close all open sockets on the underlying ConnectionPool because that would be ridiculous, it just returns the socket to the pool. To get the behaviour you want (please close all sockets), you need to use an explicit Session object and then clean up with Session.close().

@iambibhas iambibhas referenced this issue in iambibhas/instamojo-py Nov 4, 2014

@iambibhas iambibhas added a test case and travis config 5c20437

Would it be possible to expose a requests.close() method to access the global, shared Session hiding in the module? Or perhaps requests.set_session()? Some better alternative than plumbing a Session through everything and creating a leaky abstraction?

Owner

sigmavirus24 commented Nov 7, 2014

@thejohnfreeman we create a new session every time you make a request with requests.{get,post,put,delete,etc.}. See the relevant code in requests/api.py

@sigmavirus24 sigmavirus24 added a commit to sigmavirus24/requests that referenced this issue Nov 7, 2014

@sigmavirus24 sigmavirus24 Close sessions created in the functional API
This is related to #1882 and #1685. By calling close on the session, we
clear the PoolManager operated by the Session and close all sockets.

Fixes #1882
Partially-fixes #1685
3155bc9

@ContinuousFunction ContinuousFunction added a commit to ContinuousFunction/requests that referenced this issue Nov 14, 2014

@sigmavirus24 @ContinuousFunction sigmavirus24 + ContinuousFunction Close sessions created in the functional API
This is related to #1882 and #1685. By calling close on the session, we
clear the PoolManager operated by the Session and close all sockets.

Fixes #1882
Partially-fixes #1685
c0e927c

@mureev mureev referenced this issue in mureev/kinopoiskpy Nov 20, 2014

unknown Move project to python 3 interpreter 852f945
Contributor

siebenschlaefer commented Sep 2, 2015

@here suppressing the warning with warnings.catch_warnings() is not guaranteed to work because the warning is issued when the connection pool is garbage collected which can happen after the context manager has closed.

Owner

Lukasa commented Sep 3, 2015

@siebenschlaefer That's correct, which is why we recommend you use explicit Session objects. =)

@joohoi joohoi referenced this issue in plamere/spotipy Nov 4, 2015

@joohoi joohoi Close the connection when done 660f34b

@NNTin NNTin added a commit to NNTin/Reply-Dota-2-Reddit that referenced this issue Mar 30, 2016

@NNTin NNTin Resource Warning fix 608beff

@stefanoborini stefanoborini added a commit to simphony/simphony-remote that referenced this issue Jun 23, 2016

@stefanoborini stefanoborini Silence warning for resource leak
Fixes a warning due to a python request resource leak (from connectionpool
being GC before being cleaned up). This seems to be a request issue and I am
unsure we can do anything on our side. This PR simply silences the warning.

See requests/requests#1882
for details about the issue itself.
19930ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment