classification
Title: File leak in ElementTree.iterparse()
Type: resource usage Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eli.bendersky, martin.panter, python-dev, scoder, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-11-21 11:11 by serhiy.storchaka, last changed 2015-11-23 13:58 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
etree_iterparse_leak.patch serhiy.storchaka, 2015-11-21 11:11 review
etree_iterparse_leak_2.patch serhiy.storchaka, 2015-11-23 07:57 review
Messages (6)
msg255051 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-21 11:11
ElementTree.iterparse() can leak internally open file in case of error. Proposed patch fixes the leak.
msg255127 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-23 06:08
What’s the point of the catch_warnings() and filterwarnings() calls in the tests? They don’t seem to be doing much; I think the CleanContext manager is already enabling warnings. Perhaps you could call simplefilter("error", ResourceWarning) instead.

Also I’m not enthusiastic about the iterparse() API when passing a file name. If it has to stay, maybe there should be an explicit way to clean it up without exhausting the iterator, like a generator.close() method.
msg255136 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-23 07:57
Warning is emitted in destructor, and exceptions in destructors are ignored. Hence turning it into error makes the test passed.

Tests were not correct, here is fixed patch.

Yes, CleanContext does the work. But depending on it looks fragile. CleanContext can be removed after getting rid of all deprecated methods.

I agree that we have to add the close() method to iterparse object. But this is new feature and separate issue.
msg255147 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-23 11:16
Patch 2 looks good. I like the new version of the tests better.
msg255157 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-23 13:47
New changeset 6e23777948f3 by Serhiy Storchaka in branch '3.4':
Issue #25688: Fixed file leak in ElementTree.iterparse() raising an error.
https://hg.python.org/cpython/rev/6e23777948f3

New changeset 267d04459ba3 by Serhiy Storchaka in branch '3.5':
Issue #25688: Fixed file leak in ElementTree.iterparse() raising an error.
https://hg.python.org/cpython/rev/267d04459ba3

New changeset d841205776fe by Serhiy Storchaka in branch 'default':
Issue #25688: Fixed file leak in ElementTree.iterparse() raising an error.
https://hg.python.org/cpython/rev/d841205776fe

New changeset 09a8ac75b351 by Serhiy Storchaka in branch '2.7':
Issue #25688: Fixed file leak in ElementTree.iterparse() raising an error.
https://hg.python.org/cpython/rev/09a8ac75b351
msg255160 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-23 13:58
Thank you for your review Martin. Opened issue25707 for adding the close method.
History
Date User Action Args
2015-11-23 13:58:32serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg255160

stage: patch review -> resolved
2015-11-23 13:47:11python-devsetnosy: + python-dev
messages: + msg255157
2015-11-23 11:16:59martin.pantersetmessages: + msg255147
2015-11-23 07:57:42serhiy.storchakasetfiles: + etree_iterparse_leak_2.patch

messages: + msg255136
2015-11-23 06:08:16martin.pantersetnosy: + martin.panter
messages: + msg255127
2015-11-21 11:11:33serhiy.storchakacreate