[Numpy-discussion] Ticket review: #848, leak in PyArray_DescrFromType

Charles R Harris charlesr.harris@gmail....
Thu Jul 17 16:13:50 CDT 2008


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.
> __
>

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

> _____________________________________________
> Numpy-discussion mailing list
> Numpy-discussion@scipy.org
> http://projects.scipy.org/mailman/listinfo/numpy-discussion
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://projects.scipy.org/pipermail/numpy-discussion/attachments/20080717/f78e0263/attachment.html 


More information about the Numpy-discussion mailing list