MrShoot
MrShoot

Reputation: 863

Object being released too early?

I've been hitting my head against a wall for 2 days now, trying to figure out what the heck is wrong with my code. So far, nothing. The only thing I've discovered is the fact that an object is trying to call release on itself and it hasn't been instantiated yet. (although, for some reason it doesn't happen 100% of the time, just whenever the heck it thinks)

Let me explain the problem (maybe I'm overlooking something, I hope you guys can shed some light into my darkness)

My model object

Person {
  NSString *name;
  NSNumber *age;
}

@property(nonatomic, retain) NSNumber* TheAge;
@property(nonatomic, retain) NSString *TheName;

My implementation

@synthesize TheName = name;
@synthesize TheAge = age;

+(Person*)personFromDictionary:(NSDictionary*)dic {
  Person* newPerson = [[[Person alloc] init]autorelease];
  newPerson.theAge = [dic objectForKey:kAge];
  newPerson.theName = [dic objectForKey:kName];

  return newPerson;
}

-(void)dealloc {
  self.TheAge = nil;
  self.TheName = nil;
}

I have a "collector thread" that reads a JSON array from the server, downloads it and parses it into a dictionary. Every entry in the dictionary corresponds to a person object This thread is a different class simply using the Person model

The thread does something like this (in an autorelease pool)

NSDictionary *parsedDict = download.returnValue;

NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
Person *tmpPerson = personFromDictionary

[entryDictionary setObject:tmpPerson forKey:kAge];

[pool release];

[self updateEntries:entryDictionary];

-(void)updateEntries:(NSMutableDictionary*)updatedDict {
   NSArray *keys = [updatedDict allKeys];
   for(NSString *key in allKeys){
        Person *entry = [updatedDict valueForKey:key];
        [entriesLock lock];
        [currentPersonEntries setObject:entry forKey:key];
        [entriesLock unlock];

    }
}

When I get the crash (which happens randomly for some damn reason, I get the stack trace as follows

person dealloc

person setTheAge (bam crash)

I'm guessing because the setter would look something like this

-(void)setTheAge:(NSString*)theAge {
  [theAge retain];
  [age release]; // it doesn't exist for some reason???
  age = theAge;
}

How can I protect vs this type of thing?

Upvotes: 0

Views: 186

Answers (1)

Rob
Rob

Reputation: 437967

A couple of thoughts.

First, you have:

Person {
    NSString *name;
    NSNumber *age;
}

@property(nonatomic, retain) NSNumber* TheAge;
@property(nonatomic, retain) NSString *TheName;

And in your implementation, you have:

@synthesize TheName = name;
@synthesize TheAge = age;

Apple no longer recommends you define your ivars explicitly, but let your synthesize statement take care of it (probably because if you misspell one of those, you can end up with an extra, unintended ivar). So you might just want:

@interface Person : NSObject

@property(nonatomic, retain) NSNumber* theAge;
@property(nonatomic, retain) NSString* theName;

@end

Also, when naming your ivars, the emerging standard is (but, of course, you can do what you want) to use underscores for ivars, and starting your variable names with lowercase ... uppercase for classes, lowercase for variables, e.g.:

@synthesize theName = _theName;
@synthesize theAge = _theAge;

Second, in your dealloc you're setting those ivars to nil. While that's the correct way to release in ARC code, in non-ARC code, you should just use your ivars, and use the release command:

- (void)dealloc {
    [_theAge release];
    [_theName release];

    [super dealloc];
}

Third, are you writing that setter setTheAge or are you guessing what the compiler is doing itself? Your code can probably be improved (you're using a local variable which is the same as your property, which is just confusing, you'd want to do key value notifications, etc.), but I won't address that because you'd be better off just letting the compiler do its own setter unless there is something else that you're trying to accomplish. Let me know.

Fourth, your -(Person*)personFromDictionary:(NSDictionary*)dic is a curious method. I'd suggest writing your own init, like:

- (Person*)initFromDictionary:(NSDictionary*) dic 
{
    self = [super init]; // I always inherit from NSObject, in which case you'd have this line

    if (self)
    {
        self.theAge  = [dic objectForKey:kAge];
        self.theName = [dic objectForKey:kName];
    }

    return self;
}

This way, you could create your Person object like:

Person *aPerson = [[Person alloc] initWithDictionary:entryDictionary];

Your current implementation assumes the Person object already exists (because you preface the method name with "-" rather than "+"), but then creates a new one. It's a little strange. You might be able to do something like that, but the above code pattern is more commonplace and achieves what I think you want.

Finally, I'm not sure what you're trying to do with updateEntries, so I'm not sure what to suggest there, but I don't quite get the updateEntries outside of your pool, whether you meant it to be a dictionary full of Person objects, etc. If you describe what you're trying to accomplish there, I'd be happy to give you my two cents on that, too.

Update:

By the way, if you're debugging your code, sometimes adding NSLog statements is helpful. You might want to create a description method for your Person class to facilitate that, though, so you can see the contents of your Person objects, e.g.:

- (NSString *)description
{
    return [NSString stringWithFormat:@"Person (%p) {\n  theName = '%@'\n  theAge = %@\n}", self, self.theName, self.theAge];
}

This way, if you have a Person object named, say, aRandomPerson, you can then have a statement like:

NSLog(@"%@", aRandomPerson);

Also, if you have a dictionary entry with Person objects, if you NSLog that dictionary, you'll now have a meaningful log statement. This may help you diagnose what's in your NSDictionary items (if it really has Person objects rather than just NSString and NSNumber objects).

Upvotes: 2

Related Questions