Objective C Help - Encoding

Discussion in 'Mac Programming' started by nashyo, Dec 18, 2011.

  1. macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #1
    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.


    Code:
    #import <Foundation/Foundation.h>
    
    @interface Employee : NSObject {
    @private
        NSString *name;
        int grade;
    }
      
    @property (retain) NSString *name;
    @property int grade;
    
    
    @end

    Code:
    #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

    Code:
    #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;
    
    }
     
  2. macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #2
    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
     
  3. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #3
    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'.

    [​IMG]
     
  4. macrumors 6502a

    Joined:
    Jan 23, 2010
    Location:
    San Diego, CA USA
    #4
    Add a 'retain' when you pull the name out of the archiver.
     
  5. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #5
    Still not working...

    This doesn't seem to solve the issue.
     
  6. macrumors 603

    Joined:
    Aug 9, 2009
    #6
    Show your revised code.
     
  7. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #7
    .
     
  8. macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #8
    I bet you can get bob to crash by changing your init method to this
    Code:
    -(id) init 
    {
       // [super init];
        return self;
    }
    
     
  9. mduser63, Dec 18, 2011
    Last edited: Dec 18, 2011

    macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #9
    Without actually running the code, this looks like the problem to me:

    Code:
    -(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:

    Code:
    [self setName:[coder decodeObjectForKey:@"EMPname"]];
    
    or retain it (or copy it) when you assign to the name ivar:
    Code:
    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.
     
  10. macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #10
    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.
     
  11. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #11
    The penny drops

    Thanks for all your responses.

    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!
     
  12. mduser63, Dec 18, 2011
    Last edited: Dec 18, 2011

    macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #12
    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.

    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:

    Code:
    #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
     
  13. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #13
    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.
     
  14. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #14
    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?
     
  15. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #15
    The name ivar is also defined with "@property (retain)" and synthesized. The synthesis creates code that looks something like this

    Code:
    - (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.
     
  16. macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #16
    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:

    Code:
    - (void)dealloc
    {
        [name release];
        
        [super dealloc];
    }
    
     
  17. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #17
    I can understand the following quote, but I have some fundamental questions to ask about it.

    Code:
    - (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?

    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')

    Code:
    - (void) setName: (NSSTring*) newName;
    and the following for the implementation (instead of @Synthesize).

    Code:
    - (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
     
  18. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #18
    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.
    Yes.
    Yes
    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.
    Code:
    - (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.
     
  19. macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #19
    Probably more like
    Code:
    - (void) setName: (NSString *)newName {
        [newName retain];
        [name release];
        name = newName;
    }
    
    On the off chance that (name == newName) the original code could ruin the property.
     
  20. macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #20
    I was referring to cases where you have:

    Code:
    @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).

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

    Code:
    @property (nonatomic, retain) NSString *name;
    is exactly equivalent to doing this (the complier effectively inserts this for you when you do the above):

    Code:
    - (NSString *)name;
    - (void)setName:(NSString *)name;
    Independently, in your .m file, you can replace this:

    Code:
    @synthesize name;
    with this:

    Code:
    - (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:

    Code:
    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.
     
  21. thread starter macrumors 6502

    nashyo

    Joined:
    Oct 1, 2010
    Location:
    Bristol
    #21
    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.
     
  22. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #22
    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.
     
  23. macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #23
    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...
     
  24. macrumors 68000

    Sydde

    Joined:
    Aug 17, 2009
    #24
    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.
     
  25. macrumors 68040

    mduser63

    Joined:
    Nov 9, 2004
    Location:
    Salt Lake City, UT
    #25
    The ability to do this relies on the modern Objective-C runtime:

    Code:
    @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:

    Code:
    @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
     

Share This Page