Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File leak in ElementTree.iterparse() #69874

Closed
serhiy-storchaka opened this issue Nov 21, 2015 · 6 comments
Closed

File leak in ElementTree.iterparse() #69874

serhiy-storchaka opened this issue Nov 21, 2015 · 6 comments
Labels
performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@serhiy-storchaka
Copy link
Member

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
    Copy link
    Member Author

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

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir performance Performance or resource usage labels Nov 21, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 23, 2015

    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
    Copy link
    Member Author

    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
    Copy link
    Member

    vadmium commented Nov 23, 2015

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

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2015

    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
    Copy link
    Member Author

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants