On Tue, Jul 15, 2008 at 9:28 AM, Michael Abbott <
michael@araneidae.co.uk> wrote:
On Tue, 15 Jul 2008, Michael Abbott wrote:
> Only half of my patch for this bug has gone into trunk, and without the
> rest of my patch there remains a leak.
I think I might need to explain a little more about the reason for this
patch, because obviously the bug it fixes was missed the last time I
posted on this bug.
So here is the missing part of the patch:
> --- numpy/core/src/scalartypes.inc.src (revision 5411)
> +++ numpy/core/src/scalartypes.inc.src (working copy)
> @@ -1925,19 +1925,30 @@
> goto finish;
> }
>
> + Py_XINCREF(typecode);
> arr = PyArray_FromAny(obj, typecode, 0, 0, FORCECAST, NULL);
> - if ((arr==NULL) || (PyArray_NDIM(arr) > 0)) return arr;
> + if ((arr==NULL) || (PyArray_NDIM(arr) > 0)) {
> + Py_XDECREF(typecode);
> + return arr;
> + }
> robj = PyArray_Return((PyArrayObject *)arr);
>
> finish:
> - if ((robj==NULL) || (robj->ob_type == type)) return robj;
> + if ((robj==NULL) || (robj->ob_type == type)) {
> + Py_XDECREF(typecode);
> + return robj;
> + }
> /* Need to allocate new type and copy data-area over */
> if (type->tp_itemsize) {
> itemsize = PyString_GET_SIZE(robj);
> }
> else itemsize = 0;
> obj = type->tp_alloc(type, itemsize);
> - if (obj == NULL) {Py_DECREF(robj); return NULL;}
> + if (obj == NULL) {
> + Py_XDECREF(typecode);
> + Py_DECREF(robj);
> + return NULL;
> + }
> if (typecode==NULL)
> typecode = PyArray_DescrFromType(PyArray_@TYPE@);
> dest = scalar_value(obj, typecode);
On the face of it it might appear that all the DECREFs are cancelling out
the first INCREF, but not so. Let's see two more lines of context:
> src = scalar_value(robj, typecode);
> Py_DECREF(typecode);
Ahah. That DECREF balances the original PyArray_DescrFromType, or maybe
the later call ... and of course this has to happen on *ALL* return paths.
If we now take a closer look at the patch we can see that it's doing two
separate things:
1. There's an extra Py_XINCREF to balance the ref count lost to
PyArray_FromAny and ensure that typecode survives long enough;
2. Every early return path has an extra Py_XDECREF to balance the creation
of typecode.
I rest my case for this patch.
Yes, there does look to be a memory leak here. Not to mention a missing NULL check since PyArray_Scalar not only doesn't swallow a reference, it can't take a Null value for desc. But the whole function is such a mess I want to see if we can rewrite it to have a better flow of logic <puts on todo list>
Chuck