iOS NSMutable Array contents Corrupted

[jmerkow]

I am writing a very simple iPhone app. I have a view with a number of objects.

right after initializing the array, it works fine. However, the contents seem to get corrupted when I call on the object later. I also get exec_bad_access even when I am within the range of the array.
Here is the initializer for the class with the object

-(SDGameState*) initWithMaxScore:(NSInteger) maxscore andMaxFace:(NSInteger) maxface{
	self = [super init];
	
	if (self) {
		self->maxScore = maxscore;
		self->currentScore = 0;
		self->DI = [[Dice alloc] initWithMaxFace: maxface];
		self->playerArray = [[[NSMutableArray alloc] init] retain];
		self->lastRoll = 1;
	}
	return self;
}

Here is how I am adding to the array. Note when reading the contents of the array at this point everything is working fine.

(BOOL) addPlayer:(IPlayer*) player{
	
	//TODO: Add Checks
	NSLog(@"Adding Player");
	[self->playerArray addObject:player];

	

	return YES;

Here is some time later when I try to print out the array in a function. The app crashes in the for-loop some of the time. But always outputs junk.

-(NSInteger) rollDice{
	NSInteger roll = [DI roll];
	NSLog(@"%d",[playerArray count]);
	for (int i = 0; i < [playerArray count]; i++) {
		NSLog(@"Internal Player %d: %@", i+1,[[playerArray objectAtIndex:i] name]);
	}
		NSLog(@"%d",[playerArray count]);
	//IPlayer *currentPlayer = [playerArray objectAtIndex:currentPlayerIndex];
	
	[[playerArray objectAtIndex:currentPlayerIndex] addRoll];

	[self addScore: roll];
	if ([self isGameOver]) {
		[[playerArray objectAtIndex:currentPlayerIndex]
		 addLoss];
	}
	
	//[playerArray replaceObjectAtIndex:currentPlayerIndex withObject:currentPlayer];
	self->lastRoll = roll;
	//[self beginNextTurn];
	
	return roll;
}

This is how im initializing the containing class. This is done in viewDidLoad

//-- game variables
	if (!gs) {
	gs = [[[SDGameState alloc] initWithMaxScore: kMaxScore andMaxFace: kMaxFace] retain];

What am I doing wrong here. Let me know if there is anything else I should post.

 

[chown33]

Your memory management is wrong.

When you alloc an object, you own it. Therefore, calling retain on it creates a second claim of ownership. The fact that you do this (retain an object you alloc'ed) strongly suggests you don't fully understand ownership and memory management.
http://developer.apple.com/library/ios/#documentation/cocoa/Conceptual/MemoryMgmt/MemoryMgmt.html

The inconsistent use of -> doesn't help your case, either. When used at all, it's idiosyncratic at best. When used in some places but not others, it's downright confusing. Where did you learn that from?

I also get exec_bad_access even when I am within the range of the array.

That error is often associated with an under-retain or an over-release. A cursory inspection shows neither one in the code you posted, so my first guess is that the cause of the problem is in code you haven't posted.

I recommend that you run your program under Instruments with zombies enabled.
http://developer.apple.com/library/...mentsUserGuide/AboutTracing/AboutTracing.html

 

[jmerkow]

I am not a native ObjC user. I am used to malloc and free more than auto release pools etc.

I can most more code related to the objects. I had retain taken out originally but I tried it to solve this problem. I haven't been able to find a decent explanation of those memory management properties. The object life-span in ObjC is a bit confusing to me. The code I've posted is essentially the only places I call anything that has to do with this array at all.

I will search through and find look for some more shortly.

 

[PhoneyDeveloper]

How do you call the addPlayer: method?

 

[jmerkow]

	if (!gs) {
	gs = [[[SDGameState alloc] initWithMaxScore: kMaxScore andMaxFace: kMaxFace]retain];
		for (int i = 1; i<6; i++) {
			[gs addPlayer:[[IPlayer alloc] initWithName:[NSString stringWithFormat:@"Player%d",i]]];
		}
		
	}
	
	for (int i = 0; i< [[gs playerArray] count]; i++) {
		NSLog(@"Player %d: %@", i+1,[[[gs playerArray] objectAtIndex:i] name]);
	}

 

[chown33]

I haven't been able to find a decent explanation of those memory management properties. The object life-span in ObjC is a bit confusing to me.

Read the Memory Management guide link I posted. It's quite clear.
If you have questions after reading it, then ask specific questions.

You will also need a debugging tool that helps you find memory problems.
That's why I pointed you to Instruments and zombies.

 

[PhoneyDeveloper]

Well, you have a lot of over-retains all over the place but I think you're under-retaining the player's name.

Anyway you need to read about the memory management rules and understand them better. The tools are also helpful but understanding the rules is more important at the start.

 

[jmerkow]

I will read the rules. Can you explain what you mean in this case though?

EDIT:

I changed it to this:

NSString *playerName = [[NSString alloc] initWithString:[NSString stringWithFormat:@"Player%d",i]];
			[gs addPlayer:[[IPlayer alloc] initWithName:playerName]];

So anything that I alloc I own and I don't need to claim ownership of? I think I like classic memory management better, haha.

 

[PhoneyDeveloper]

That's definitely not the right solution.

Memory management errors are usually in one of two flavors: over-retain, under-retain. One causes a crash the other causes a memory leak. It's not wise to trade one for the other.

 

[dejo]

I think I like classic memory management better, haha.

What do you mean "classic" memory management? Do you mean garbage-collection? If so, that's not as old as having to do your own memory management and, thus, IMHO, less-"classic".

 

[jmerkow]

Can you be more specific?

 

[PhoneyDeveloper]

Can you be more specific?

If that question's to me, yes I could be but you're shooting in the dark now. Go read the docs, reread what's been said in this thread, look at your code again. I think the answer's there.

 

[chown33]

NSString *playerName = [[NSString alloc] initWithString:[NSString stringWithFormat:@"Player%d",i]];
			[gs addPlayer:[[IPlayer alloc] initWithName:playerName]];

That's a bad design. It will probably lead to memory leaks.

The IPlayer instance should own its own instance variables, and manage their lifetime by itself. If it's not doing that, then you'd have to write a global superviser that manages every object's ownership. That global superviser then has to peek and poke inside every object's internals so it can deduce how to manage the lifetime of every object's internal object references. Why would you add that much complexity for what is basically the same model as malloc/free? If you're going to use malloc/free, then do it without using objects.

I think you need to write a few simpler programs (such as command-line programs) until you've mastered memory management. You should also probably spend some time with an actual tutorial or book, so you can get some guided practice with managing memory.

You should be able to write a simple program with no leaks (over-retains) and no under-retains or over-releases (crashes or zombies). If you haven't done that yet, then you're already in over your head and you need to stop coding and go review the fundamentals.

 

[jmerkow]

If that question's to me, yes I could be but you're shooting in the dark now. Go read the docs, reread what's been said in this thread, look at your code again. I think the answer's there.

I read the docs. I did what it explained, but that lead to the same problem. Please be more specific to this problem, I am working through the memory management docs. But they seem to be causing the same problem with with the aggregate collection of classes.

IPlayer does manage its own memory, (though it must be done incorrectly). My understanding is that once you add an object to a collection it increases your count automatically. So my problem is that IPlayer isn't allocating for the string name in its constructor?

So I should do something like this:

-(IPlayer *) initWithName:(NSString *)str{
	self = [super init];
	if (self){
		self->name = [[NSString alloc] initWithString: str];
		self->hasLife = YES;
	}
	
	return self;
	
}

EDIT:

What do you mean "classic" memory management? Do you mean garbage-collection? If so, that's not as old as having to do your own memory management and, thus, IMHO, less-"classic".

As far as embedded code I am more used to working directly with the registers/memory addresses. OO embedded design is a bit new to me. The complexity of the managment here is new since there are a lot of new keywords to understand. I am more used to using malloc/free or new/delete (In OO). The mapping of those concepts to retain,release,autorelease, etc is something Im still learning.

 

[chown33]

IPlayer does manage its own memory, (though it must be done incorrectly).

Then post its actual code. We can't debug descriptions.

So my problem is that IPlayer isn't allocating for the string name in its constructor?

Can't tell without seeing actual code.

So I should do something like this:

-(IPlayer *) initWithName:(NSString *)str{
	self = [super init];
	if (self){
		self->name = [[NSString alloc] initWithString: str];
		self->hasLife = YES;
	}
	
	return self;
	
}

That's one way, but this is simpler:

self->name = [str retain];

And you really ought to get rid of the C++'ism of self->foo.

I am more used to using malloc/free or new/delete (In OO).

Which language? It looks like C++, but C++ isn't the only OO language.

An OO design is the same regardless of language. An object has responsibility for itself and for its internal state.

 

[jmerkow]

And you really ought to get rid of the C++'ism of self->foo.

Ok. Will do.
Is it doing something that I'm not aware of?

An OO design is the same regardless of language. An object has responsibility for itself and for its internal state.

Yes but you don't need to allocate the same way you do in ObjC. Strings don't need to allocation beyond the constructing the class. I think I understand now.


Oh by the way chown, your explanation and sites were most helpful

 

[chown33]

Ok. Will do.
Is it doing something that I'm not aware of?

I think it's interfering with your understanding of Objective-C as a language that is distinctly different from C++. The less you treat it like C++, the better.

Yes but you don't need to allocate the same way you do in ObjC. Strings don't need to allocation beyond the constructing the class. I think I understand now.

Allocation is a responsibility of an object. Deallocation is another responsibility of an object. Some OO languages require the objects themselves to take direct responsibility. Others leave it up to something else, like a garbage collector or a block-scope mechanism.

The point here is that different OO languages do things differently, but the idea of making objects responsible for themselves and their own internal state crosses all OO languages. Contrast with structs in C, which hold state, but the responsibility for allocation, deallocation, etc. lies in functions which are separate from the struct.

There can even be different divisions of responsibilities in a single language. For example, Objective-C with garbage collection enabled does not place direct responsibility for lifetimes on the objects themselves. Too bad that feature's not available for iOS.

 

[jmerkow]

Yes agreed. It can get confusing when you first start out in ObjC and the iPhone SDK. I have a better handle on how iOS handles memory management now. This is my first iPhone app and I am making it just to learn the basics of al the SDK and ObjC. Thanks for your help.