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

refleak in C functools.lru_cache #72839

Closed
1st1 opened this issue Nov 9, 2016 · 12 comments
Closed

refleak in C functools.lru_cache #72839

1st1 opened this issue Nov 9, 2016 · 12 comments
Assignees
Labels
3.7 only security fixes performance Performance or resource usage release-blocker stdlib Python modules in the Lib dir

Comments

@1st1
Copy link
Member

1st1 commented Nov 9, 2016

BPO 28653
Nosy @gvanrossum, @larryhastings, @ned-deily, @serhiy-storchaka, @1st1, @ilevkivskyi
Files
  • refleak.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/1st1'
    closed_at = <Date 2016-11-10.01:11:57.284>
    created_at = <Date 2016-11-09.23:47:28.829>
    labels = ['3.7', 'library', 'release-blocker', 'performance']
    title = 'refleak in C functools.lru_cache'
    updated_at = <Date 2016-11-10.01:11:57.277>
    user = 'https://github.com/1st1'

    bugs.python.org fields:

    activity = <Date 2016-11-10.01:11:57.277>
    actor = 'yselivanov'
    assignee = 'yselivanov'
    closed = True
    closed_date = <Date 2016-11-10.01:11:57.284>
    closer = 'yselivanov'
    components = ['Library (Lib)']
    creation = <Date 2016-11-09.23:47:28.829>
    creator = 'yselivanov'
    dependencies = []
    files = ['45417']
    hgrepos = []
    issue_num = 28653
    keywords = ['patch']
    message_count = 12.0
    messages = ['280468', '280470', '280471', '280472', '280473', '280474', '280475', '280476', '280477', '280478', '280479', '280480']
    nosy_count = 7.0
    nosy_names = ['gvanrossum', 'larry', 'ned.deily', 'python-dev', 'serhiy.storchaka', 'yselivanov', 'levkivskyi']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue28653'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 9, 2016

    The attached patch fixes a refleak in functools.lru_cache.

    @1st1 1st1 added 3.7 only security fixes release-blocker stdlib Python modules in the Lib dir performance Performance or resource usage labels Nov 9, 2016
    @serhiy-storchaka
    Copy link
    Member

    LGTM.

    @ilevkivskyi
    Copy link
    Member

    FWIW, if I comment out this code

    try:
    from _functools import _lru_cache_wrapper
    except ImportError:
    pass

    in functools.py to use the pure Python version, then I get much larger numbers:

    $ ./python -m test -R : test_typing
    Run tests sequentially
    0:00:00 [1/1] test_typing
    beginning 9 repetitions
    123456789
    .........
    test_typing leaked [25003, 25003, 25003, 25003] references, sum=100012
    test_typing leaked [9350, 9352, 9352, 9352] memory blocks, sum=37406
    test_typing failed

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 9, 2016

    New changeset ba59f3328032 by Yury Selivanov in branch '3.5':
    Issue bpo-28653: Fix a refleak in functools.lru_cache.
    https://hg.python.org/cpython/rev/ba59f3328032

    New changeset 5b253d641826 by Yury Selivanov in branch '3.6':
    Merge 3.6 (issue bpo-28653)
    https://hg.python.org/cpython/rev/5b253d641826

    New changeset 784fea019cab by Yury Selivanov in branch 'default':
    Merge 3.6 (issue bpo-28653)
    https://hg.python.org/cpython/rev/784fea019cab

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 10, 2016

    FWIW, if I comment out this code

    Oh boy... This is something else. Please re-open bpo-28649 explaining that it now leaks with pure-Python lru_cache. Could you please take a look at it?

    @ilevkivskyi
    Copy link
    Member

    It seems to be unrelated to typing.py
    With your test from test_functools:

        def test_lru_type_error(self):
            @functools.lru_cache(maxsize=None)
            def infinite_cache(o):
                pass
            with self.assertRaises(TypeError):
                infinite_cache([])

    I get [2, 2, 2, 2] for unpatched C version (this is now fixed)
    and [24764, 24764, 24764, 24764] for Python version.

    Should I maybe open a separate issue for this?

    @ilevkivskyi
    Copy link
    Member

    Anyway, this is not urgent, typing.py uses the default version, which is the C version.

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 10, 2016

    It seems to be unrelated to typing.py

    Wow. I can reproduce refleaks in test_typing, but not in test_functools. This is weird.

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 10, 2016

    Anyway, this is not urgent

    Not so sure, a refleak in pure-python version kind of scares me. It can be a bug in the core.

    @ilevkivskyi
    Copy link
    Member

    Wow. I can reproduce refleaks in test_typing, but not in test_functools. This is weird.

    Indeed, I copied the new test from test_functools to test_typing, but now I see that pure Python version of lru_cache fails even if I remove this test and test that was previously failing with C version (test_extended_generic_rules_eq).

    Not so sure, a refleak in pure-python version kind of scares me. It can be a bug in the core.

    Indeed. Now we need to figure out what it so special about typing that triggers the leak.

    @ilevkivskyi
    Copy link
    Member

    I am not sure what does this mean, but when I remove __slots__ = () from some classes, numbers of leaked references change.

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 10, 2016

    Ivan, I think I figured it out, I'll close this one and reopen the typing one.

    @1st1 1st1 closed this as completed Nov 10, 2016
    @1st1 1st1 changed the title refleak in functools.lru_cache refleak in C functools.lru_cache Nov 10, 2016
    @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
    3.7 only security fixes performance Performance or resource usage release-blocker stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants