CCNode retained by CCCallBlock after stopAllActions is called

Question

I have a utility function that I use to move a CCNode in a circular path either for a full circle, or a partial circle.

The function works really well, but if I want the CCNode to continually follow the path, which I do via a passed in Block that ends up calling the same function (sort of recursive, but not really).

The problem I'm finding is that because the function makes use of blocks internally, the CCNode on which the actions are being run is retained and even after calling stopAllActions or removeFromParentAndCleanup:YES, even though the CCNode is cleaned up, and removed from the screen, it remains in memory and is not dealloc'd. This then appears to affect performance even though the node is not being displayed as the CCNode and other dependents are still (somehow) in the cocos2d system.

Here's the function that moves the CCNode:

@interface CocosUtil : NSObject {

}

typedef void (^NodeCompletionBlock)(CCNode *sprite);

+ (void) moveOperand:(CCNode*)operand throughCircleWithCentre:(CGPoint)centreOfElipse
     startingDegrees:(float)startingDegrees
     endingAtDegrees:(float)endingDegrees
      startingRadius:(float)startingRadius
        endingRadius:(float)endingRadius
 withInitialDuration:(ccTime)initialDuration
    withMainDuration:(ccTime)duration
           clockwise:(BOOL)clockwise
     completionBlock:(NodeCompletionBlock)handler;

@end


@implementation CocosUtil

+ (float) angleFromDegrees:(float)deg {
    return fmodf((450.0 - deg), 360.0);
}

// Calculates the angle from one point to another, in radians.
//
+ (float) angleFromPoint:(CGPoint)from toPoint:(CGPoint)to {
    CGPoint pnormal = ccpSub(to, from);
    float radians = atan2f(pnormal.x, pnormal.y);

    return radians;
}

+ (CGPoint) pointOnCircleWithCentre:(CGPoint)centerPt andRadius:(float)radius atDegrees:(float)degrees {
    float x = radius + cos (CC_DEGREES_TO_RADIANS([self angleFromDegrees:degrees])) * radius;
    float y = radius + sin (CC_DEGREES_TO_RADIANS([self angleFromDegrees:degrees])) * radius;
    return ccpAdd(centerPt, ccpSub(CGPointMake(x, y), CGPointMake(radius, radius)));
}

+ (void) moveOperand:(CCNode*)operand throughCircleWithCentre:(CGPoint)centreOfElipse
     startingDegrees:(float)startingDegrees
     endingAtDegrees:(float)endingDegrees
      startingRadius:(float)startingRadius
        endingRadius:(float)endingRadius
 withInitialDuration:(ccTime)initialDuration
    withMainDuration:(ccTime)duration
           clockwise:(BOOL)clockwise
     completionBlock:(NodeCompletionBlock)handler {
    float range;

    if (clockwise == YES) {
        if (endingDegrees <= startingDegrees) {
            range = (360.0 + endingDegrees) - startingDegrees;
        } else {
            range = endingDegrees - startingDegrees;
        }
    } else {
        if (endingDegrees >= startingDegrees) {
            range = (360.0 + startingDegrees) - endingDegrees;
        } else {
            range = startingDegrees - endingDegrees;
        }
    }

    __block float degrees = startingDegrees;
    __block float radius = startingRadius;

    const float incrementAngle = 10.0;

    float intervals = (range / incrementAngle) - 1;

    ccTime interval = duration / intervals;
    float radiusStep = (endingRadius - startingRadius) / intervals;

    if (clockwise == YES) {
        degrees += incrementAngle;
    } else {
        degrees -= incrementAngle;
    }

    radius += radiusStep;

    __block void (^moveToNextPoint)();

    moveToNextPoint = [^(){
        if (fabsf(degrees - endingDegrees) < 1.0) {
            [operand runAction:[CCSequence actions:
                                [CCEaseBounceOut actionWithAction:
                                 [CCMoveTo actionWithDuration:interval position:[self pointOnCircleWithCentre:centreOfElipse andRadius:radius atDegrees:degrees]]],
                                [CCCallBlock actionWithBlock:
                                 ^{
                                     if (handler != nil) {
                                         handler(operand);
                                     }
                                 }],
                                nil]];
        } else {
            [operand runAction:[CCSequence actions:
                                [CCMoveTo actionWithDuration:interval position:[self pointOnCircleWithCentre:centreOfElipse andRadius:radius atDegrees:degrees]],
                                [CCCallBlock actionWithBlock:moveToNextPoint],
                                nil]];

            if (clockwise == YES) {
                degrees += incrementAngle;

                if (degrees > 360.0) {
                    degrees = degrees - 360.0;
                }
            } else {
                degrees -= incrementAngle;

                if (degrees < 0.0) {
                    degrees = degrees + 360.0;
                }
            }

            radius += radiusStep;
        }
    } copy];

    [operand runAction:[CCSequence actions:
                        [CCMoveTo actionWithDuration:initialDuration position:[self pointOnCircleWithCentre:centreOfElipse andRadius:startingRadius atDegrees:startingDegrees]],
                        [CCCallBlock actionWithBlock:moveToNextPoint],
                        nil]];

}

@end

You'll note that the arc that the node is moved through is broken into 10 degree steps. This is done to get a circular motion without writing a CCActionInterval subclass, but it means using blocks or selectors to keep the motion running until completion.

Now, to get my CCNode to continually move through a full circle, I call this function using:

- (void) moveLayer:(CCNode*)layer
startingDegrees:(float)startingDegrees
endingAtDegrees:(float)endingDegrees
startingRadius:(float)startingRadius
endingRadius:(float)endingRadius
withInitialDuration:(ccTime)initialDuration
withMainDuration:(ccTime)duration
clockwise:(BOOL)clockwise {
    [CocosUtil moveOperand:layer throughCircleWithCentre:CGPointMake(240.0, 160.0) startingDegrees:startingDegrees endingAtDegrees:endingDegrees startingRadius:startingRadius endingRadius:endingRadius withInitialDuration:initialDuration withMainDuration:duration clockwise:clockwise completionBlock:^(CCNode *sprite) {
        [self moveLayer:layer startingDegrees:startingDegrees endingAtDegrees:endingDegrees startingRadius:startingRadius endingRadius:endingRadius withInitialDuration:initialDuration withMainDuration:duration clockwise:clockwise];
    }];
}

I've tried a few different things, like not passing in a block at all, but nothing prevents the retain except not using the function at all.

From what I can tell, and reading in the XCode doco, we have:

"If you use a block within the implementation of a method, the rules for memory management of object instance variables are more subtle:

If you access an instance variable by reference, self is retained; If you access an instance variable by value, the variable is retained."

So this tells me that by using the block within my function in the way I am is causing the hidden retain.

A later call to stopAllActions doesn't trigger a release.

The only way this works for me is if, in my node's cleanup() message, I add [self release].

I don't like this as it's decoupled from the code doing the retain.

One new thought I've had is to rewrite it somehow as a new CCActionInterval subclass, but I'm still unsure if that will fix the problem.

Any suggestions?

Answer 1

OK. Taking the advice from @LearnCocos2D, and doing what I was already thinking of doing here is my solution for others that may want it.

Basically, I rewrote the entire thing as a relatively simple CCActionInterval subclass.

There are no longer any blocks involved at all, and thus, no hidden retains. Much cleaner, and much, much more elegant (I think).

The interface:

#import "cocos2d.h"

@interface CCMoveThroughArc : CCActionInterval

/** creates the action */
+(id) actionWithDuration:(ccTime)duration
                  centre:(CGPoint)centreOfCircle
       startingAtDegrees:(float)startingDegrees
         endingAtDegrees:(float)endingDegrees
        startingAtRadius:(float)startingRadius
          endingAtRadius:(float)endingRadius;

/** creates the action */
+(id) actionWithDuration:(ccTime)duration
                  centre:(CGPoint)centreOfCircle
       startingAtDegrees:(float)startingDegrees
         endingAtDegrees:(float)endingDegrees
        startingAtRadius:(float)startingRadius
          endingAtRadius:(float)endingRadius
                reversed:(BOOL)reversed;

/** initializes the action */
-(id) initWithDuration:(ccTime)duration
                centre:(CGPoint)centreOfCircle
     startingAtDegrees:(float)startingDegrees
       endingAtDegrees:(float)endingDegrees
      startingAtRadius:(float)startingRadius
        endingAtRadius:(float)endingRadius
              reversed:(BOOL)reversed;

@end

The implementation:

#import "CCMoveThroughArc.h"


@implementation CCMoveThroughArc {

    CGPoint centre_;

    float startingDegrees_;
    float endingDegrees_;
    float diffDegrees_;

    float startingRadius_;
    float endingRadius_;
    float diffRadius_;

    BOOL reversed_;
}

/** creates the action */
+(id) actionWithDuration:(ccTime)duration
                  centre:(CGPoint)centreOfCircle
       startingAtDegrees:(float)startingDegrees
         endingAtDegrees:(float)endingDegrees
        startingAtRadius:(float)startingRadius
          endingAtRadius:(float)endingRadius {
    return [[self alloc] initWithDuration:duration centre:centreOfCircle startingAtDegrees:startingDegrees endingAtDegrees:endingDegrees startingAtRadius:startingRadius endingAtRadius:endingRadius reversed:NO];
}

/** creates the action */
+(id) actionWithDuration:(ccTime)duration
                  centre:(CGPoint)centreOfCircle
       startingAtDegrees:(float)startingDegrees
         endingAtDegrees:(float)endingDegrees
        startingAtRadius:(float)startingRadius
          endingAtRadius:(float)endingRadius
                reversed:(BOOL)reversed {
    return [[self alloc] initWithDuration:duration centre:centreOfCircle startingAtDegrees:startingDegrees endingAtDegrees:endingDegrees startingAtRadius:startingRadius endingAtRadius:endingRadius reversed:reversed];
}

/** initializes the action */
-(id) initWithDuration:(ccTime)duration
                centre:(CGPoint)centreOfCircle
     startingAtDegrees:(float)startingDegrees
       endingAtDegrees:(float)endingDegrees
      startingAtRadius:(float)startingRadius
        endingAtRadius:(float)endingRadius
              reversed:(BOOL)reversed {
    if( (self=[super initWithDuration:duration]) ) {
        centre_ = centreOfCircle;

        startingDegrees_ = fminf(startingDegrees, endingDegrees);
        endingDegrees_ = fmaxf(startingDegrees, endingDegrees);

        startingRadius_ = startingRadius;
        endingRadius_ = endingRadius;

        reversed_ = reversed;

        diffDegrees_ = endingDegrees_ - startingDegrees_;
        diffRadius_ = endingRadius_ - startingRadius_;
    }

    return self;
}

-(id) copyWithZone: (NSZone*) zone
{
    CCAction *copy = [[[self class] allocWithZone: zone] initWithDuration:[self duration] centre:centre_ startingAtDegrees:startingDegrees_ endingAtDegrees:endingDegrees_ startingAtRadius:startingRadius_ endingAtRadius:endingRadius_ reversed:reversed_];

    return copy;
}

-(CCActionInterval*) reverse
{
    return [[self class] actionWithDuration:duration_ centre:centre_ startingAtDegrees:startingDegrees_ endingAtDegrees:endingDegrees_ startingAtRadius:startingRadius_ endingAtRadius:endingRadius_ reversed:!reversed_];
}

-(void) startWithTarget:(CCNode *)aTarget
{
    NSAssert1(([aTarget isKindOfClass:[CCNode class]] == YES), @"CCMoveThroughArc requires a CCNode as target.  Got a %@", [[aTarget class] description]);

    [super startWithTarget:aTarget];
}

- (float) angleFromDegrees:(float)deg {
    return fmodf((450.0 - deg), 360.0);
}

- (CGPoint) pointOnCircleWithCentre:(CGPoint)centerPt andRadius:(float)radius atDegrees:(float)degrees {
    float x = radius + cos (CC_DEGREES_TO_RADIANS([self angleFromDegrees:degrees])) * radius;
    float y = radius + sin (CC_DEGREES_TO_RADIANS([self angleFromDegrees:degrees])) * radius;
    return ccpAdd(centerPt, ccpSub(CGPointMake(x, y), CGPointMake(radius, radius)));
}

-(void) update:(ccTime) t {

    float angle;
    float radius;

    if (reversed_ == NO) {
        angle = (diffDegrees_ * t) + startingDegrees_;
        radius = (diffRadius_ * t) + startingRadius_;
    } else {
        angle = endingDegrees_ - (diffDegrees_ * t);
        radius = (diffRadius_ * (1.0 - t)) + startingRadius_;
    }

    CGPoint pos = [self pointOnCircleWithCentre:centre_ andRadius:radius atDegrees:angle];
    [(CCNode*)target_ setPosition:pos];
}

@end

And the code that uses it:

- (void) moveStartingAtDegrees:(float)startingDegrees
               endingAtDegrees:(float)endingDegrees
                startingRadius:(float)startingRadius
                  endingRadius:(float)endingRadius
           withInitialDuration:(ccTime)initialDuration
              withMainDuration:(ccTime)duration {

    [self runAction:[CCRepeatForever actionWithAction:
                     [CCMoveThroughArc actionWithDuration:duration centre:CGPointZero startingAtDegrees:startingDegrees endingAtDegrees:endingDegrees startingAtRadius:startingRadius endingAtRadius:endingRadius reversed:YES]]];
}

I hope that this is of some use to others. It certainly helped me. It doesn't truly answer my actual question, but it deals with the problem behind the question very effectively.

Answer 2

This code is convoluted. Not really a "helper".

What I see is an unnecessary copy of the moveToNextPoint block.

I also see several possibilities for really nasty issues, one of them is probably the cause of the retain. I don't see how to fix it without significantly refactoring this code.

One problem is the operand. It's used inside the (copied) block, which will retain operand. But worse, the operand itself runs a sequence using the moveToNextPoint block, the very same you're (re)assigning at that moment. I didn't know that was legal. Furthermore the callblock action will also copy the block, further complicating matters because that may also retain operand.

Since the context of this method is a class method of an unrelated class, and not the operand itself, I suspect that also plays a role.

You also pass in a handler block, which is used in a CCCallBlock action block that will be copied, which means the handler block is also retained at least for the duration of the sequence.

Long story short, this code violates several memory management guidelines and the use of several interleaved blocks makes it difficult to debug. The list of parameters for this method alone is a code smell: it is doing way too many different things all at once.

Take it apart, start over. Consider where you need to do what at every step. Who owns which block, which reference. Test every step for retain cycles/leaks. Ask yourself: does it really need to use this many interleaved blocks? Can you make it more sequential? Who should run which code? Perhaps a CCNode category for some parts makes more sense.