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.

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

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

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.

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

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

(I left some comments on Rietveld.)

Addressed review comments

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

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

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\>

---