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.

Sign up for GitHub

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

C Unpickler memory leak via memo #85594

Closed
kale-smoothie mannequin opened this issue Jul 28, 2020 · 3 comments
Closed

C Unpickler memory leak via memo #85594

kale-smoothie mannequin opened this issue Jul 28, 2020 · 3 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@kale-smoothie
Copy link
Mannequin

kale-smoothie mannequin commented Jul 28, 2020

BPO 41422
Nosy @serhiy-storchaka, @kale-smoothie
PRs
  • bpo-41422: Visit the Unpickler's memo in Unpickler_traverse #21664
    		•
    Files
  • leak_pickler.py
    		•

    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 = None
created_at = <Date 2020-07-28.18:26:29.426>
labels = ['3.8', '3.9', '3.10', 'performance', '3.7', 'library']
title = 'C Unpickler memory leak via memo'
updated_at = <Date 2020-07-28.19:23:14.683>
user = 'https://github.com/kale-smoothie'

    bugs.python.org fields:

    activity = <Date 2020-07-28.19:23:14.683>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-07-28.18:26:29.426>
creator = 'kale-smoothie'
dependencies = []
files = ['49345']
hgrepos = []
issue_num = 41422
keywords = ['patch']
message_count = 2.0
messages = ['374518', '374521']
nosy_count = 3.0
nosy_names = ['python-dev', 'serhiy.storchaka', 'kale-smoothie']
pr_nums = ['21664']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue41422'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']
    @kale-smoothie
    Copy link
    Mannequin Author

    kale-smoothie mannequin commented Jul 28, 2020

    I'm not familiar with the workings of GC/pickle, but it looks like the traverse code in the C Unpickler omits a visit to the memo, potentially causing a memory leak?

    @kale-smoothie kale-smoothie mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir performance Performance or resource usage labels Jul 28, 2020
    @kale-smoothie
    Copy link
    Mannequin Author

    kale-smoothie mannequin commented Jul 28, 2020

    The leak demonstrated in the attachment is, to my understanding, caused by memoizing the closure returned from the find_class method that's used to intercept global references. The cycle is then: Unpickler, memo table, closure, Unpickler (via cell reference to self).

    My proposed patch visits every entry in the memo table.

    Pre-patch run of valgrind on leak_pickler.py:

    ==20339== HEAP SUMMARY:
    ==20339== in use at exit: 190,189,238 bytes in 2,406,919 blocks
    ==20339== total heap usage: 3,150,288 allocs, 743,369 frees, 233,766,596 bytes allocated
    ==20339==
    ==20339== LEAK SUMMARY:
    ==20339== definitely lost: 0 bytes in 0 blocks
    ==20339== indirectly lost: 0 bytes in 0 blocks
    ==20339== possibly lost: 190,176,150 bytes in 2,406,835 blocks
    ==20339== still reachable: 13,088 bytes in 84 blocks
    ==20339== suppressed: 0 bytes in 0 blocks
    ==20339== Rerun with --leak-check=full to see details of leaked memory

    Post-patch run of valgrind on leak_pickler.py:

    ==20880== HEAP SUMMARY:
    ==20880== in use at exit: 667,277 bytes in 6,725 blocks
    ==20880== total heap usage: 2,853,739 allocs, 2,847,014 frees, 216,473,216 bytes allocated
    ==20880==
    ==20880== LEAK SUMMARY:
    ==20880== definitely lost: 0 bytes in 0 blocks
    ==20880== indirectly lost: 0 bytes in 0 blocks
    ==20880== possibly lost: 654,624 bytes in 6,646 blocks
    ==20880== still reachable: 12,653 bytes in 79 blocks
    ==20880== suppressed: 0 bytes in 0 blocks
    ==20880== Rerun with --leak-check=full to see details of leaked memory

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error and removed performance Performance or resource usage labels Aug 17, 2022
    @hugovk hugovk added 3.11 only security fixes 3.12 bugs and security fixes and removed 3.10 only security fixes 3.9 only security fixes 3.8 only security fixes 3.7 (EOL) end of life labels Apr 7, 2023
    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution, @kale-smoothie.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    Pickle and copy issues 🥒
    Status: Done
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @iritkatriel @hugovk @serhiy-storchaka