CODEWITHSUNDEEP
objective-cioscocoa-touchasihttprequestobjective-c-blocks
Ertai
Ertai

Reputation: 1745

ASIHTTPRequest block memory leak

I'm unable to find error in this code:

-(void)downloadImageFromURL:(NSURL*)url withCompletionBlock:(RSSMessageImageDownloadCompletionBlock)completionBlock
{

    ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:url];

    __block RSSMessage *_self = self;
    request.completionBlock =
    ^{    
        __block NSData *responseData = request.responseData;

        dispatch_async( dispatch_get_main_queue(), ^{
            _self.image = responseData;            
            [[[UIApplication sharedApplication] delegate] saveContext];
            if(completionBlock != nil)
            {
                completionBlock();
            }

        });
    };

    [request startAsynchronous];
}

In this form I've got a memory leak from instruments. I was assuming it is because I've lacked the __block keyword before: ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:url];

But when I've added this keyword to above line I've got error like:

* -[NSConcreteMutableData isNSData__]: message sent to deallocated instance 0xdeab380

I do not know how to retain the request data and not leak memory.

Upvotes: 0

Views: 1506

Answers (2)

Jody Hagins
Jody Hagins

Reputation: 28349

I know nothing about this library, so you should probably heed the advice of viking inthat regard.

However, I can help you with your leak, which is a result of the retain cycle created between the block and the request object. Specifically, note that your request object is holding a strong reference to the code block object (via request.completionBlock).

In turn, your code block object is holding a strong reference to request because it accesses request.responseData. Further, note that your code looks like it is most likely ARC, but that does not explain your _self variable construct, which looks like a non-ARC weak reference. Non-ARC __block did not retain the object. Under ARC, __block does cause a retain.

Assuming ARC, I would suggest the following change.

-(void)downloadImageFromURL:(NSURL*)url withCompletionBlock (RSSMessageImageDownloadCompletionBlock)completionBlock
{
    ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:url];

    __weak ASIHTTPRequest *weakRequest = request;
    __weak RSSMessage *weakSelf = self;

    request.completionBlock = ^{
        NSData *responseData = weakRequest.responseData;
        // Check for nil if not ok with nil data
        dispatch_async( dispatch_get_main_queue(), ^{
            weakSelf.image = responseData;            
            [[[UIApplication sharedApplication] delegate] saveContext];
            if(completionBlock != nil)
            {
                completionBlock();
            }
        });
    };
    [request startAsynchronous];
}

Now, the completion block holds a weak reference to the response object, which breaks the retain cycle. Note, in general, you should create a local strong reference to a weak reference, to make sure the object stays around long enough to do its job. However, in this specific case, it does not appear to be necessary. I assume it's OK to have a nil image.

Upvotes: 3

vikingosegundo
vikingosegundo

Reputation: 52227

from the docs:

- (IBAction)grabURLInBackground:(id)sender
{
   NSURL *url = [NSURL URLWithString:@"http://allseeing-i.com"];
   __block ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:url];
   [request setCompletionBlock:^{
      // Use when fetching text data
      NSString *responseString = [request responseString];

      // Use when fetching binary data
      NSData *responseData = [request responseData];
   }];
   [request setFailedBlock:^{
      NSError *error = [request error];
   }];
   [request startAsynchronous];
}

seems like the request itself needs to be marked with __block

Note, that the original author of ASIHTTPRequest isnt supporting it anymore. He explains his reasons very well and give links and suggestions for alternative projects. I am happy with AFNetworking, that has a great block based interface.

Upvotes: 1

Related Questions