tempfile.NamedTemporaryFile can close too early if used as iterator

[bkabrda]

BPO	23700
Nosy	@birkenfeld, @pfmoore, @ncoghlan, @vstinner, @bitdancer, @ethanfurman, @serhiy-storchaka, @wm75
Files	python-3.4-tempfile-iter.patch python-3.4-tempfile-iter-v2.patch tempfile_iter_fix.patch: Works, but I don't know why csv_iter_commemt.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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-03-20.15:22:24.390>
created_at = <Date 2015-03-18.15:17:06.581>
labels = ['type-bug', 'library']
title = 'tempfile.NamedTemporaryFile can close too early if used as iterator'
updated_at = <Date 2015-03-22.19:28:57.514>
user = 'https://bugs.python.org/bkabrda'

bugs.python.org fields:

activity = <Date 2015-03-22.19:28:57.514>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-03-20.15:22:24.390>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2015-03-18.15:17:06.581>
creator = 'bkabrda'
dependencies = []
files = ['38543', '38556', '38586', '38635']
hgrepos = []
issue_num = 23700
keywords = ['patch']
message_count = 35.0
messages = ['238451', '238505', '238506', '238510', '238511', '238512', '238513', '238518', '238575', '238606', '238611', '238613', '238614', '238615', '238617', '238619', '238621', '238622', '238623', '238664', '238677', '238678', '238683', '238691', '238697', '238698', '238699', '238700', '238757', '238895', '238911', '238917', '238918', '238919', '238929']
nosy_count = 12.0
nosy_names = ['georg.brandl', 'paul.moore', 'ncoghlan', 'vstinner', 'Arfrever', 'r.david.murray', 'ethan.furman', 'python-dev', 'serhiy.storchaka', 'bkabrda', 'sYnfo', 'wolma']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23700'
versions = ['Python 3.4', 'Python 3.5']

[bkabrda]

This bug is very similar to bpo-18879, the only difference is that _TemporaryFileWrapper.__iter__ is the problem (in bpo-18879, __getattr__ was fixed, but __iter__ was not). The real world use case that helped me find this bug is at the bottom of this report, this is a simple reproducer:

>>> import tempfile
>>> for l in tempfile.NamedTemporaryFile(mode='a+b'):
...     print(l)
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: readline of closed file

I'm attaching a patch that fixes this (+ testcase).

Note: I actually discovered this while using

>>> from urllib.request import urlopen
>>> for l in urlopen('ftp://<some_ftp>'):
...     print(l)
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: readline of closed file

Opening FTP uses urllib.response, which in turn uses tempfile._TemporaryFileWrapper, which makes this example fail.

[bkabrda]

I'm attaching second version of the patch. It now contains link to this bug and a more real test case according to suggestion from the review.

One of the reviews of first patch version mentioned that there should be a better explanation of how this issue can be provoked. I think that the test shows it nice and there's no need to explain it in greater detail. Also, the comment references this bug, where anyone can find the explanation. (Fix for bpo-18879 fixes pretty much the same in a different method and has very similar comment, so I think this should be fine.)

[serhiy-storchaka]

LGTM.

[pfmoore]

Agreed, the test is sufficient documentation. However, I can't make the test fail here (Windows 7, Python 3.4.3):

py ti.py
b'spam\n' b'spam\n'
b'eggs\n' b'eggs\n'
b'beans\n' b'beans\n'
cat ti.py
import tempfile

def test_iter():
    # getting iterator from a temporary file should keep it alive
    # as long as it's being iterated over
    lines = [b'spam\n', b'eggs\n', b'beans\n']
    def make_file():
        f = tempfile.NamedTemporaryFile(mode='w+b')
        f.write(b''.join(lines))
        f.seek(0)
        return iter(f)
    for i, l in enumerate(make_file()):
        print(l, lines[i])

test_iter()

Is it somehow OS-specific?

Regardless, the patch seems fine and I have no problem with it being applied.

[python-dev]

New changeset 7fa741fe9425 by Serhiy Storchaka in branch '3.4':
Issue bpo-23700: Iterator of NamedTemporaryFile now keeps a reference to
https://hg.python.org/cpython/rev/7fa741fe9425

New changeset c84a0b35999a by Serhiy Storchaka in branch 'default':
Issue bpo-23700: Iterator of NamedTemporaryFile now keeps a reference to
https://hg.python.org/cpython/rev/c84a0b35999a

[serhiy-storchaka]

Thank you for your contribution Bohuslav.

[bkabrda]

Thank you!
To answer Paul's question: I honestly have no idea why this can't be reproduced on Windows. I managed to reproduce this in 100 % cases on various RPM-flavour Linux distros (Fedora, CentOS, RHEL) as well as on Debian and Ubuntu.

[pfmoore]

Cool, no problem.

[vstinner]

test_csv now fails on Windows:

http://buildbot.python.org/all/builders/x86 Windows7 3.x/builds/9421/

======================================================================
ERROR: test_read_dict_fieldnames_from_file (test.test_csv.TestDictFields)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_csv.py", line 629, in test_read_dict_fieldnames_from_file
    self.assertEqual(next(reader), {"f1": '1', "f2": '2', "f3": 'abc'})
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\csv.py", line 110, in __next__
    row = next(self.reader)
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\tempfile.py", line 431, in __iter__
    yield from iter(self.file)
ValueError: I/O operation on closed file.

[serhiy-storchaka]

Following patch fixes the issue, but I don't understand why.