Search code examples
objective-cmemory-leaksabaddressbook

Memoryleak when using CFRelease


I have the method which causes a memory leak as shown by the instrument:

enter image description here

-(BOOL)checkIfGroupExistWithName:(NSString*)groupName
{
    BOOL hasGroup = NO;
    //checks to see if the group is created ad creats group for Handheld contacts

    CFErrorRef error = NULL;
    ABAddressBookRef ab = ABAddressBookCreateWithOptions(NULL, &error);

    CFIndex groupCount = ABAddressBookGetGroupCount(ab);
    CFArrayRef allGroups = ABAddressBookCopyArrayOfAllGroups(ab);

    for (int i=0; i<groupCount; i++) {

        ABRecordRef group = CFArrayGetValueAtIndex(allGroups, i);

        CFStringRef  CFcurrentGroupName = ABRecordCopyCompositeName(group);
        NSString *currentGroupName = (__bridge_transfer NSString *)CFcurrentGroupName;

        if ([currentGroupName isEqualToString:groupName]) {
            //!!! important - save groupID for later use
            groupId = ABRecordGetRecordID(group);
            hasGroup = YES;
            i = (int) groupCount;
        }
        CFRelease(CFcurrentGroupName);
        CFRelease(group);
    }


    return hasGroup;
}

If I use CFRelease(ab); before return hasGroup, it crashes. I couldn't understand what is happening here.


Solution

  • The static analyzer (shift+command+B or "Analyze" on Xcode's "Product" menu) is remarkably good at identifying these issues for you:

    enter image description here

    Bottom line, the Create Rule dictates that you have to CFRelease any objects returned from Core Foundation functions with either Copy or Create in their name (except those that you transfer ownership with either CFBridgingRelease or __bridge_transfer, as that allows ARC to clean those up for you).

    Needless to say, you shouldn't CFRelease anything else (such as group, which was returned by an function without Copy or Create in the name, nor CFcurrentGroupName, which you already transferred ownership with __bridge_transfer).

    Anyway, you end up with something like:

    - (BOOL)checkIfGroupExistWithName:(NSString*)groupName {
        BOOL hasGroup = NO;
        //checks to see if the group is created ad creats group for Handheld contacts
    
        CFErrorRef error = NULL;
        ABAddressBookRef ab = ABAddressBookCreateWithOptions(NULL, &error);
        if (!ab) {
            NSLog(@"%@", CFBridgingRelease(error));
            return false;
        }
    
        CFIndex groupCount = ABAddressBookGetGroupCount(ab);
        CFArrayRef allGroups = ABAddressBookCopyArrayOfAllGroups(ab);
    
        for (int i = 0; i < groupCount; i++) {
    
            ABRecordRef group = CFArrayGetValueAtIndex(allGroups, i);
    
            NSString *currentGroupName = CFBridgingRelease(ABRecordCopyCompositeName(group));
    
            if ([currentGroupName isEqualToString:groupName]) {
                //!!! important - save groupID for later use
                groupId = ABRecordGetRecordID(group);
                hasGroup = true;
                break;
            }
        }
    
        if (allGroups) CFRelease(allGroups);
        CFRelease(ab);
    
        return hasGroup;
    }
    

    I suspect that your attempt to CFRelease(ab) was crashing for other reasons (e.g., ab was NULL because ABAddressBookCreateWithOptions failed, perhaps you neglected to add NSContactsUsageDescription to your plist; ab was released before subsequent references to ab; you were releasing things you shouldn't have; you received NULL from some Core Foundation function and tried to CFRelease that; etc.). But the above should work fine as it fixes the aforementioned issues.

    Clearly, if using iOS 9 and later only, you can use the Contacts.framework and avoid all of these bridging annoyances altogether.