On Oct 15, 2013, at 3:50 PM, Steve Mills <smi...@makemusic.com> wrote:
> We have extended NSMenu so we could add some other methods. Many of the 
> methods iterate over the itemArray like so:
> 
>       for(NSMenuItem* item in [self itemArray])
> 
> Instruments shows that we're leaking NSArrays created when itemArray returns.

I would submit a bug report to Apple that itemArray leaks its return value.  
Should be easy to demonstrate with a tiny example app.  I'm curious to verify 
the leak myself, but laziness is currently winning over curiosity. :)  Not that 
I don't trust you, I just like to see for myself.

> I assume whoever wrote these methods was assuming that itemArray would simply 
> return the internal NSArray and not make a copy. I guess I would make the 
> same assumption since it's not a copy* method.

No assumption should be made about the implementation of itemArray, including 
whether it returns an internal value or makes a copy.  When you're using manual 
retain/release, the memory management rules only say that you do not own the 
returned value and should not release it.  At most you can say that itemArray 
is violating the rules by under-releasing the returned object.

> So my question is, what's the best way to write extension methods on an 
> existing Cocoa class like this?

This question isn't really about class extensions.  The issues apply to any 
code that calls itemArray, given that as far as you can tell, it leaks.

Out of curiosity, when you say you have "extended" NSMenu, do you mean you've 
subclassed it?  That is what I would infer from your mention of class 
extensions.  Is there some reason you don't add a category instead?  I wonder 
because you only mentioned adding methods, not overriding them or adding ivars.

> One way would be to assign the itemArray result to a local variable and 
> release it when done (ARC is not turned on in this project yet):
> 
>       NSArray*                items = [self itemArray];
>       
>       for(NSMenuItem* item in items)
>               blah;
>       
>       [items release];

Even if this worked (you mention that it didn't), it's not good to fix a memory 
management problem by breaking the rules.  Besides trying to make two wrongs 
into a right, what happens when Apple fixes itemArray?  Then you'd be 
over-releasing.

> Thoughts?

Assuming itemArray is definitely leaking, you're stuck choosing a workaround.  
One possibility is to go through different methods that (hopefully) don't leak:

NSInteger numItems = [self numberOfItems];
for (NSInteger itemIndex = 0; itemIndex < numItems; itemIndex++)
{
        NSMenuItem *item = [self itemAtIndex:itemIndex];
        // blah
}

You could also use the above logic to implement a non-leaking version of 
itemArray in a category or class extension:

- (NSArray *)nonLeakingItemArray
{
        NSMutableArray *itemArray = [NSMutableArray array];
        NSInteger numItems = [self numberOfItems];

        for (NSInteger itemIndex = 0; itemIndex < numItems; itemIndex++)
        {
                [itemArray addObject:[self itemAtIndex:itemIndex]];
        }

        return itemArray;
}

Then instead of [self itemArray] you could use [self nonLeakingItemArray].  I 
would guess the cost of constructing the array yourself is totally negligible, 
but only you can know for sure by trying it.

--Andy


_______________________________________________

Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)

Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com

Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com

This email sent to arch...@mail-archive.com

Reply via email to