views:

80

answers:

2

I'm having an issue understanding why releasing an object that I just created is causing a memory access error in my application.

I'm creating a bunch of objects and adding them to an NSMutableArray. After I add them to the array, I release them. The first time the function runs, things go smoothly. The second time the function runs, releasing the object crashes the application and I'm not sure why.

Here is the part of the function in question (full project source is available here: http://github.com/cpjolicoeur/echowaves-notifier-osx)

- (void)connectionDidFinishLoading:(NSURLConnection *)connection {
    [connection release];
    NSString *responseString = [[NSString alloc] initWithData:echowaves.responseData encoding:NSUTF8StringEncoding];
    [echowaves.responseData release];

    ...more code...

    [[echowaves updatedConvos] removeAllObjects];
    NSDictionary *dictionary = [responseString JSONValue];
    if ( [dictionary count] ) {
        for (NSDictionary *subscription in dictionary ) {
            UpdatedConvo *convo = [[UpdatedConvo alloc] initWithConvoName:[[subscription objectForKey:@"subscription"] objectForKey:@"convo_name"]
                                                                 convoURI:[[subscription objectForKey:@"subscription"] objectForKey:@"conversation_id"]
                                                              unreadCount:[[[subscription objectForKey:@"subscription"] objectForKey:@"new_messages_count"] integerValue]];


            [[echowaves updatedConvos] addObject:convo];
            [convo release];  // THIS LINE CAUSES CRASH 2ND TIME THROUGH
        }
    } else {

        ...more code....

    }
}            

The app crashes on the [convo release] line the 2nd time through the function. Since the convo object is created two lines before it I'm not sure why.

I'm under the impression that the UpdatedConvo *convo = [[UpdatedConvo alloc] initWithConvoName...] call will create the object and give it a retain count of 1. Then when I add the object to the [echowaves updatedConvos] NSMutableArray object, it should bump the retain count by 1 (now count is 2). I'm done with that "temporary" convo object so I then release it, which should move its retain count back down to 1, correct??

What am I missing here?

To get the app to run successfully, I have to comment out the [convo release] line. I don't know why though, and I feel that this is probably giving me a slow memory leak since that newly created UpdatedConvo object isn't being freed from memory properly.

+3  A: 

Your method should work. It is a common pattern to create an object, add it to a dictionary, and then release it.

The fact that it doesn't work suggests that the problem is elsewhere. The first place to check is in the UpdatedConvo class itself. Does it do anything fancy in its dealloc method?

edit: The problem is definitely in UpdatedConvo. You assign your instance variables (with =) in your init method:

- (id)initWithConvoName:(NSString *)convoName
               convoURI:(NSString *)convoURI
            unreadCount:(int)updatesCount 
{
    if ( self = [super init] ) {
        ewURI = convoURI;
        ewName = convoName;
        newMessagesCount = updatesCount;
    }
    return self;
}

But then you call release on them in dealloc:

- (void)dealloc {
    [ewURI release];
    [ewName release];
    [super dealloc];
}

Using = in the init method does not increase the retain count, so when you release them, you are actually reducing the retain count too far.

e.James
I dont think it does anything special. It calls release on two NSString instance variables that are set during the custom init method, then just calls `[super dealloc]`
cpjolicoeur
so, since I'm doing a "simple assignment" on the instance variables instead of retaining them, what actually happens to them. How long do they stay around during the object lifecycle?Also, why does the app run correctly during the first time through the loop, but not during subsequent runs?If the code is incorrect (which I do believe you that it is) why does it "work" fine the first time through?
cpjolicoeur
It depends. The error in the code is not guaranteed to cause the error at the spot when run! If so, the debugging would be so much easier. Alas. Anyway, you need to `retain` the strings in the `init`. Or maybe you need to `copy` them.
Yuji
The first time through, the simple assignment means that your member variables simply point to the original objects that were passed in as arguments to the `init` method. They will exist as long as the original arguments remain in memory. The program succeeds the first time because the call to `[convo release]` does not actually release anything (the retain count on each `convo` is still 1, since they exist in the array).
e.James
by not retaining or copying them, what is actually happening to them though? I can still access those instance variables fine through the objects stored in the array. I guess my question is, why can I though since I'm not retaining or copying them properly?
cpjolicoeur
The *second* time around, however, your call to `removeAllObjects` will result in the dealloc method being invoked, which will leave those member variables (and, in fact, the objects that were originally used to set them) in an undefined state. The exact failure mode after that is difficult to know for sure. The most likely suspect is that somewhere along the line a message is sent to an object that no longer exists (also known as a zombie). This is why such objects are called zombies: They have been killed (released) but you still have a pointer to them, and you *might* send a message later.
e.James
By not retaining or copying them, you rely on whatever container held them in the first place. In this case, `[responseString JSONValue]`. Those objects will exist as long as that original source is available. Note that you may be able to message the objects even after they *have* been released, as long as nothing has written over them in memory.
e.James
I see. So my next misunderstanding then is with @property and @synthesize on those ivars.If the @property declaration for them uses (retain), then I thought that when I assigned them via `=` that they would be retained. This clearly is not the case, so what does setting the (retain) or even (copy) attribute on the @property declaration actually do for me ?
cpjolicoeur
The `retain` and `copy` attributes only come into effect when you use a property accessor, such as `[myObject setName:@"name"];` or `myObject.name = @"name";` You can use such accessors in your init method, but it is usually discouraged. The best approach is to manually call `retain` or `copy` in the init method.
e.James
I see. Thanks so much for the time and insight.If, in my init method I had done `[self.name = @"name]` would that call the `retain` attribute since I'd be using the accessor in my init method?
cpjolicoeur
You're welcome! Using `self.name = @"name"` in your init method would work, but such things are usually discouraged. 90% of the time it wouldn't be a problem, but if you ever (later on down the line) write a custom propety accessor, you could end up doing things that you shouldn't during initialization.
e.James
See http://stackoverflow.com/questions/192721/why-shouldnt-i-use-obective-c-2-0-accessors-in-init-dealloc
e.James
+1  A: 

As e.James already mentioned: UpdatedConvo seems to be your problem.
I looked at the github project you posted. Probably you are over-releasing in UpdatedConvo:

- (void)dealloc 
{
    [ewURI release];
    [ewName release];
    [super dealloc];
}

You are neither creating, nor retaining ewURI and ewName in the class.
So you shouldn't release it there.

Update: Fixed a misleading typo (I wrote "releasing" instead of "retaining" in last sentence)

Update2: I would run your code through Instruments.app with the "Zombie" and "Leaks" presets.
If you haven't tried Instruments before, this is a great opportunity to see it at work.

weichsel
I did run it through "Leaks" and saw a bunch of UpdatedConvo objects leaked. But I expected that because I wasn't releasing them. I"ve never tried "Zombie" before
cpjolicoeur
The "Zombies" instrument warns you whenever your code is over-releasing an object. (which is the case in your UpdatedConvo class). e.James provided a very detailed explanation in his comments.
weichsel