Issue 21463: RuntimeError when URLopener.ftpcache is full

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 #21463)
http://hg.python.org/cpython/rev/b8f9ae84d211

New changeset 6f70a18313e5 by Benjamin Peterson in branch 'default':
merge 3.4 (#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>
> _______________________________________
>

nosy: + python-dev
messages: + msg219988

resolution: fixed
stage: test needed -> resolved

messages: + msg219986

messages: + msg218188

type: crash -> behavior
stage: test needed