classification
Title: Memory leak in curses.panel
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: akuchling Nosy List: akuchling, ezio.melotti, ishimoto, python-dev, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2013-06-01 02:35 by ishimoto, last changed 2013-06-25 01:35 by akuchling. This issue is now closed.

Files
File name Uploaded Description Edit
userptr-leak.py ishimoto, 2013-06-01 02:35
userptr-segfault.py serhiy.storchaka, 2013-06-16 15:05
userptr_segfault.patch serhiy.storchaka, 2013-06-16 15:07 review
userptr_segfault_revised.patch akuchling, 2013-06-16 23:14
Messages (11)
msg190433 - (view) Author: Atsuo Ishimoto (ishimoto) * Date: 2013-06-01 02:35
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.
msg191223 - (view) Author: Roundup Robot (python-dev) Date: 2013-06-15 18:48
New changeset aff0bdab358e by Andrew Kuchling in branch '2.7':
#18113: Objects associated to a curses.panel object with set_userptr() were leaked.
http://hg.python.org/cpython/rev/aff0bdab358e
msg191225 - (view) Author: Roundup Robot (python-dev) Date: 2013-06-15 19:03
New changeset 3d75bd69e5a9 by Andrew Kuchling in branch '3.3':
#18113: Objects associated to a curses.panel object with set_userptr() were leaked.
http://hg.python.org/cpython/rev/3d75bd69e5a9
msg191228 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2013-06-15 19:34
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.
msg191268 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-16 15:05
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).
msg191269 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-16 15:07
And here is a patch which fixes a segfault. But I can't write a test for it.
msg191297 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2013-06-16 23:14
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.)
msg191335 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-17 14:06
> 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.
msg191646 - (view) Author: Roundup Robot (python-dev) Date: 2013-06-22 16:33
New changeset 99733ff98a50 by Andrew Kuchling in branch '2.7':
#18113: avoid segfault if Py_XDECREF triggers code that calls set_panel_userptr again
http://hg.python.org/cpython/rev/99733ff98a50
msg191655 - (view) Author: Roundup Robot (python-dev) Date: 2013-06-22 18:51
New changeset 61fafef4c8a2 by Andrew Kuchling in branch '3.3':
#18113: avoid segfault if Py_XDECREF triggers code that calls set_panel_userptr again
http://hg.python.org/cpython/rev/61fafef4c8a2
msg191829 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2013-06-25 01:35
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.
History
Date User Action Args
2013-06-25 01:35:14akuchlingsetstatus: open -> closed
resolution: fixed
messages: + msg191829

stage: commit review -> resolved
2013-06-22 18:51:15python-devsetmessages: + msg191655
2013-06-22 16:33:20python-devsetmessages: + msg191646
2013-06-17 14:06:27serhiy.storchakasetmessages: + msg191335
2013-06-16 23:14:16akuchlingsetfiles: + userptr_segfault_revised.patch

messages: + msg191297
2013-06-16 15:07:25serhiy.storchakasetfiles: + userptr_segfault.patch
keywords: + patch
messages: + msg191269
2013-06-16 15:05:25serhiy.storchakasetstatus: closed -> open
files: + userptr-segfault.py
messages: + msg191268

resolution: fixed -> (no value)
stage: resolved -> commit review
2013-06-15 19:34:43akuchlingsetstage: needs patch -> resolved
2013-06-15 19:34:31akuchlingsetstatus: open -> closed
assignee: akuchling
resolution: fixed
messages: + msg191228
2013-06-15 19:03:31python-devsetmessages: + msg191225
2013-06-15 18:48:15python-devsetnosy: + python-dev
messages: + msg191223
2013-06-15 14:42:07akuchlingsetnosy: + akuchling
2013-06-08 17:41:57ezio.melottisetnosy: + ezio.melotti
2013-06-01 08:40:14serhiy.storchakasetnosy: + serhiy.storchaka
versions: + Python 3.4
components: + Extension Modules
keywords: + easy
type: behavior
stage: needs patch
2013-06-01 02:35:55ishimotocreate