Arfrever Frehtes Taifersar Arahesis added the comment:
Comment in Lib/tempfile.py mentions issue #23000, but should mention issue
#23700.
--
nosy: +Arfrever
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
Serhiy Storchaka added the comment:
Indeed. And all the comment could be better.
Does anyone want to write better comment in the light of recent estimations?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
R. David Murray added the comment:
How's this?
--
Added file: http://bugs.python.org/file38635/csv_iter_commemt.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
Serhiy Storchaka added the comment:
LGTM
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Python-bugs-list mailing list
Unsubscribe:
Roundup Robot added the comment:
New changeset e9f03315d66c by R David Murray in branch '3.4':
#23700: fix/improve comment
https://hg.python.org/cpython/rev/e9f03315d66c
New changeset 64f4dbac9d07 by R David Murray in branch 'default':
Merge: #23700: fix/improve comment
STINNER Victor added the comment:
Yeah, the new comment is better :-) Thanks.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Serhiy Storchaka added the comment:
Following patch fixes the issue, but I don't understand why.
--
Added file: http://bugs.python.org/file38586/tempfile_iter_fix.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
Serhiy Storchaka added the comment:
No, it doesn't help.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Python-bugs-list mailing
Wolfgang Maier added the comment:
I think this is a consequence of PEP380 and its decision to finalize the
subgenerator when the delegating generator is closed.
Consider this simple example without tempfile:
def yielder (fileobj):
yield from fileobj
with open('some_test_file', 'w') as f:
STINNER Victor added the comment:
Maybe we need to keep explicitly a reference to self.file in the method
(file = self.file) to keep it alive in the frame?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
Wolfgang Maier added the comment:
Actually, its scary that use of yield from can have such a subtle side-effect.
Maybe PEP380 should have taken this more seriously?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
Serhiy Storchaka added the comment:
Ah yes, correct: when a generator using yield from obj is destroyed while
yield from is not done, obj.close() is called if the method exists.
But why obj.close() is called? The reference to fileobj is live, it shouldn't
be closed.
This solution looks
Wolfgang Maier added the comment:
@Serhiy
in this line of code:
reader = csv.DictReader(fileobj, fieldnames=next(csv.reader(fileobj)))
csv.reader(fileobj) returns the generator created by fileobj.__iter__, but no
reference to it is kept so the object gets destroyed right afterwards. This
STINNER Victor added the comment:
Ah yes, correct: when a generator using yield from obj is destroyed while
yield from is not done, obj.close() is called if the method exists.
So yield from file *is* different than for line in file: yield file when we
don't consume the whole generator.
A
Bohuslav Slavek Kabrda added the comment:
@wolma any idea why this only happens on Windows? I can't reproduce the CSV
failing test on Linux.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
Roundup Robot added the comment:
New changeset a90ec6b96af2 by Serhiy Storchaka in branch '3.4':
Issue #23700: NamedTemporaryFile iterator closed underlied file object in
https://hg.python.org/cpython/rev/a90ec6b96af2
New changeset e639750ecd92 by Serhiy Storchaka in branch 'default':
Issue
Wolfgang Maier added the comment:
so let's look at this step-by-step (and I hope I fully understood this myself):
- calling fileobj.__iter__ creates a generator because the method uses yield
from
- that generator does not get assigned to any reference so it will be
garbage-collected
- when
STINNER Victor added the comment:
tempfile_iter_fix.patch looks good to me, can you commit it please?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
Serhiy Storchaka added the comment:
csv.reader(fileobj) returns the generator created by fileobj.__iter__, but no
reference to it is kept so the object gets destroyed right afterwards. This
closes the generator and because it uses yield from also the contained
subgenerator, which is the
Serhiy Storchaka added the comment:
Thank you for your explanation Wolfgang! Now it is clear to me. The issue is
that the generator calls the close() method of the subgenerator, but if the
subgenerator is a file, the close() method closes (surprise!) the file. Two
different protocols use the
R. David Murray added the comment:
Isn't there some discussion somewhere that if iter(x) returns x you probably
have buggy code? Maybe it is io that is broken, design-wise. I think there
was another issue related to iter(file) recently...someone surprised by the
fact that you can't iterate
Changes by Serhiy Storchaka storch...@gmail.com:
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
R. David Murray added the comment:
There's actually an issue about exactly that broken-per-docs issue for io. IMO
if the goal of calling close is to close only things that are generator objects
or pretending to be one, the method should have been named close_generator or
something.
STINNER Victor added the comment:
Isn't there some discussion somewhere that if iter(x) returns x you probably
have buggy code?
I agree that the issue comes from TextIOWrapper.__iter__(),
BufferedReader.__iter__() and FileIO.__iter__() returns simply return
self *and* have a close method.
Wolfgang Maier added the comment:
OTOH, it would be very hard to change the way fileobjects compared to designing
yield from differently so I'd still blame it partly.
Maybe it is unfortunate that generators have a close method instead of, say,
__close__ ?
--
Wolfgang Maier added the comment:
You are probably right that the io classes are broken.
From https://docs.python.org/3/library/stdtypes.html#iterator-types:
Once an iterator’s __next__() method raises StopIteration, it must continue to
do so on subsequent calls. Implementations that do not
Wolfgang Maier added the comment:
@bkabrda
not sure, but it may have to do with when exactly the object gets garbage
collected
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
STINNER Victor added the comment:
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)
Changes by Wolfgang Maier wolfgang.ma...@biologie.uni-freiburg.de:
--
nosy: +wolma
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Bohuslav Slavek Kabrda added the comment:
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
Roundup Robot added the comment:
New changeset 7fa741fe9425 by Serhiy Storchaka in branch '3.4':
Issue #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 #23700:
Serhiy Storchaka added the comment:
LGTM.
--
assignee: - serhiy.storchaka
stage: patch review - commit review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
Bohuslav Slavek Kabrda added the comment:
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.
--
Paul Moore added the comment:
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
Paul Moore added the comment:
Cool, no problem.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Python-bugs-list mailing list
Serhiy Storchaka added the comment:
Thank you for your contribution Bohuslav.
--
resolution: - fixed
stage: commit review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
New submission from Bohuslav Slavek Kabrda:
This bug is very similar to #18879, the only difference is that
_TemporaryFileWrapper.__iter__ is the problem (in #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
Changes by Matěj Stuchlík matej.stuch...@gmail.com:
--
nosy: +sYnfo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
___
Python-bugs-list
Changes by Ethan Furman et...@stoneleaf.us:
--
nosy: +ethan.furman, georg.brandl, ncoghlan
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
Changes by Serhiy Storchaka storch...@gmail.com:
--
nosy: +serhiy.storchaka
stage: - patch review
versions: -Python 3.6
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23700
___
40 matches
Mail list logo
