classification
Title: RuntimeError when URLopener.ftpcache is full
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, eric.araujo, erik.bray, jesstess, orsenthil, python-dev, shiinee
Priority: normal Keywords: patch

Created on 2014-05-09 15:09 by erik.bray, last changed 2014-06-14 04:22 by shiinee. This issue is now closed.

Files
File name Uploaded Description Edit
urllib-request-ftpcache-error.patch erik.bray, 2014-05-09 15:09 review
urllib-request-ftpcache-test.patch shiinee, 2014-06-07 19:43 review
urllib-request-ftpcache-test.patch shiinee, 2014-06-07 22:01 review
Messages (10)
msg218170 - (view) Author: Erik Bray (erik.bray) * Date: 2014-05-09 15:09
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.
msg218188 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-05-09 17:38
Thanks for the report and patch.  Would you mind adding a unit test?

(Note that the “crash” type is for segfaults, not Python exceptions.)
msg218189 - (view) Author: Erik Bray (erik.bray) * Date: 2014-05-09 17:57
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.
msg219912 - (view) Author: Jessica McKellar (jesstess) * Date: 2014-06-07 04:25
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
msg219966 - (view) Author: Skyler Leigh Amador (shiinee) Date: 2014-06-07 19:43
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
msg219972 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-06-07 20:37
(I left some comments on Rietveld.)
msg219986 - (view) Author: Skyler Leigh Amador (shiinee) Date: 2014-06-07 22:01
Addressed review comments
msg219988 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-07 22:09
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
msg220101 - (view) Author: Erik Bray (erik.bray) * Date: 2014-06-09 15:31
Thanks Skyler for finishing the job.  I got busy/distracted.
msg220530 - (view) Author: Skyler Leigh Amador (shiinee) Date: 2014-06-14 04:22
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>
> _______________________________________
>
History
Date User Action Args
2014-06-14 04:22:49shiineesetmessages: + msg220530
2014-06-09 15:31:09erik.braysetmessages: + msg220101
2014-06-07 22:09:42python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg219988

resolution: fixed
stage: test needed -> resolved
2014-06-07 22:01:01shiineesetfiles: + urllib-request-ftpcache-test.patch

messages: + msg219986
2014-06-07 20:37:42benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg219972
2014-06-07 19:43:38shiineesetfiles: + urllib-request-ftpcache-test.patch
nosy: + shiinee
messages: + msg219966

2014-06-07 04:25:00jesstesssetnosy: + jesstess
messages: + msg219912
2014-05-09 17:57:12erik.braysetmessages: + msg218189
2014-05-09 17:38:40eric.araujosetversions: + Python 2.7, Python 3.4, Python 3.5
nosy: + eric.araujo

messages: + msg218188

type: crash -> behavior
stage: test needed
2014-05-09 15:16:57berker.peksagsetnosy: + orsenthil
2014-05-09 15:09:08erik.braycreate