Question

I'm trying to do a simple thing; read an image from the internet, save it to the app's documents directory on the iphone, and read it back from that file so that i can do other things with it later. Writing the file works fine but when i try to read it back i get an EXC_BAD_ACCESS error in GDB that i have no idea how to resolve. Here is what my code basically looks like:

-(UIImage *) downloadImageToFile {
NSURL * url = [[NSURL alloc] initWithString: self.urlField.text];

NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);

NSString *documentsDirectory = [paths objectAtIndex:0];

[paths release]
NSString * path = [documentsDirectory stringByAppendingString:@"/testimg.png"];

NSData * data = [[NSData alloc] initWithContentsOfURL:url];

[data writeToFile:path atomically:YES];

return [[UIImage alloc] initWithContentsOfFile:path];
}

The code fails in the return statement when i try to initialize the UIImage from the file. Any ideas?

Edit: neglected to add a release that was the problem in the code originally.

Was it helpful?

Solution

Your code shows a severe lack of knowledge of how memory management works in Objective-C. In addition to the EXC_BAD_ACCESS errors you're receiving, improper memory management also causes memory leaks which, on a small device like the iPhone, can lead to random crashes.

I recommend you give this a thorogh read:

Introduction to Memory Management Programming Guide for Cocoa

OTHER TIPS

Note: This applies specifically to non-ARC memory management.

Since this has had so many views and the checked answer appropriately states that "code shows a severe lack of knowledge of how memory management works in Objective-C," yet no one has pointed the specific errors out, I figure I'd add an answer that touched on them.

The baseline-level rule that we have to remember about calling methods:

  • If the method call includes the words alloc, new, copy, or retain, we have ownership of the object created.¹ If we have ownership of an object, it's our responsibility to release it.

  • If the method call does not contain those words, we do not have ownership of the object created.¹ If we don't have ownership of an object, releasing it is not our responsibility, and we therefore should never do it.

Let's look at each line the OP's code:

-(UIImage *) downloadImageToFile {

We started a new method. In doing so we have started a new context in which each of the created objects lives. Keep this in mind for a bit. The next line:

    NSURL * url = [[NSURL alloc] initWithString: self.urlField.text];

We own url: the word alloc there tells us that we have ownership of the object and that we will need to release it ourselves. If we do not then the code will leak memory.

    NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);

We do not own paths: no use of the four magic words, so we do not have ownership and must never release it ourselves.

    NSString *documentsDirectory = [paths objectAtIndex:0];

We do not own documentsDirectory: no magic words = no ownership.

    [paths release]

Going back a couple of lines we see that we don't own paths, so this release will cause an EXC_BAD_ACCESS crash as we try to access something that no longer exists.

    NSString * path = [documentsDirectory stringByAppendingString:@"/testimg.png"];

We do not own path: no magic words = no ownership.

    NSData * data = [[NSData alloc] initWithContentsOfURL:url];

We own data: the word alloc there tells use that we have ownership of the object and that we will need to release it ourselves. If we do not then the code will leak memory.

The following two lines don't create or release anything. Then comes the last line:

}

The method is over, so the context for the variables has ended. Looking at the code we can see that we owned both url and data, but didn't release either of them. As a result our code will leak memory every time this method is called.

The NSURL object url isn't very big, so it's possible that we might never notice the leak, though it should still be cleaned up, there's no reason to leak it.

The NSData object data is a png image, and could be very large; we are leaking the entire size of the object every time this method is called. Imagine this was called every time a table cell was drawn: it wouldn't take long at all to crash the whole app.

So what do we need to do to fix the problems? It's pretty simple, we simply need to release the objects as soon as we no longer need them, commonly right after the last time they're used:

-(UIImage *) downloadImageToFile {

    // We own this object due to the alloc
    NSURL * url = [[NSURL alloc] initWithString: self.urlField.text];

    // We don't own this object
    NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);

    // We don't own this object
    NSString *documentsDirectory = [paths objectAtIndex:0];

    //[paths release] -- commented out, we don't own paths so can't release it

    // We don't own this object
    NSString * path = [documentsDirectory stringByAppendingString:@"/testimg.png"];

    // We own this object due to the alloc
    NSData * data = [[NSData alloc] initWithContentsOfURL:url];

    [url release]; //We're done with the url object so we can release it

    [data writeToFile:path atomically:YES];

    [data release]; //We're done with the data object so we can release it

    return [[UIImage alloc] initWithContentsOfFile:path];

    //We've released everything we owned so it's safe to leave the context
}

Some people prefer to release everything at once, right before the context closes at the end of the method. In that case both [url release]; and [data release]; would appear right before the closing } brace. I find that if I release them as soon as I can the code is clearer, making it clear when I go over it later exactly where I'm done with objects.

To summarize: we own objects created with alloc, new, copy, or retain in the method calls so must release them before the context ends. We don't own anything else and must never release them.


¹There's nothing actually magical in the four words, they're just a consistently-used reminder by the folks at Apple who created the methods in question. If we create our own initialization or copying methods for a class of our own then the inclusion of the words alloc, new, copy, or retain in its appropriate methods is our responsibility, and if we don't use them in our names then we'll need to remember for ourselves whether ownership has passed.

One thing that helps me a lot is to have a breakpoint on objc_exception_throw. Anytime I'm about to get an exception thrown, I hit this breakpoint and I can debug back up the stack chain. I just leave this breakpoint enabled all the time in my iPhone projects.

To do this, in xcode go to near the bottom of the left pane "Groups & Files" and find "Breakpoints". Open it and click on Project Breakpoints and in the detail pane (top), you'll see a blue field labeled "Double-Click for Symbol." Double-click on it and enter "objc_exception_throw".

Next time you throw an exception, you'll stop and in the debugger, you can walk back up the stack chain to your code that caused the exception.

Definitely give memory management rules a quick review. Nothing jumps out that would cause the error you're getting, but you're leaking all those objects your allocating. If you don't understand the retain/release pattern, chances are there's another spot in your code where you're not retaining an object properly, and that's whats causing the EXC_BAD_ACCESS error.

Also note that NSString has methods for dealing with filesystem paths, you should never have to worry about the separator yourself.

In general though, if you're getting EXC_BAD_ACCESSs in your code and you can't for the life of you figure out why, try using NSZombie (no, I'm not kidding).

In Xcode, expand the Executables section on the left. Double click on the listing that has the same name as your project (it should be the only one). In the window that pops up, go to Arguments, and in the bottom portion, click the plus button. The name should be NSZombieEnabled and the value should be set to YES

This way, when you try to access a released object, you'll have a better of what you're doing. Just set the value to NO once you've figured out the bug.

Hope this helps someone!

These errors occur when you are mismanaging memory (ie. an object is being released prematurely or similar)

Try doing something like the following..

UIImage *myImage = [[UIImage alloc] initWithContentsOfFile:path];
return [myImage autorelease];

I spent a lot of time experimenting whilst getting to grips with the concepts of release/autorelease. Sometimes the retain keyword needs to be played also (although probably not in this case)

Another option could be simply the path does not exist, or cannot be read from?

Perhaps, initWithContentsOfFile doesn't take a path argument? Browse around at the different init methods for UIImage, I think there's a different one for accepting a path.

There also might be something fancier you have to do for making a path? I remember doing something with "bundles"? Sorry to be so vague, it's all I remember offhand.

take the slash off the path, and make sure it is in the project. doesn't matter if it is in that dir, but it has to be added to the project for you to access it.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top