Reputation: 351
I'm making an iPhone app in cocos2d and I've been having a bit of trouble with the leaks instrument flagging up an NSMutableArray that my app is apparently leaking. I've fixed the issue, but I don't really understand why it occurred in the first place so hopefully someone can explain it to me.
I've subclassed CCParticleSystemQuad so that I can add a few instance variables, including an NSMutable Array called 'damagedObjects':
@interface SonicBoom : CCParticleSystemQuad{
NSMutableArray *damagedObjects;
HelloWorldLayer *gameClass;
CGPoint radiusPoint;
CGPoint origin;
}
@property(nonatomic, retain) NSMutableArray *damagedObjects;
@property(nonatomic, retain) HelloWorldLayer *gameClass;
@property CGPoint radiusPoint;
@property CGPoint origin;
@end
This gets initialised and the damagedObjects array gets allocated in the main game class, the particle system is removed on completion by setting the autoRemoveOnFinish property:
-(void)createSonicBoom{
sonicBoom = [SonicBoom particleWithFile:@"SonicBoom.plist"];
sonicBoom.damagedObjects = [[NSMutableArray alloc]init];
sonicBoom.gameClass = self;
sonicBoom.autoRemoveOnFinish = YES;
//etc..........
I have then overridden the dealloc method of the 'SonicBoom' class to also release the 'damagedObjects' array:
-(void)dealloc{
NSLog(@"Removing SonicBoom");
NSLog(@"damaged objects retain count = %i", damagedObjects.retainCount);
gameClass.sonicBoomActive = NO;
[damagedObjects removeAllObjects];
[damagedObjects release];
[damagedObjects release];
[super dealloc];
}
For some reason, with just one release message to the array I was getting a leak. I checked the retain count (I never bother thinking about these usually), and it was 2, so I'm now sending the release message twice and this seems to have cured the problem.
Is this the best way to go about releasing it, and can someone explain why this would be required?
Thanks
Upvotes: 1
Views: 235
Reputation: 43330
This line:
sonicBoom.damagedObjects = [[NSMutableArray alloc]init];
Is the problem. Roughly put, here's what the compiler expands that out to:
[sonicBoom setDamagedObjects:[[[NSMutableArray alloc]init]retain]];
sonicBoom, by having it's damagedObjects
property use a retain qualifier, tries to stake a claim to the array that you've created in your method, incrementing it's retain count by 1 over the 1 that is already inherently returned by the alloc and init pair. (alloc eventually calls
Thus, you have two references to the array because you're not following standard cocoa memory management guidelines or MVC. Either autorelease the array or use the convenience constructor [NSMutableArray array];
(which autoreleases for you because cocoa allows only a strict subset of methods to return +1 references to objects) . Better yet, make the sonic boom object create its own array, so that it "owns" it memory-wise.
Edit (for those who feel I have provided an insufficient level of detail <cough>).
retain
as a memory qualifier in a Manual Reference Counting environment for Objective-C objects (specifically those that descend from NSObject, or NSProxy) with a standard, compiler generated setter through the use of the @synthesize directive, consists of a standard set of calls which may or may not appear in the following form (the general concept of how retain counts are balanced in the generated setter is nearly the identical to the pseudo-code below):
- (void)setMyObject:(NSObject*)newMyObject {
[_myObject release];
_myObject = [newMyObject retain];
}
Of course, this is an nonatomic
(able to be interrupted by another thread) variation of the setter. The atomic
version, is implemented very similarly to
- (void)setMyObject:(NSObject*)newMyObject {
@synchronized(self) {
[_myObject release];
_myObject = [newMyObject retain];
}
}
Obviously, these omit the Key-Value-Coding mechanism inherent in Cocoa setters, but those operators are not generally exposed to the public by Apple, and are out of bounds for a subject such as memory management, so it’s implementation is left as an exercise to the reader.
We can, however, take a closer look at memory management by evaluating the calls the Open Source version of NSObject puts out*.
*Note that this version of NSObject corresponds to the version present in the Mac OS X SDK, and therefore, not all underlying implementations of NSObject will be guaranteed to match what is provided.
-retain
, as implemented by the latest open source version, at the time of this writing, of NSObject (runtime version 532, rev. 2), calls down to 3 individual internal constructs, one after another, should the previous one fail, finally ending in a "slow retain" of the root NSObject. It is important to note: NSObject is implemented in Objective-C++, with gratuitous calls to the internal LLVM library. Such is where our analysis of NSObject will end should it be encountered.
-retain
is implemented, like all root Objective-C memory management calls, as a 16-byte aligned method returning the object itself upon success. According to NSObject.mm, retain looks like this:
- (id)retain __attribute__((aligned(16))) {
if (OBJC_IS_TAGGED_PTR(self)) return self;
SideTable *table = SideTable::tableForPointer(self);
if (OSSpinLockTry(&table->slock)) {
table->refcnts[DISGUISE(self)] += 2;
OSSpinLockUnlock(&table->slock);
return self;
}
return _objc_rootRetain_slow(self);
}
*retain is replaced with ObjectAlloc when the appropriate flag is passed at launch for easier debugging. An analysis of ObjectAlloc is not provided at this time.
To examine each in pieces, it is also necessary to access the file objc-private.h
, which is also freely along with the NSObject version tagged in this post in the same directory.
To begin, retain checks whether the pointer has been tagged. Of course, a tag could mean anything, but to NSObject, it means whether or not the pointer contains the address of 0x1 in it’s last bit (if you’ll recall, because of 16 byte alignments, all types except char* have addresses that are guaranteed by Mac OS X to have a 0 at the end of their addresses). Thus, OBJC_IS_TAGGED_PTR(PTR)
expands out to
((uintptr_t)(PTR) & 0x1)
If the pointer is tagged, NSObject takes the easy way out and simply returns itself (because usually tagged pointers indicate invalid addresses).
Next, -retain
tries a spin lock (note that OS
-prefixed methods are unavailable on iOS) on the table for the given pointer to self. Tables, in the sense of NSObject, are what keep track of the retain count of the object. They are very simple C++ classes allocated along with the root NSObject. An interesting trick lies in that DISCGUISE(x)
macro. It expands out to:
#define DISGUISE(x) ((id)~(uintptr_t)(x))
If you’ll notice, it’s flipping the pointer of the given object. I can only assume that this is done to hide SideTable
’s double increment of the reference count of the object on the next line from instruments, as any object with a doubled retain count from one call could likely be seen as undefined behavior (when -release
is sent, so to is the SideTable told to decrement it’s retain count by 2). The reference count is increased by two so as to keep the lowest bit of the address available to check if the object is in the process of being deallocated (which again, circles back around to tagged pointers). If all goes well, then the spin lock is released, and NSObject returns itself.
If the spin-lock happens to be unavailable for the taking, NSObject resorts to what it calls a “slow retain” (because the process of locking and unlocking the SideTable
’s spin lock is mildly expensive), in which case the same process occurs, but NSObject steals and locks down the spin lock for it’s SideTable
, increments it’s reference count by 2, then unlocks the spin lock. The entire process is represented by one C-function _objc_rootRetain_slow
, which looks like this:
id _objc_rootRetain_slow(id obj) {
SideTable *table = SideTable::tableForPointer(obj);
OSSpinLockLock(&table->slock);
table->refcnts[DISGUISE(obj)] += 2;
OSSpinLockUnlock(&table->slock);
return obj;
}
Upvotes: 3
Reputation: 4558
It's happening because of this line:
sonicBoom.damagedObjects = [[NSMutableArray alloc]init];
The init increases the reference count by 1, and then setting the retained property also increases it.
Change it to:
NSMutableArray *array = [[NSMutableArray alloc]init];
sonicBoom.damagedObjects = array;
[array release];
Alternatively you could have used:
sonicBoom.damagedObjects = [NSMutableArray array];
Which returns an autoreleased object, and the only object your class owns is the one it retained with the setter.
Also, FWIW, fixing a leak by releasing something twice in dealloc is definitely not a good idea. If one day you decided to set damagedObjects
using some other method that was returning an autoreleased array, your app would start crashing and tracking down crashes like that can be a pain.
Upvotes: 5
Reputation: 33592
When not using ARC (or writing code that can be used in ARC or non-ARC mode), I prefer the following style:
In -createSonicBoom,
sonicBoom.damagedObjects = [NSMutableArray array]; // or e.g. arrayWithCapacity:10?
In dealloc,
self.damagedObjects = nil;
There's a minor gotcha: If you've overridden the setter method to do something clever, it might do the wrong thing in -dealloc
.
Upvotes: 2