classification
Title: ResourceWarning in urllib.request
Type: resource usage Stage: patch review
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alexis, daniel.ugra, davide.rizzo, eric.araujo, ezio.melotti, haypo, jhylton, nadeem.vawda, orsenthil, pitrou, python-dev
Priority: normal Keywords: needs review, patch

Created on 2011-05-21 12:42 by ezio.melotti, last changed 2011-07-18 08:31 by daniel.ugra. This issue is now closed.

Files
File name Uploaded Description Edit
issue12133.diff ezio.melotti, 2011-05-21 12:42 Patch against 3.3 to fix the resource warning. review
Messages (13)
msg136433 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-05-21 12:42
In case of error (e.g. timeout error), urllib.request leaves the socket open:

import urllib.request as ur
import socket
s = socket.socket()
s.bind(('localhost', 10000))
s.listen(0)
socket.setdefaulttimeout(5)
ur.urlopen('http://localhost.localdomain:10000')

outputs:

sys:1: ResourceWarning: unclosed <socket.socket object, fd=4, family=2, type=1, proto=6>

Traceback (most recent call last):
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 1146, in do_open
    r = h.getresponse()  # an HTTPResponse instance
  File "/home/wolf/dev/py/py3k/Lib/http/client.py", line 1046, in getresponse
    response.begin()
  File "/home/wolf/dev/py/py3k/Lib/http/client.py", line 346, in begin
    version, status, reason = self._read_status()
  File "/home/wolf/dev/py/py3k/Lib/http/client.py", line 308, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/home/wolf/dev/py/py3k/Lib/socket.py", line 279, in readinto
    return self._sock.recv_into(b)
socket.timeout: timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 138, in urlopen
    return opener.open(url, data, timeout)
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 369, in open
    response = self._open(req, data)
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 387, in _open
    '_open', req)
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 347, in _call_chain
    result = func(*args)
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 1163, in http_open
    return self.do_open(http.client.HTTPConnection, req)
  File "/home/wolf/dev/py/py3k/Lib/urllib/request.py", line 1148, in do_open
    raise URLError(err)
urllib.error.URLError: <urlopen error timed out>
>>>

AFAIU, when urlopen returns or raises, the socket can be closed, so the attached patch adds a "finally" that calls close() on the HTTPConnection object.  The test suite pass (except for a mock that was missing the close method), but I'm not 100% sure that it's always safe to call close().

This ResourceWarning has been exposed by test_packaging.
msg136436 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-05-21 13:05
Hi Ezio, the connection can be closed via the finally call as you do in the patch. There are times when request object is re-used, but before the connection is made. It may also help to understand how the code in the packaging was invoking it.

If you run the whole suite and see the nothing breaks (to ensure that something is not waiting for the socket and trying to close later), go ahead with the patch.
msg136437 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-05-21 13:16
The packaging test (test_pypi_simple.py:test_uses_mirrors) creates a server and a mirror, starts the mirror only, tries to connect to the server, and then falls back on the mirror when the server raises a timeout error.
The code in the first message does more or less the same, causing a timeout error and the resource warning.

I haven't checked in detail the urllib tests, but the fact that there are no ResourceWarnings while running test_urllib might mean that this case isn't currently tested.
msg138393 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-06-15 21:53
Oh, I wrote a similar patch to kill the ResourceWarning of test_pypi_simple (except that I didn't patch test_urllib2).
msg138500 - (view) Author: Roundup Robot (python-dev) Date: 2011-06-17 12:02
New changeset ad6bdfd7dd4b by Victor Stinner in branch '3.2':
Issue #12133: fix a ResourceWarning in urllib.request
http://hg.python.org/cpython/rev/ad6bdfd7dd4b

New changeset 57a98feb508e by Victor Stinner in branch 'default':
(Merge 3.2) Issue #12133: fix a ResourceWarning in urllib.request
http://hg.python.org/cpython/rev/57a98feb508e
msg138501 - (view) Author: Roundup Robot (python-dev) Date: 2011-06-17 12:06
New changeset 18e6ccc332d5 by Victor Stinner in branch '2.7':
Issue #12133: AbstractHTTPHandler.do_open() of urllib.request closes the HTTP
http://hg.python.org/cpython/rev/18e6ccc332d5
msg138502 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-06-17 12:07
I tested the patch on Python 3.3: the full test suite pass on Linux. I applied your patch on Python 2.7, 3.2 and 3.3, thanks Ezio.
msg138532 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-06-17 17:42
Yay!
msg140508 - (view) Author: Ugra Dániel (daniel.ugra) Date: 2011-07-16 23:27
This patch has introduced some problems for me with Python 3.2.1 (64-bit Arch Linux).

The following code:

with urllib.request.urlopen(url) as page:
    pass

raises "ValueError: I/O operation on closed file." exception when url is "http://www.imdb.com/".

When I removed "h.close()" (added by this patch) from request.py everything worked as expected.

Interestingly other URLs work flawlessly with patched code ("http://www.google.com/" for example).

I had no time to further investigate the differences between HTTP responses of "good" and "bad" sites... and I am by no means an HTTP expert :)

Should I open a new bug report for this one or is it OK to just leave this comment here?
msg140509 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-07-16 23:29
> Should I open a new bug report for this one or is it OK to just leave
> this comment here?

I think it's better to open a new issue, thank you.
msg140565 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-18 07:40
I reopen the issue.
msg140566 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-18 07:41
(Oh, I missed Antoine's comment, yes, reopen a new issue)
msg140568 - (view) Author: Ugra Dániel (daniel.ugra) Date: 2011-07-18 08:31
Sorry, I've forgotten to post a reference to the new bug: #12576
History
Date User Action Args
2011-07-18 08:31:54daniel.ugrasetmessages: + msg140568
2011-07-18 07:41:53hayposetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg140566
2011-07-18 07:40:25hayposetstatus: closed -> open
resolution: fixed -> accepted
messages: + msg140565
2011-07-16 23:29:36pitrousetmessages: + msg140509
2011-07-16 23:27:34daniel.ugrasetnosy: + daniel.ugra
messages: + msg140508
2011-06-17 17:42:19eric.araujosetnosy: + eric.araujo, alexis
messages: + msg138532
2011-06-17 12:07:21hayposetstatus: open -> closed
resolution: fixed
messages: + msg138502
2011-06-17 12:06:30python-devsetmessages: + msg138501
2011-06-17 12:02:40python-devsetnosy: + python-dev
messages: + msg138500
2011-06-15 21:53:41hayposetnosy: + haypo

messages: + msg138393
versions: + Python 2.7, Python 3.2
2011-05-21 13:16:33ezio.melottisetmessages: + msg136437
2011-05-21 13:05:54orsenthilsetnosy: + orsenthil
messages: + msg136436
2011-05-21 12:48:43nadeem.vawdasetnosy: + nadeem.vawda
2011-05-21 12:42:35ezio.melotticreate