Analyzer issue: Potential leak of an object allocated on line 25 and stored into 'oneCopy'

Question

I've upgraded Xcode and have been presented with tons of analyzer warnings like this:

Potential leak of an object allocated on line 25 and stored into 'oneCopy'

Can anyone point me in the right direction?

@implementation NSDictionary(DeepMutableCopy)
-(NSMutableDictionary *)mutableDeepCopy
{
    NSMutableDictionary *ret = [[NSMutableDictionary alloc] initWithCapacity:[self count]];
    NSArray *keys = [self allKeys];
    for (id key in keys)
    {
        id oneValue = [self valueForKey:key];
        id oneCopy = nil;

        if ([oneValue respondsToSelector:@selector(mutableDeepCopy)])
            oneCopy = [oneValue mutableDeepCopy];
        else if ([oneValue respondsToSelector:@selector(mutableCopy)])
            oneCopy = [oneValue mutableCopy];
        if (oneCopy == nil)
            oneCopy = [oneValue copy];
        [ret setValue:oneCopy forKey:key];
    }
    return ret;
}
@end

Screenshot showing lines numbers:
enter image description here

#import "NSDictionary-DeepMutableCopy.h"


@implementation NSDictionary(DeepMutableCopy)
-(NSMutableDictionary *)mutableDeepCopy
{
    //NSMutableDictionary *ret = [[NSMutableDictionary alloc] initWithCapacity:[self count]];
    NSMutableDictionary *ret = [NSMutableDictionary dictionaryWithCapacity:[self count]];
    NSArray *keys = [self allKeys];
    for (id key in keys)
    {
        id oneValue = [self valueForKey:key];
        id oneCopy = nil;

        if ([oneValue respondsToSelector:@selector(mutableDeepCopy)])
            oneCopy = [oneValue mutableDeepCopy];
        else if ([oneValue respondsToSelector:@selector(mutableCopy)])
            oneCopy = [oneValue mutableCopy];
        if (oneCopy == nil)
            oneCopy = [oneValue copy];
        [ret setValue:oneCopy forKey:key];
        [oneCopy release];
    }
    return ret;

}
@end

Can you tell us which line is line 25? – Alexsander Akers Nov 09 '11 at 22:30 — Alexsander Akers, Nov 09 '11 at 22:30
See edit, I'm also getting a further error. – Jules Nov 10 '11 at 19:46 — Jules, Nov 10 '11 at 19:46

score 1 · Accepted Answer · edited May 23 '17 at 12:34

Addressing your second issue first, instead of [[NSMutableDictionary alloc] initWithCapacity:[self count]] you could use [NSMutableDictionary dictionaryWithCapacity:[self count]] which will return an autoreleased object and you will have to retain it yourself in the calling code.

On the other hand, you could rename your method to start with the word copy if you want it to return a retained object and not throw errors - which I think is exactly what you want to do in this case. The rest of my response assumes you have taken this path.

My original answer was as follows: You are not releasing oneCopy at the end of each iteration. Try adding [oneCopy release]; right after [ret setValue:oneCopy forKey:key];.

However, as Alexsander Akers points out the compiler thinks that -mutableDeepCopy has a 0 refcount. So, if you rename as suggested above and include [oneCopy release] as I originally suggested it should take care of both issues. If it doesn't, definitely check out some of the other solutions in the question he referenced.

Example:

@implementation NSDictionary(DeepMutableCopy)
-(NSMutableDictionary *)copyWithDeepCopiedValues
{
    NSMutableDictionary *ret = [[NSMutableDictionary alloc] initWithCapacity:[self count]];

    NSArray *keys = [self allKeys];
    for (id key in keys)
    {
        id oneValue = [self valueForKey:key];
        id oneCopy = nil;

        if ([oneValue respondsToSelector:@selector(mutableDeepCopy)])
            oneCopy = [oneValue copyWithDeepCopiedValues];
        else if ([oneValue respondsToSelector:@selector(mutableCopy)])
            oneCopy = [oneValue mutableCopy];
        if (oneCopy == nil)
            oneCopy = [oneValue copy];
        [ret setObject:oneCopy forKey:key];

        [oneCopy release];
    }

    return ret;
}
@end

It didn't like that... I get a `Incorrect decrement of the reference count of an object that is not owned at this point by the caller` on that new line now.. any futher ideas ? — Jules, Nov 10 '11 at 19:47
@Jules I updated my answer to reflect the additional information you provided. — David Brainer, Nov 10 '11 at 20:12
@Dave, i'm still getting a `Incorrect decrement of the reference count of an object that is not owned at this point by the caller` See edit above — Jules, Nov 10 '11 at 20:27
@Jules I apologize, I might have been confusing by starting off with a mention of the autorelease method. Because in your case you (presumably) do want to return a retained object the second part of my answer assumes that you continue to retain the object and simply change your method name. — David Brainer, Nov 10 '11 at 20:40

score 0 · Answer 2 · edited May 23 '17 at 12:17

There are two problems here. First, as @David Brainer-Banker says, you need to release oneCopy at the end of each iteration by placing [oneCopy release]; after you set[ret setValue:oneCopy forKey:key];`.

The incorrect reference count decrement is the second problem. This is because the oneCopy object may have a +1 or a 0 reference count. The objects returned by -copy and -mutableCopy have a +1 refcount, but the object returned by -deepMutableCopy has a 0 refcount because it isn't in the new, copy, or create (et al.) families.

This question is an exact duplicate of this one and has some great responses.

Analyzer issue: Potential leak of an object allocated on line 25 and stored into 'oneCopy'

2 Answers2