Daniel Shahaf wrote on Tue, Jan 24, 2012 at 13:36:07 +0200: > Trent Nelson wrote on Mon, Jan 23, 2012 at 15:07:20 -0500: > > Fix a memory leak in convert_rangelist by *always* Py_DECREF'ing the object > > returned by convert_to_swigtype (a SWIG wrapper around svn_merge_range_t) > > once we've established it's not NULL. > > > > This is required because PyList_Append takes ownership of references passed > > in (i.e. calls Py_INCREF). Prior to this fix, the reference counts of any > > Nice log message. > > > svn_merge_range_t objects would always be off-by-one, preventing them from > > ever being garbage collected, having dire effects on the memory usage of > > long-running programs calling svn_mergeinfo_parse() on real-world data. > > > > Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c > > =================================================================== > > --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c > > (revision 1234786) > > +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c > > (working copy) > > @@ -653,15 +653,27 @@ > > static PyObject *convert_rangelist(void *value, void *ctx, PyObject > > *py_pool) > > { > > int i; > > + int result; > > + PyObject *obj; > > PyObject *list; > > - apr_array_header_t *array = value; > > + apr_array_header_t *array = (apr_array_header_t *)value; > > > > As before: this cast shouldn't need to be explicit. Also, I've moved > the two new declarations to a more inner scope. > > > list = PyList_New(0); > > + if (list == NULL) > > + return NULL; > > + > > for (i = 0; i < array->nelts; i++) > > { > > svn_merge_range_t *range = APR_ARRAY_IDX(array, i, svn_merge_range_t > > *); > > - if (PyList_Append(list, convert_to_swigtype(range, ctx, py_pool)) == > > -1) > > + > > + obj = convert_to_swigtype(range, (swig_type_info *)ctx, py_pool); > > + if (obj == NULL) > > goto error; > > + > > + result = PyList_Append(list, obj); > > + Py_DECREF(obj); > > + if (result == -1) > > + goto error; > > } > > return list; > > error: > > I then ended up with the following patch: > > + if (result == -1); > goto error;
My version of the patch contained a bug. When fixed, the memory usage in the pastebin test remained constant, so I went ahead and applied the C part of the patch in r1235264. I'll look at the py part next. Thanks, Daniel