Memory Leak in startMonitoringNetworkReachability #696

Closed
phoney opened this Issue Dec 22, 2012 · 3 comments

Projects

None yet

2 participants

@phoney

There is a memory leak in [AFHTTPClient startMonitoringNetworkReachability]. Every time the method is called one or a few 32 byte malloc blocks with the Responsible Frame of _Block_copy_internal are leaked. I believe the problem is passing a block as the info pointer in the SCNetworkReachabilityContext. Maybe the retain/release callbacks aren't implemented correctly. I'm not certain, but I don't see where the block is released.

This is easily reproduced with the iOS Example app. In the file Post.m change the method: globalTimelinePostsWithBlock to the below code. Run under the leaks Instrument and every time the refresh button is tapped in the app memory blocks will leak.

+ (void)globalTimelinePostsWithBlock:(void (^)(NSArray *posts, NSError *error))block {

    AFAppDotNetAPIClient* client = [[AFAppDotNetAPIClient alloc] initWithBaseURL:[NSURL URLWithString:@"https://alpha-api.app.net/"]];

    [client getPath:@"stream/0/posts/stream/global" parameters:nil success:^(AFHTTPRequestOperation *operation, id JSON) {
        NSArray *postsFromResponse = [JSON valueForKeyPath:@"data"];
        NSMutableArray *mutablePosts = [NSMutableArray arrayWithCapacity:[postsFromResponse count]];
        for (NSDictionary *attributes in postsFromResponse) {
            Post *post = [[Post alloc] initWithAttributes:attributes];
            [mutablePosts addObject:post];
        }

        if (block) {
            block([NSArray arrayWithArray:mutablePosts], nil);
        }
    } failure:^(AFHTTPRequestOperation *operation, NSError *error) {
        if (block) {
            block([NSArray array], error);
        }
    }];
}

It is more common to pass self as the info pointer in a SCNetworkReachabilityContext. I don't see any reason to pass a block since all the block does is call a method on self. Passing self avoids the need for the retain/release callbacks and there is no block_copy operation going on.

@mattt

Looking at this again, it seems that this could be solved by actually implementing AFNetworkReachabilityReleaseCallback:

static void AFNetworkReachabilityReleaseCallback(const void *info) {
    if (info) {
        CFRelease(info);
    }
}

If you patch that in, does that eliminate the memory leak (and more importantly, not crash)?

@phoney

Yes, it does fix the memory leak and I don't see any crashes.

I ran the sample app on a device with this change both using the original code in the app and the modified code I showed in the first post. I also added the reachability callback block to call self reload. It all worked as expected with no memory leaks or crashes. Toggled Airplane mode and it worked as expected.

@mattt

Alright, I've merged in this fix with 314afe9. Thanks for opening this issue.

@mattt mattt closed this Dec 27, 2012
@greghe greghe pushed a commit to skillz/AFNetworking that referenced this issue Sep 3, 2015
@mattt mattt [Issue #696] Potentially fixing leak for reachability callback block ed0b8e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment