On Tue, Jul 15, 2008 at 9:28 AM, Michael Abbott <[EMAIL PROTECTED]> 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([EMAIL PROTECTED]@); > > 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
_______________________________________________ Numpy-discussion mailing list Numpy-discussion@scipy.org http://projects.scipy.org/mailman/listinfo/numpy-discussion