File leak in ElementTree.iterparse()

[serhiy-storchaka]

BPO	25688
Nosy	@scoder, @vadmium, @serhiy-storchaka
Files	etree_iterparse_leak.patch etree_iterparse_leak_2.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 = None
closed_at = <Date 2015-11-23.13:58:32.253>
created_at = <Date 2015-11-21.11:11:33.131>
labels = ['library', 'performance']
title = 'File leak in ElementTree.iterparse()'
updated_at = <Date 2015-11-23.13:58:32.251>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2015-11-23.13:58:32.251>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2015-11-23.13:58:32.253>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2015-11-21.11:11:33.131>
creator = 'serhiy.storchaka'
dependencies = []
files = ['41111', '41129']
hgrepos = []
issue_num = 25688
keywords = ['patch']
message_count = 6.0
messages = ['255051', '255127', '255136', '255147', '255157', '255160']
nosy_count = 5.0
nosy_names = ['scoder', 'eli.bendersky', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue25688'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

[serhiy-storchaka]

ElementTree.iterparse() can leak internally open file in case of error. Proposed patch fixes the leak.

[vadmium]

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.

[serhiy-storchaka]

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.

[vadmium]

Patch 2 looks good. I like the new version of the tests better.

[python-dev]

New changeset 6e23777948f3 by Serhiy Storchaka in branch '3.4':
Issue bpo-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 bpo-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 bpo-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 bpo-25688: Fixed file leak in ElementTree.iterparse() raising an error.
https://hg.python.org/cpython/rev/09a8ac75b351

[serhiy-storchaka]

Thank you for your review Martin. Opened bpo-25707 for adding the close method.