1

The following code that downloads an image and returns the result with a block so that I can take advantage of the async features of blocks and Grand Central Dispatch. I find that if the image or error object is nil I get an EXC_BAD_ACCESS error. Is it always going to cause an error if the value of a parameter is nil?

The part which is failing is the returnImage block which is used to return the image at the end of the method.

- (void)downloadImageWithBlock:(void (^)(UIImage *image, NSError *error))returnImage {
    dispatch_queue_t callerQueue = dispatch_get_current_queue();
    dispatch_queue_t downloadQueue = dispatch_queue_create("Image Downloader Queue", NULL);
    dispatch_async(downloadQueue, ^{
        UIImage *image = nil;
        NSError *error;

        // get the UIImage using imageURL (ivar)

        dispatch_async(dispatch_get_main_queue(), ^{
            NSLog(@"Downloading: %@", imageURL);
        });

        NSURL *url = [NSURL URLWithString:imageURL];
        NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:url];

        NSURLResponse *urlResponse = nil;
        NSData *response = [NSURLConnection sendSynchronousRequest:request returningResponse:&urlResponse error:&error];

        if (!error && response) {
            dispatch_async(dispatch_get_main_queue(), ^{
                NSLog(@"Success!");
            });
            image = [UIImage imageWithData:response];
        }

        // image will be nil if the request failed

        dispatch_async(callerQueue, ^{
            returnImage(image, error);
        });
    });
    dispatch_release(downloadQueue);
}

Below is the stack trace which I do not know how to read.

0x00002d20  <+0000>  push   %ebp
0x00002d21  <+0001>  mov    %esp,%ebp
0x00002d23  <+0003>  sub    $0x18,%esp
0x00002d26  <+0006>  mov    0x8(%ebp),%eax
0x00002d29  <+0009>  mov    %eax,-0x4(%ebp)
0x00002d2c  <+0012>  mov    -0x4(%ebp),%eax
0x00002d2f  <+0015>  mov    0x14(%eax),%eax
0x00002d32  <+0018>  mov    %eax,(%esp)
0x00002d35  <+0021>  movl   $0x3,0x4(%esp)
0x00002d3d  <+0029>  call   0x314c <dyld_stub__Block_object_dispose>
0x00002d42  <+0034>  mov    -0x4(%ebp),%eax
0x00002d45  <+0037>  mov    0x18(%eax),%eax
0x00002d48  <+0040>  mov    %eax,(%esp)
0x00002d4b  <+0043>  movl   $0x3,0x4(%esp)
0x00002d53  <+0051>  call   0x314c <dyld_stub__Block_object_dispose>
0x00002d58  <+0056>  mov    -0x4(%ebp),%eax
0x00002d5b  <+0059>  mov    0x1c(%eax),%eax
0x00002d5e  <+0062>  mov    %eax,(%esp)
0x00002d61  <+0065>  movl   $0x7,0x4(%esp)
0x00002d69  <+0073>  call   0x314c <dyld_stub__Block_object_dispose>
0x00002d6e  <+0078>  add    $0x18,%esp
0x00002d71  <+0081>  pop    %ebp
0x00002d72  <+0082>  ret    
0x00002d73  <+0083>  nopw   0x0(%eax,%eax,1)
0x00002d79  <+0089>  nopl   0x0(%eax)
Brennan
  • 11,546
  • 16
  • 64
  • 86
  • What is making debugging this code hard is the fact that the error does not show up on a line of code so I have to comment out chunks of code to try to identify where the error is happening. Pinpointing the line of code for the error is difficult. – Brennan May 23 '11 at 23:58

2 Answers2

5

NSError *error; creates a pointer to an random/invalid memory location.

If an error does not occur in the block, it will not assign a new (valid) value to error. As a result, when you test whether error is nil, you are dereferencing an invalid pointer:

NSError *error; // invalid pointer
NSLog(@"%@", error); // crash -- dereferencing invalid pointer

You should either:

  1. Always assign nil to error variables before passing them to a method of this nature.
  2. Consult the method's documentation which will usually tell you whether the error variable has had a valid value assigned to it based the method's return value.

Update: Local object variables now default to nil under ARC.

titaniumdecoy
  • 18,900
  • 17
  • 96
  • 133
  • 2
    +1 and I just want to emphasize the point: Apple [guarantees](http://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/ErrorHandlingCocoa/CreateCustomizeNSError/CreateCustomizeNSError.html#//apple_ref/doc/uid/TP40001806-CH204-SW1) that Cocoa methods which return NSError objects indirectly will have a valid error object if the method indicates failure with its return value. However, they do not guarantee the reverse: that if the method indicates success, the error object will be nil. It is incorrect to check the error unless the direct return value of the method indicates failure. – jscs May 24 '11 at 02:09
  • Thanks Jason and Josh. This makes sense now. I was thinking somehow this was a GCD issue when it was just a local variable issue. I also set the value of imageURL which is an ivar to a local variable so that it does not become a problem in the block. Now the code is working pretty reliably. I did find I need to check the HTTP status code because not having an error does not mean I did not get 403 or 404, etc. I require 200 for success now. – Brennan May 24 '11 at 02:33
  • 2
    @Brennan To summarise: always check the return value first. If the return value indicates an error (e.g., it’s `nil` or `NO`), check the `NSError` output parameter. Assigning `nil` to the `NSError` variable and relying on it is not robust. –  May 24 '11 at 03:42
1

So couple of points that may help:

You shouldn't be checking error or make any assumptions about error unless response is nil.

If response is not nil, then error is undefined, and yet you're asking the block to retain error. Are you sure that's not your issue?

Are you sure that returnImage() can handle nils (or an undefined NSError *)?

Firoze Lafeer
  • 17,133
  • 4
  • 54
  • 48
  • Generally, there's no point in NOT setting NSError pointers to nil, although runtime does that for us automatically. @Brennan Post stack trace and check callerQueue. – TheBlack May 24 '11 at 00:12
  • I've added the stack trace to the question. – Brennan May 24 '11 at 00:35
  • Setting error to nil when it is declared now allows it to run without the exception. I am not sure what is going on here. – Brennan May 24 '11 at 00:37
  • 1
    @TheBlack: The runtime doesn't set NSError (or any other) pointers to nil, _unless_ they are instance variables, which isn't the case here. – omz May 24 '11 at 00:41
  • @TheBlack, I'm not sure what you mean by the runtime does this automatically? Can you pls explain. @Brennan, I think the issue is trying to [error retain] (or perhaps release?) when 'error' was undefined. – Firoze Lafeer May 24 '11 at 00:42
  • @omz You're right. It was assumption based on instance variables behavior. – TheBlack May 24 '11 at 00:52
  • Setting the error variable to nil fixed the problem. The block was likely trying to retain on an undefined value. A nil value is alright but clearly not an undefined value. – Brennan May 24 '11 at 02:29