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

Jump to bottom

os.scandir(str) reference leak (test_os refleak) #73699

Closed
vstinner opened this issue Feb 9, 2017 · 10 comments
Closed

os.scandir(str) reference leak (test_os refleak) #73699

vstinner opened this issue Feb 9, 2017 · 10 comments
Assignees
@serhiy-storchaka
Labels
3.7 (EOL) end of life performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

vstinner commented Feb 9, 2017

BPO 29513
Nosy @vstinner, @serhiy-storchaka, @zhangyangyu
Files
  • scandir-refleak-3.6.patch
  • scandir-refleak-3.7.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 2017-02-09.18:08:13.490>
created_at = <Date 2017-02-09.12:09:14.511>
labels = ['3.7', 'library', 'performance']
title = 'os.scandir(str) reference leak (test_os refleak)'
updated_at = <Date 2017-02-09.21:16:00.157>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-02-09.21:16:00.157>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-02-09.18:08:13.490>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2017-02-09.12:09:14.511>
creator = 'vstinner'
dependencies = []
files = ['46608', '46609']
hgrepos = []
issue_num = 29513
keywords = ['patch']
message_count = 10.0
messages = ['287409', '287415', '287416', '287420', '287422', '287423', '287424', '287447', '287451', '287456']
nosy_count = 4.0
nosy_names = ['vstinner', 'python-dev', 'serhiy.storchaka', 'xiang.zhang']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue29513'
versions = ['Python 3.6', 'Python 3.7']
    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 9, 2017

    The following code leaks one reference on Python 3.6:

        def test_attributes(self):
        d = dict((entry.name, entry) for entry in os.scandir('.'))

    Or try the command:

    ./python -m test -R 3:3 test_os -v -m test_attributes

    @vstinner vstinner added stdlib Python modules in the Lib dir performance Performance or resource usage labels Feb 9, 2017
    @serhiy-storchaka
    Copy link
    Member

    The owning of references in the path_t structure was changed in bpo-29034, but some code still directly manipulates reference counters of path_t fields. Proposed patch should fix the leak.

    @serhiy-storchaka
    Copy link
    Member

    3.7 doesn't leak, but contains outdated comment and code.

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Feb 9, 2017
    @zhangyangyu
    Copy link
    Member

    3.6 LGTM. 3.7 is technically right to me. But it looks to me AC shouldn't call path_cleanup in .c.h. It always do nothing.

    @serhiy-storchaka
    Copy link
    Member

    If PyObject_New(ScandirIterator, &ScandirIteratorType) fails the path should be cleaned up by Argument Clinic.

    @serhiy-storchaka serhiy-storchaka self-assigned this Feb 9, 2017
    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 9, 2017

    scandir-refleak-3.6.patch LGTM. The following removed Py_CLEAR() is just redundant, so it's ok to remove it.

    • Py_CLEAR(iterator->path.object);

    @zhangyangyu
    Copy link
    Member

    If PyObject_New(ScandirIterator, &ScandirIteratorType) fails the path should be cleaned up by Argument Clinic.

    Ohh yes. My stupid. Then both LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 9, 2017

    New changeset 4e3a16bdadae by Serhiy Storchaka in branch '3.6':
    Issue bpo-29513: Fixed a reference leak in os.scandir() added in issue bpo-29034.
    https://hg.python.org/cpython/rev/4e3a16bdadae

    New changeset a3f8c5d172b4 by Serhiy Storchaka in branch 'default':
    Issue bpo-29513: Fix outdated comment and remove redundand code is os.scandir().
    https://hg.python.org/cpython/rev/a3f8c5d172b4

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 9, 2017

    New changeset be85fd4ef41979dbe68262938da12328fb6cfb8c by Serhiy Storchaka in branch '3.6':
    Issue bpo-29513: Fixed a reference leak in os.scandir() added in issue bpo-29034.
    be85fd4

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 9, 2017

    Thanks for the quick fix Serhiy! I was working on patch and I like to check
    for refleaks. Sadly, it wasn't noticed before. At least my test was pass :-)

    @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
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.7 (EOL) end of life performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @serhiy-storchaka @zhangyangyu