RuntimeError when URLopener.ftpcache is full

[embray]

BPO	21463
Nosy	@orsenthil, @benjaminp, @merwok, @embray
Files	urllib-request-ftpcache-error.patch urllib-request-ftpcache-test.patch urllib-request-ftpcache-test.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-06-07.22:09:42.297>
created_at = <Date 2014-05-09.15:09:08.006>
labels = ['type-bug', 'library']
title = 'RuntimeError when URLopener.ftpcache is full'
updated_at = <Date 2014-06-14.04:22:49.387>
user = 'https://github.com/embray'

bugs.python.org fields:

activity = <Date 2014-06-14.04:22:49.387>
actor = 'shiinee'
assignee = 'none'
closed = True
closed_date = <Date 2014-06-07.22:09:42.297>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2014-05-09.15:09:08.006>
creator = 'erik.bray'
dependencies = []
files = ['35199', '35522', '35529']
hgrepos = []
issue_num = 21463
keywords = ['patch']
message_count = 10.0
messages = ['218170', '218188', '218189', '219912', '219966', '219972', '219986', '219988', '220101', '220530']
nosy_count = 7.0
nosy_names = ['orsenthil', 'benjamin.peterson', 'eric.araujo', 'jesstess', 'python-dev', 'erik.bray', 'shiinee']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21463'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[embray]

This is probably a pretty rare corner case, but a coworker reported this to me while testing code that does open several ftp connections to different files.

[merwok]

Thanks for the report and patch. Would you mind adding a unit test?

(Note that the “crash” type is for segfaults, not Python exceptions.)

[embray]

Ah, didn't know that about "crash".

I wanted to add a test but hesitated only because that code is not well tested to begin with (perhaps, hence, this going unnoticed for so long). But I guess it could be done by mocking the ftpwrapper class.

[jesstess]

I want to state explicitly what the error is for some new contributors who might pick this up at a sprint this weekend:

The issue is that you can't change a dictionary while iterating over it:

>>> d = {"a": "b"}
>>> for elt in d.keys():
...     del d[elt]
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: dictionary changed size during iteration

In Python 2, d.keys() produced a copy of the list of keys, and we delete items from the dictionary while iterating over that copy, which is safe. In Python 3, d.keys() produces a view object* instead, and mutating the dictionary while iterating over it is unsafe and raises an error.

The patch makes a copy of the keys before iterating over it so that it is safe in Python 3.

• https://docs.python.org/3/library/stdtypes.html#dict-views

[shiinee]

I've made a test for this patch with a very minimal mock ftpwrapper. We can see it fails on dictionary size change without Erik's fix:

======================================================================
ERROR: test_ftp_cache_pruning (test.test_urllib.urlopen_HttpTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/skyler/cpython/Lib/test/test_urllib.py", line 336, in test_ftp_cache_pruning
    urlopen('ftp://localhost')
  File "/home/skyler/cpython/Lib/test/test_urllib.py", line 45, in urlopen
    return opener.open(url)
  File "/home/skyler/cpython/Lib/urllib/request.py", line 1631, in open
    return getattr(self, name)(url)
  File "/home/skyler/cpython/Lib/urllib/request.py", line 1914, in open_ftp
    for k in self.ftpcache.keys():
RuntimeError: dictionary changed size during iteration

[benjaminp]

(I left some comments on Rietveld.)

[shiinee]

Addressed review comments

[python-dev]

New changeset b8f9ae84d211 by Benjamin Peterson in branch '3.4':
in ftp cache pruning, avoid changing the size of a dict while iterating over it (closes bpo-21463)
http://hg.python.org/cpython/rev/b8f9ae84d211

New changeset 6f70a18313e5 by Benjamin Peterson in branch 'default':
merge 3.4 (bpo-21463)
http://hg.python.org/cpython/rev/6f70a18313e5

[embray]

Thanks Skyler for finishing the job. I got busy/distracted.

[shiinee]

You're welcome, happy to help :)

On Mon, Jun 9, 2014 at 8:31 AM, Erik Bray <report@bugs.python.org> wrote:

Erik Bray added the comment:

Thanks Skyler for finishing the job. I got busy/distracted.

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue21463\>

---