On Sat, Jul 19, 2008 at 8:57 AM, Michael Abbott <michael@araneidae.co.uk> wrote:
On Fri, 18 Jul 2008, Travis E. Oliphant wrote:
It looks like with that added DECREF, the reference count leak is gone.
I've looked at the latest head, and I agree that the problem is now solved.
There is an important difference from my original solution: typecode is no longer reused after the finish label (instead it is always created anew). This makes all the difference in the world.
I'm not actually convinced by the comment that's there now, which says /* typecode will be NULL */ but in truth it doesn't matter -- because of the correcly placed DECREF after the PyArray_Scalar calls the routine no longer owns typecode.
If I can refer to my last message, I made the point that there wasn't a good invariant at the finish label -- we didn't know how many references to typecode we were responsible for at that point -- and I offered the solution to keep typecode. Instead you have chosen to recreate typecode, which I hadn't realised was just as good.
This code is still horrible, but I don't think I want to try to understand it anymore. It'd be really nice (it'd make me feel a lot better) if you'd agree that my original patch was in fact correct. I'm not disputing the correcness of the current solution (except I think that typecode can end up being created twice, but who really cares?) but I've put a lot of effort into arguing my case, and the fact is my original patch was not wrong.
Yep, the original patch looks good now.