macOS Objective C Help - Encoding

[nashyo]

Hello MacRumors,

I'm getting a EXC_BAD_ACCESS error message within the 'description' method, specifically at the NSString statement. I have no idea why. Someone please help me.

#import <Foundation/Foundation.h>

@interface Employee : NSObject {
@private
    NSString *name;
    int grade;
}
  
@property (retain) NSString *name;
@property int grade;


@end

#import "Employee.h"

@implementation Employee

@synthesize name, grade;

-(void) encodeWithCoder: (NSCoder *) coder 
{
    [coder encodeObject:name forKey:@"EMPname"];
    NSNumber *gradeBox = [NSNumber numberWithInt:grade];
    [coder encodeObject:gradeBox forKey:@"EMPgrade"];
}

-(id) initWithCoder: (NSCoder*) coder
{
    name = [coder decodeObjectForKey:@"EMPname"];
    NSNumber *gradeBox = [coder decodeObjectForKey:@"EMPgrade"];
    grade = (int) [gradeBox integerValue];
    return self;
}

-(NSString *) description 
{
    NSString *desc =
    [NSString stringWithFormat:@"Employee: %@, Grd: %i", name, grade];
    return desc;
}

-(id) init 
{
    [super init];
    return self;
}

-(void) dealloc 
{
    [super dealloc];
}

@end

#import <Foundation/Foundation.h>
#import "Employee.h"

int main (int argc, const char * argv[])
{
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
    
    Employee *bob = [[Employee alloc] init];
    [bob setName:@"Bob Jones"];
    [bob setGrade:21];
    
    NSLog(@"Details are %@", [bob description]);
    
    [NSKeyedArchiver archiveRootObject:bob toFile:@"/Users/Rob/Bob.plist"];

    Employee *fred = [NSKeyedUnarchiver unarchiveObjectWithFile:@"/Users/Rob/Bob.plist"];
    NSLog (@"The new object is %@", [fred description]);
    
    [pool drain];
    
return 0;

}

 

[lee1210]

description is called twice. Which time does it fail? Why don't you NSLog individual elements in that method to see what your ivar's variables are?

-Lee

 

[nashyo]

description is called twice. Which time does it fail? Why don't you NSLog individual elements in that method to see what your ivar's variables are?

-Lee

If I remove the unarchiver statement and the 'fred' object, runtime is fine. The error only seems to happen when the description is called a second time, by the object 'fred'.

 

[mfram]

Add a 'retain' when you pull the name out of the archiver.

 

[nashyo]

Still not working...

Add a 'retain' when you pull the name out of the archiver.

This doesn't seem to solve the issue.

 

[chown33]

This doesn't seem to solve the issue.

Show your revised code.

 

[Sydde]

.

 

[jared_kipe]

I bet you can get bob to crash by changing your init method to this

-(id) init 
{
   // [super init];
    return self;
}

 

[mduser63]

Without actually running the code, this looks like the problem to me:

-(id) initWithCoder: (NSCoder*) coder
{
    name = [coder decodeObjectForKey:@"EMPname"]; // THIS IS BAD
    NSNumber *gradeBox = [coder decodeObjectForKey:@"EMPgrade"];
    grade = (int) [gradeBox integerValue];
    return self;
}

You should either set name using the setter method:

[self setName:[coder decodeObjectForKey:@"EMPname"]];

or retain it (or copy it) when you assign to the name ivar:

name = [[coder decodeObjectForKey:@"EMPname"] retain];

The first option (using the setter method) is better coding style. (Note that it's fine to do "self.name = [coder...", the code I wrote is equivalent.)

The problem with what you've got is that the object returned by decodeObjectForKey: is autoreleased, and since you don't retain it, it gets released at some arbitrary point in the future. You still have a reference to it in the name instance variable. When the description method runs, it sends a message to name, and since name has already been released, you get a crash. It's actually a very simple memory management problem, and is the kind of thing that people new to Objective-C often have trouble with at first. I'd suggest reading through Apple's documentation on Objective-C memory management. Also Google NSZombie for information about how to track these kinds of problems down.

 

[jared_kipe]

Without actually running the code, this looks like the problem to me:

-(id) initWithCoder: (NSCoder*) coder
{
    name = [coder decodeObjectForKey:@"EMPname"]; // THIS IS BAD
    NSNumber *gradeBox = [coder decodeObjectForKey:@"EMPgrade"];
    grade = (int) [gradeBox integerValue];
    return self;
}

You should either set name using the setter method:

[self setName:[coder decodeObjectForKey:@"EMPname"]];

or retain it (or copy it) when you assign to the name ivar:

name = [[coder decodeObjectForKey:@"EMPname"] retain];

The first option (using the setter method) is better coding style. (Note that it's fine to do "self.name = [coder...", the code I wrote is equivalent.)

The problem with what you've got is that the object returned by decodeObjectForKey: is autoreleased, and since you don't retain it, it gets released at some arbitrary point in the future. You still have a reference to it in the name instance variable. When the description method runs, it sends a message to name, and since name has already been released, you get a crash. It's actually a very simple memory management problem, and is the kind of thing that people new to Objective-C often have trouble with at first. I'd suggest reading through Apple's documentation on Objective-C memory management. Also Google NSZombie for information about how to track these kinds of problems down.

While I agree that this is a problem, the autorelease pool isn't drained until after the the decoding and description call. Though I guess I don't know THAT much about autorelease pools, I assume that an autoreleased object is at least valid until the nearest context pool drains.

 

[nashyo]

The penny drops

Thanks for all your responses.

The problem with what you've got is that the object returned by decodeObjectForKey: is autoreleased, and since you don't retain it, it gets released at some arbitrary point in the future. You still have a reference to it in the name instance variable. When the description method runs, it sends a message to name, and since name has already been released, you get a crash. It's actually a very simple memory management problem, and is the kind of thing that people new to Objective-C often have trouble with at first. I'd suggest reading through Apple's documentation on Objective-C memory management. Also Google NSZombie for information about how to track these kinds of problems down.

Your absolutely right. Memory management has definitely been the hardest thing to grasp so far.

Calling retain for object 'name' immediately after the method 'decodeObjectForKey, ensured 'name' stuck around long enough for when it was called in the description method.

I added a release call for object 'name' to the dealloc method.

I LOVE IT WHEN THE PENNY DROPS!

 

[mduser63]

While I agree that this is a problem, the autorelease pool isn't drained until after the the decoding and description call. Though I guess I don't know THAT much about autorelease pools, I assume that an autoreleased object is at least valid until the nearest context pool drains.

You can't and shouldn't make assumptions like that. Often this assumption is correct, but this is one of those cases where it's not, and it causes a problem. In a typical Cocoa app, main() does nothing more than call NSApplicationMain which starts up the application. The automatically created autorelease pool behavior is tied up in run loop behavior. If you get down to it, you can usually assume it's drained at the end of each run loop pass. However, this isn't always the case, so you should always assume that autoreleased objects are only good through the end of the current outermost scope of your own code in the call stack.

As it turns out, this is one of those cases where you don't know when the autorelease pool is drained at all. In this case, NSKeyedUnarchiver creates its own local autorelease pool, and drains it all within +[NSKeyedUnarchiver unarchiveObjectWithFile:]. This is probably an optimization to keep memory usage down when unarchiving lots of objects at once. So, the name string is actually not in the outer autorelease pool (created in main()), rather it's in in an autorelease pool internal to NSKeyedUnarchiver, and that pool gets drained before unarchiveObjectWithFile: returns. You can see all this by stepping through the assembly code that makes up the private +[NSKeyedUnarchiver _decodeObject] method.

The takeaway for the OP is that you shouldn't try to be more clever than the system, and should always adhere to the memory management rules of Cocoa. If you want object that you don't already own to stick around beyond the current scope, you need to retain it. This is made easier by using the property accessors, since the accessor methods themselves do the work of retaining and later releasing (via -dealloc) the objects for you.

Calling retain for object 'name' immediately after the method 'decodeObjectForKey, ensured 'name' stuck around long enough for when it was called in the description method.

I added a release call for object 'name' to the dealloc method.

What you've done will certainly work and is technically correct, but again I'd encourage you to use the property accessors/setter methods when you're setting properties. Except in the accessors themselves (which are generally @synthesized for you unless you need to override that and write them yourself for some reason), you generally shouldn't access instance variables directly. This is how I would write Employee.m:

#import "Employee.h"

@implementation Employee

@synthesize name, grade;

- (void)encodeWithCoder:(NSCoder *)coder 
{
    [coder encodeObject:self.name forKey:@"EMPname"];
    [coder encodeInt:self.grade forKey:@"EMPgrade"];
}

- (id)initWithCoder:(NSCoder*)coder
{
    //self = [super initWithCoder:coder]; // We'd do this if the superclass implemented NSCoding, in this case it doesn't
    self = [super init]; // So we call the superclass's designated initializer instead
    if (self != nil)
    {
        self.name = [coder decodeObjectForKey:@"EMPname"];
        self.grade = [coder decodeIntForKey:@"EMPgrade"];
    }
    return self;
}

- (NSString *)description 
{
    return [NSString stringWithFormat:@"Employee: %@, Grade: %i", self.name, self.grade];
}

- (id)init 
{
    self = [super init];
    
    if (self != nil)
    {
        // Custom initialization here
    }
    
    return self;
}

- (void)dealloc 
{
    self.name = nil;
    
    [super dealloc];
}

@end

 

[Sydde]

Look at it from the perspective of the unarchiver. There is no way for it to guess what any given -initWithCoder: method might involve. An object must completely restore itself, which could mean creating a lot of temporary objects – dynamically regenerating content that did not actually need to be written to the archive, in order to minimize archive data. With a large graph, this could amount to a great deal of cruft. Every object that is decoded may not even be needed, which is the assumption of the unarchiver, so they are autoreleased. From what I have observed, the safest course for the programmer is to assume that every -initWithCoder: method is immediately bracketed by an autorelease pool.

 

[nashyo]

Floating Pointer

I just noticed mduser63, that you have assigned the name ivar to nil here

- (void)dealloc
{
self.name = nil;

[super dealloc];
}

@end

but you have not released?

I believe you need to release first, to drop the retain count to zero and free up that area of memory, then remove the floating pointer by assigning it to nil.

Is that right?

 

[Sydde]

I just noticed mduser63, that you have assigned the name ivar to nil here

- (void)dealloc
{
self.name = nil;

[super dealloc];
}

@end

but you have not released?

I believe you need to release first, to drop the retain count to zero and free up that area of memory, then remove the floating pointer by assigning it to nil.

Is that right?

The name ivar is also defined with "@property (retain)" and synthesized. The synthesis creates code that looks something like this

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

The dot-syntax assignment is equivalent to calling that method, which is the same as releasing name while at the same time protecting against a possible dangling pointer.

 

[mduser63]

The dot-syntax assignment is equivalent to calling that method, which is the same as releasing name while at the same time protecting against a possible dangling pointer.

Sydde is exactly correct. I like that style better for a couple reasons. For one thing, it makes it so you can change the @property declaration so that name is an assigned property instead of retained, and you don't need to change any other line in the code. If you explicitly released name instead, you'd have to remove that or you'd be likely to overrelease name and crash.

Also, in more complex programs, it's not uncommon to write your own setter and do something a little more complicated than just setting the ivar. For example, sometimes you might start observing a notification that the ivar's object posts. Anytime the property changes, you need to remove yourself as an observer of the old object, and add yourself as an observer on the new object. Especially important is removing yourself as an observer of any notification when dealloced (again to avoid crashing). By keeping the notification registration/unregistration confined to the property setter and doing self.name = nil in dealloc, it's easy to handle this.

All that said, you could do this and it would be correct, just not my favorite way of doing things:

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

 

[nashyo]

I can understand the following quote, but I have some fundamental questions to ask about it.

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

it makes it so you can change the @property declaration so that name is an assigned property instead of retained, and you don't need to change any other line in the code.[/code]

1) For the above set name method; if I didn't call the retain instance method of object 'newName', then I would surely need to change the code somewhere else, in order to retain it?

2) For the above set name method, are you saying that I avoid a dangling pointer issue, because I'm assigning the ivar to something new immediately after releasing it?

3) Even simpler question: If I wanted to use the above set name method, is it simply a case of writing out the following for the header file (instead of @Property 'parameter')

- (void) setName: (NSSTring*) newName;

and the following for the implementation (instead of @Synthesize).

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

4) If so, would the complier parse dot syntax if it was written this way (instead of @Property and @Synthesize)?

Thanks

 

[Sydde]

1) For the above set name method; if I didn't call the retain instance method of object 'newName', then I would surely need to change the code somewhere else, in order to retain it?

Theoretically, yes. But if you create newName with +alloc, -copy or +new, you will receive an object that is already retained (you own it), so the retain method would be redundant (and would probably leak). Also, if you access the ivar directly, you need to send the existing value a -release to be consistent. The main point of using the accessor is to support KVC/KVO so that any other object, like one having a binding, can be informed of a change.

2) For the above set name method, are you saying that I avoid a dangling pointer issue, because I'm assigning the ivar to something new immediately after releasing it?

Yes.

3) Even simpler question: If I wanted to use the above set name method, is it simply a case of writing out the following for the header file (instead of @Property 'parameter')

- (void) setName: (NSSTring*) newName;

and the following for the implementation (instead of @Synthesize).

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

Yes

4) If so, would the complier parse dot syntax if it was written this way (instead of @Property and @Synthesize)?

Thanks

One thing to be careful about here. If you use dot syntax, remember to also write a getter method if there is any chance that another object will want or need to obtain the value.

- (NSString *)name {
    return name;
}

Using @property and @synthesize simply writes these methods for you. Your .m file becomes much more compact. Also, if I understand correctly, @synthesize looks at your .h file to see if the method it is about to generate is already prototyped and does not create an existing method. If you need a customized setter or getter, you can write that method and let @synthesize create the other one for you.

 

[jared_kipe]

The name ivar is also defined with "@property (retain)" and synthesized. The synthesis creates code that looks something like this

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

The dot-syntax assignment is equivalent to calling that method, which is the same as releasing name while at the same time protecting against a possible dangling pointer.

Probably more like

- (void) setName: (NSString *)newName {
    [newName retain];
    [name release];
    name = newName;
}

On the off chance that (name == newName) the original code could ruin the property.

 

[mduser63]

I can understand the following quote, but I have some fundamental questions to ask about it.

- (void)setName:(NSString *)newName {
    [name release];
    name = [newName retain];
}

1) For the above set name method; if I didn't call the retain instance method of object 'newName', then I would surely need to change the code somewhere else, in order to retain it?

I was referring to cases where you have:

@property (assign) NSString *name;

and a corresponding @synthesized setter. If you use self.name = nil in dealloc, you can switch assign to retain in the @property declaration without changing anything else. The compiler takes care of changing the setter, since it's @synthesized. Note that in your specific case, it would be incorrect to use an assigned property, but there are other cases where it is the correct thing to do (delegate properties are the really common case).

3) Even simpler question: If I wanted to use the above set name method, is it simply a case of writing out the following for the header file (instead of @Property 'parameter')

4) If so, would the complier parse dot syntax if it was written this way (instead of @Property and @Synthesize)?

Think of it this way. Putting this in your .h file:

@property (nonatomic, retain) NSString *name;

is exactly equivalent to doing this (the complier effectively inserts this for you when you do the above):

- (NSString *)name;
- (void)setName:(NSString *)name;

Independently, in your .m file, you can replace this:

@synthesize name;

with this:

- (NSString *)name
{
    return name;
}

- (void)setName:(NSString *)aName
{
    if (aName != name)
    {
        [name release];
        name = [aName retain];
    }
}

You can do this regardless of whether you used @property in the .h file.

Further, you can actually write @synthesize, then still write your own setter and/or getter. The compiler will see that you've written your own method(s) and will skip inserting its own synthesized version of it/them. (Note that you can't write your own accessors for @property declaration that are atomic, only nonatomic.)

No matter which way you've done all this, you can use either .property syntax, or -setProperty: syntax:

self.name = @"Bob"; // This 
[self setName:@"Bob"]; // is the same as this

//likewise,

NSString *theName = self.name; // this
NSString *theName = [self name]; // is the same as this

You've gotten quite the discussion here based on what was initially a simple memory management bug. Please don't feel overwhelmed. Some of this is stuff that you'll only really fully understand when you've played around with it, written a fair amount of code, made mistakes and fixed them, etc.

 

[nashyo]

Thanks

Thanks for all of your responses. This definitely looks like something that's grasped from experience. It all seems very overwhelming at this point.

 

[Sydde]

Thanks for all of your responses. This definitely looks like something that's grasped from experience. It all seems very overwhelming at this point.

It really is quite simple. What I see you doing that is making it confusing is that you declare ivars in the interface context scope (within the braces) and then declare the same variable with @property. The @property declaration creates an ivar, @synthesize creates the accessors, so for clarity and conciseness, I would put the ivar declaration either within the interface context scope or in a @property statement, but not both (why be redundant?).

I think of it this way: variables declared with @property are basically public to any code that uses the object (since you will probably use @synthesize as well) while variables declared within the interface context scope are nominally private (you specifically design what kind of access to them your object allows). But that is just me, others may look at it differently.

 

[mduser63]

It really is quite simple. What I see you doing that is making it confusing is that you declare ivars in the interface context scope (within the braces) and then declare the same variable with @property. The @property declaration creates an ivar, @synthesize creates the accessors, so for clarity and conciseness, I would put the ivar declaration either within the interface context scope or in a @property statement, but not both (why be redundant?).

This isn't strictly true. With the modern Objective-C runtime you don't need to explicitly declare ivars, the @property declaration is enough. However, the modern runtime is only available on (64-bit 10.6+ and iOS. So if you want to target any 32-bit machines or OS versions older than 10.6, you can't omit ivar declarations. Of course, that's becoming less and less of a limitation as time goes on...

 

[Sydde]

This isn't strictly true. With the modern Objective-C runtime you don't need to explicitly declare ivars, the @property declaration is enough.

Sorry, I thought that is what I said? It seems clearer to me to either use @property or declare an ivar in the @interface { scope } but doing both seems redundant.

However, the modern runtime is only available on (64-bit 10.6+ and iOS. So if you want to target any 32-bit machines or OS versions older than 10.6, you can't omit ivar declarations. Of course, that's becoming less and less of a limitation as time goes on...

No, that is not correct. Objective-C 2.0 ( with @property/@synthesize ) debuted with Mac OS Leopard (10.5) and only EoLs from G3 and before.

 

[mduser63]

Sorry, I thought that is what I said? It seems clearer to me to either use @property or declare an ivar in the @interface { scope } but doing both seems redundant.

No, that is not correct. Objective-C 2.0 ( with @property/@synthesize ) debuted with Mac OS Leopard (10.5) and only EoLs from G3 and before.

The ability to do this relies on the modern Objective-C runtime:

@interface Something : NSObject
{
    // No declaration of a name ivar here
}

@property NSString *name;

@end

@implementation Something

@synthesize name;

@end

I misspoke earlier. The modern runtime was included with 10.5 Leopard. However, as I said it is limited to 64-bit apps. When using the legacy runtime (32-bit apps), you have to do this:

@interface Something : NSObject
{
    NSString *name; // Must explicitly declare this here
}

@property NSString *name;

@end

@implementation Something

@synthesize name;

@end

See this page for authoritative info: http://developer.apple.com/library/...timeGuide/Articles/ocrtVersionsPlatforms.html