[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix memory leak in SWIG Python bindings

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Tue, 24 Jan 2012 07:11:35 +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

Is svn_swig_py_c_strings_to_list() affected too? It appends a string to
a list but doesn't Py_DECREF() the string.

> 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.
>
> Patch by: Trent Nelson <trent_at_snakebite.org>
> Tested on: FreeBSD, OS X, Windows.
>
> * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:
> (convert_rangelist): Make sure we call Py_DECREF on the object returned
> from convert_to_swigtype. PyList_New might return NULL; check for that.
> Add 'apr_array_header_t *' and 'swig_type_info *' casts where necessary
> to suppress compiler warnings.

Those casts shouldn't be needed -- casts from 'void *' to other
pointer types are valid C. What warnings were you seeing?

> 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;
>
> 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:
Received on 2012-01-24 06:13:00 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.