[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [pygame] memory leaks with Py_BuildValue
I submitted the mask stuff because it patched code I wrote. I've
never looked at the mouse module, but if someone familiar with it
doesn't submit the patch within a day or two, I'll do it.
Nirav
On Fri, Sep 5, 2008 at 9:20 AM, Campbell Barton <ideasman42@xxxxxxxxx> wrote:
> Thanks for submitting, Is there anything wrong with the mouse patch I
> submitted earlier? - from my testing it definitely leaks and the patch
> fixes it.
>
> On Fri, Sep 5, 2008 at 3:29 PM, Nirav Patel <olpc@xxxxxxxxxxxxxx> wrote:
>> Thanks! Patch committed.
>>
>> Nirav
>>
>> On Wed, Sep 3, 2008 at 7:10 PM, Campbell Barton <ideasman42@xxxxxxxxx> wrote:
>>> Heres more fixes to mask.c - Py_BuildValue returns a variable with 1
>>> ref and PyList_Append adds another. checked all other cases of
>>> PyList_Append and they all seem correct.
>>>
>>> Index: src/mask.c
>>> ===================================================================
>>> --- src/mask.c (revision 1646)
>>> +++ src/mask.c (working copy)
>>> @@ -312,7 +312,7 @@
>>> {
>>> bitmask_t* c = PyMask_AsBitmap(self);
>>> bitmask_t* m = bitmask_create(c->w + 2, c->h + 2);
>>> - PyObject* plist;
>>> + PyObject* plist, *value;
>>> int x, y, every, e, firstx, firsty, secx, secy, currx, curry,
>>> nextx, nexty, n;
>>> int a[14], b[14];
>>> a[0] = a[1] = a[7] = a[8] = a[9] = b[1] = b[2] = b[3] = b[9] =
>>> b[10] = b[11]= 1;
>>> @@ -343,7 +343,9 @@
>>> if (bitmask_getbit(m, x, y)) {
>>> firstx = x;
>>> firsty = y;
>>> - PyList_Append(plist, Py_BuildValue("(ii)", x-1, y-1));
>>> + value = Py_BuildValue("(ii)", x-1, y-1);
>>> + PyList_Append(plist, value);
>>> + Py_DECREF(value);
>>> break;
>>> }
>>> }
>>> @@ -367,7 +369,9 @@
>>> e--;
>>> if (!e) {
>>> e = every;
>>> - PyList_Append(plist, Py_BuildValue("(ii)", secx-1, secy-1));
>>> + value = Py_BuildValue("(ii)", secx-1, secy-1);
>>> + PyList_Append(plist, value);
>>> + Py_DECREF(value);
>>> }
>>> break;
>>> }
>>> @@ -392,7 +396,9 @@
>>> if ((curry == firsty && currx == firstx) && (secx
>>> == nextx && secy == nexty)) {
>>> break;
>>> }
>>> - PyList_Append(plist, Py_BuildValue("(ii)",
>>> nextx-1, nexty-1));
>>> + value = Py_BuildValue("(ii)", nextx-1, nexty-1);
>>> + PyList_Append(plist, value);
>>> + Py_DECREF(value);
>>> }
>>> break;
>>> }
>>> @@ -1170,6 +1176,7 @@
>>> if(maskobj) {
>>> maskobj->mask = components[i];
>>> PyList_Append (ret, (PyObject *) maskobj);
>>> + Py_DECREF((PyObject *) maskobj);
>>> }
>>> }
>>>
>>>
>>>
>>>
>>> On Thu, Sep 4, 2008 at 1:51 AM, Nirav Patel <olpc@xxxxxxxxxxxxxx> wrote:
>>>> Thanks,
>>>>
>>>> I just committed a fix for it in mask.c.
>>>>
>>>> Nirav
>>>>
>>>> On Wed, Sep 3, 2008 at 11:13 AM, Campbell Barton <ideasman42@xxxxxxxxx> wrote:
>>>>> Hi, over a year ago I had a look at pygames py-c/api usage for memory
>>>>> leaks, and found a few, mainly with Py_BuildValue() using PyObjects
>>>>> which are incref'd,
>>>>> had another look at the recent SVN and found some more, not sure if
>>>>> they are new but they definitely cause memory leaks.
>>>>>
>>>>> For example - call pygame.mouse.get_cursor() in a loop of 10000 for
>>>>> every update in chimp.py and memory will go up 100's of meg fairly
>>>>> fast, patch below.
>>>>>
>>>>> you also might want to consider using PyTuple_Pack() which is faster
>>>>> but only in python 2.4 or later.
>>>>> http://docs.python.org/api/tupleObjects.html#l2h-583
>>>>>
>>>>>
>>>>> These leak too, much the same changes need to be made here also.
>>>>> ./src/mask.c:265: return Py_BuildValue ("(OO)", PyInt_FromLong(m10/m00),
>>>>> ./src/mask.c:268: return Py_BuildValue ("(OO)",
>>>>> PyInt_FromLong(0), PyInt_FromLong(0));
>>>>> ./src/mask.c:298: return Py_BuildValue ("O", PyFloat_FromDouble(theta));
>>>>> ./src/mask.c:300: return Py_BuildValue ("O", PyFloat_FromDouble(0));
>>>>>
>>>>>
>>>>> Index: src/mouse.c
>>>>> ===================================================================
>>>>> --- src/mouse.c (revision 1645)
>>>>> +++ src/mouse.c (working copy)
>>>>> @@ -171,7 +171,7 @@
>>>>> mouse_get_cursor (PyObject* self)
>>>>> {
>>>>> SDL_Cursor *cursor = NULL;
>>>>> - PyObject* xordata, *anddata;
>>>>> + PyObject* xordata, *anddata, *ret;
>>>>> int size, loop, w, h, spotx, spoty;
>>>>>
>>>>> VIDEO_INIT_CHECK ();
>>>>> @@ -201,8 +201,10 @@
>>>>> PyTuple_SET_ITEM (xordata, loop, PyInt_FromLong (cursor->data[loop]));
>>>>> PyTuple_SET_ITEM (anddata, loop, PyInt_FromLong (cursor->mask[loop]));
>>>>> }
>>>>> -
>>>>> - return Py_BuildValue ("((ii)(ii)OO)", w, h, spotx, spoty,
>>>>> xordata, anddata);
>>>>> + ret = Py_BuildValue ("((ii)(ii)OO)", w, h, spotx, spoty, xordata, anddata);
>>>>> + Py_DECREF (xordata);
>>>>> + Py_DECREF (anddata);
>>>>> + return ret;
>>>>> }
>>>>>
>>>>> static PyMethodDef mouse_builtins[] =
>>>>>
>>>>
>>>
>>
>