Re: [pygame] memory leaks with Py_BuildValue

["Nirav Patel" <olpc@xxxxxxxxxxxxxx>]

Oops, forgot about this.  I just committed Campbell Barton's patch.
Anyone see any other occurances of PyObjects not getting DECREFed?  I
found a few in my camera module (which I will commit to the SVN soon).

Nirav

On Fri, Sep 5, 2008 at 10:33 AM, Nirav Patel <olpc@xxxxxxxxxxxxxx> wrote:
> 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[] =
>>>>>>
>>>>>
>>>>
>>>
>>
>