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

Memory leak in curses.panel #62313

Closed
atsuoishimoto mannequin opened this issue Jun 1, 2013 · 11 comments
Closed

Memory leak in curses.panel #62313

atsuoishimoto mannequin opened this issue Jun 1, 2013 · 11 comments
Assignees
@akuchling
Labels
easy extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@atsuoishimoto
Copy link
Mannequin

atsuoishimoto mannequin commented Jun 1, 2013

BPO 18113
Nosy @akuchling, @atsuoishimoto, @ezio-melotti, @serhiy-storchaka
Files
  • userptr-leak.py
  • userptr-segfault.py
  • userptr_segfault.patch
  • userptr_segfault_revised.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/akuchling'
closed_at = <Date 2013-06-25.01:35:14.943>
created_at = <Date 2013-06-01.02:35:55.553>
labels = ['extension-modules', 'easy', 'type-bug']
title = 'Memory leak in curses.panel'
updated_at = <Date 2013-06-25.01:35:14.941>
user = 'https://github.com/atsuoishimoto'

    bugs.python.org fields:

    activity = <Date 2013-06-25.01:35:14.941>
actor = 'akuchling'
assignee = 'akuchling'
closed = True
closed_date = <Date 2013-06-25.01:35:14.943>
closer = 'akuchling'
components = ['Extension Modules']
creation = <Date 2013-06-01.02:35:55.553>
creator = 'ishimoto'
dependencies = []
files = ['30438', '30606', '30607', '30614']
hgrepos = []
issue_num = 18113
keywords = ['patch', 'easy']
message_count = 11.0
messages = ['190433', '191223', '191225', '191228', '191268', '191269', '191297', '191335', '191646', '191655', '191829']
nosy_count = 5.0
nosy_names = ['akuchling', 'ishimoto', 'ezio.melotti', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18113'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']
    @atsuoishimoto
    Copy link
    Mannequin Author

    atsuoishimoto mannequin commented Jun 1, 2013

    Objects associated to the panel with panel.set_userptr() are never DECREF()ed. Attached file is script to reproduce the leak.

    Confirmed with Python2.7/3.3.

    @serhiy-storchaka serhiy-storchaka added easy extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Jun 1, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 15, 2013

    New changeset aff0bdab358e by Andrew Kuchling in branch '2.7':
    bpo-18113: Objects associated to a curses.panel object with set_userptr() were leaked.
    http://hg.python.org/cpython/rev/aff0bdab358e

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 15, 2013

    New changeset 3d75bd69e5a9 by Andrew Kuchling in branch '3.3':
    bpo-18113: Objects associated to a curses.panel object with set_userptr() were leaked.
    http://hg.python.org/cpython/rev/3d75bd69e5a9

    @akuchling
    Copy link
    Member

    Thanks for the bug report! I've committed a fix to the 2.7 and 3.3 branches that retrieves the existing userptr and Py_DECREFs it if it's not null. This seems to fix the leak.

    @akuchling akuchling self-assigned this Jun 15, 2013
    @serhiy-storchaka
    Copy link
    Member

    There is a problem with this patch. Py_XDECREF can execute arbitrary Python code and this code can call set_panel_userptr. Here is a reproducer (it causes segfault).

    @serhiy-storchaka
    Copy link
    Member

    And here is a patch which fixes a segfault. But I can't write a test for it.

    @akuchling
    Copy link
    Member

    serhiy.storchaka: good point! I wonder if, for strict correctness, we should only incref obj (the new object) if set_panel_userptr() returns OK and not an error code. I've attached a new version of the patch that does this check, and also adds a test.

    (OTOH, looking at the ncurses 5.9 source code, set_panel_userptr() only returns an error if the panel object is NULL, which should never happen because Python reports an error if the panel creation fails. So maybe the rc == ERR check is pointless.)

    @serhiy-storchaka
    Copy link
    Member

    I've attached a new version of the patch that does this check, and also adds a test.

    You are right. Your patch LGTM.

    (OTOH, looking at the ncurses 5.9 source code, set_panel_userptr() only returns an error if the panel object is NULL, which should never happen because Python reports an error if the panel creation fails. So maybe the rc == ERR check is pointless.)

    Future versions or alternative implementations can returns an error in other circumstances.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 22, 2013

    New changeset 99733ff98a50 by Andrew Kuchling in branch '2.7':
    bpo-18113: avoid segfault if Py_XDECREF triggers code that calls set_panel_userptr again
    http://hg.python.org/cpython/rev/99733ff98a50

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 22, 2013

    New changeset 61fafef4c8a2 by Andrew Kuchling in branch '3.3':
    bpo-18113: avoid segfault if Py_XDECREF triggers code that calls set_panel_userptr again
    http://hg.python.org/cpython/rev/61fafef4c8a2

    @akuchling
    Copy link
    Member

    I believe the most recent 2 commits fix the segfault problem, so I'll now close this again. Please re-open if there are further issues with the bugfix.

    @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

    @akuchling akuchling

    Labels
    easy extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @akuchling @serhiy-storchaka