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);
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.
I still haven't convinced myself of this. By the time we hit finish, robj is NULL or holds a reference to typecode and the NULL case is taken care of up front. Later on, the reference to typecode might be decremented, perhaps leaving robj crippled, but in that case robj itself is marked for deletion upon exit. If the garbage collector can handle zero reference counts I think we are alright. I admit I haven't quite followed all the subroutines and macros, which descend into the hazy depths without the slightest bit of documentation, but at this point I'm inclined to leave things alone unless you have a test that shows a leak from this source.
Chuck