views:

230

answers:

2

I have an instance variable *TangramLevel(:UIView) currentLevel; in my viewController class, and I have an instance allocated at start (it's retainCount 1). I attached it to the view [self.view addSubview:currentLevel]; (retainCount 2).

When the level finishes, it notifies the controller, so controller removes it from the view [currentLevel removeFromSuperview]; (retainCount 1), and release the allocated instance - [currentLevel release]; -, to have it deallocated (retainCount 0 = invoke dealloc).

Then on the next line, controller wants to allocate/addSubview a new level instance with another level data, but the application crashes (EXEC BAD ACCESS).

If I don't release currentLevel after removeFromSuperview, the appliaction works, but I have an unused level instance left in memory then, which is also a problem (the main problem itself).

Is there any bug in the method I wrote above? Or the bug is elsewhere, maybe in the level class? I allocated some UIImageView in the level, but I release every allocated object in the levels dealloc method. Any ideas?

+1  A: 

Post your code.

This is definitely a memory management issue. The question is "where is the problem created?" To answer that, we need to examine the following:

  1. Your "currentLevel" iVar handling code (do you used synthesized properties, etc.). Post it.
  2. How are you assigning the view to currentLevel?
  3. Where are you releasing this, specifically?
  4. How is your view's dealloc implemented (what do you release and how)?
  5. Is there any other code that retains/releases this view or anything related to it?

The fact that you're calling release in your "I'm done with this level, let's swap in the next" code suggests an overall design issue. Make the memory management of each of a class's iVars the responsibility of its accessors and ONLY use the accessors to interact with it (even from within the class/instance). With synthesized properties, this makes it brain-dead-simple. That way you don't have to worry about where to retain/release iVars because it's always funneled through the accessors.

Joshua Nozzi
The actual code is quiet long, I wont post it all. I think I'm gonna shorten it as long as it produces the bug. It would be a pleasure, if you'd examine it. I use C-style array of CGPoints, could it be responsible for crash?
Geri
Re: CGPoints + C array, I don't know - you haven't posted your code yet. ;-)
Joshua Nozzi
Rethink of the whole design-pattern is something I really would appreciate. My previous question was just about that: http://stackoverflow.com/questions/1769430/looking-for-concept-for-managing-game-level-views-level-selection-views-prefere
Geri
Could you take a look on it? It was "self-answered", maybe I'm wrong.
Geri
A: 

This is not an answer. Just a response to "Post your code". Hope it wont be too long.

viewController.h

#import <UIKit/UIKit.h>
#import <Foundation/Foundation.h>
#import "BGTangramClues.h";
#import "BGTangramLevel.h";

@interface compactTangramViewController : UIViewController
{
    //The level.
    BGTangramLevel *currentLevel;
    int levelStepper;
}

//It is to be just a reference, therefore I use assign here.
@property (nonatomic, assign) BGTangramLevel *currentLevel;

-(void) notificationHandler: (NSNotification*) notification;
-(void) finishedCurrentLevel;

@end

viewController.m

#import "compactTangramViewController.h"

@implementation compactTangramViewController
@synthesize currentLevel;

//Initializer functions, setting up view hierarchy.
-(void) viewDidLoad
{   

    //Set up levelstepper.
    levelStepper = 1;

    //Set up "state" classes.
    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(notificationHandler:) name:@"finishedCurrentLevel" object:nil]; 

    //Attach level with clue 1/1.
    currentLevel = [BGTangramLevel levelWithClue: [BGTangramClues clueFromPack:1 level:levelStepper] frame:self.view.frame];
    [self.view addSubview:currentLevel];

    [super viewDidLoad];

}

//Release objects.
-(void) dealloc
{
    [currentLevel release];
    [super dealloc];
}

//Notification handling.
-(void) notificationHandler: (NSNotification*) notification
{
    //Execute kill current level.
    if ([notification name] == @"finishedCurrentLevel") [self finishedCurrentLevel];
}

-(void) finishedCurrentLevel
{
    //Remove - and release (dealloc) - previous clue.
    [currentLevel removeFromSuperview];
    [currentLevel release]; 

    //Step level.
    if (levelStepper == 1) levelStepper =2; else levelStepper = 1;

    //Attach level with clue 1/2.
    currentLevel = [BGTangramLevel levelWithClue: [BGTangramClues clueFromPack:1 level:levelStepper] frame:self.view.frame];
    [self.view addSubview:currentLevel];

}
@end

BGTangramClues.h

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

//A "semi-abstract" class to have Point object.
@interface BGPoint : NSObject
{
    float xCoord;
    float yCoord;
}
@property float xCoord;
@property float yCoord;
//Returns an autorelease-instance of BGPoint class.
+(BGPoint*) pointWithX: (float) sentX andY: (float) sentY;
@end

//A clue object can be returned from the "clue-library".
@interface BGTangramClue : NSObject
{
    NSArray *cluePolyVertices;
}
@property (nonatomic, assign) NSArray *cluePolyVertices;
//Returns an autorelease-instance of BGTangramClue class.
+(BGTangramClue*) clueWithPolyVertices: (NSArray*) sentPolyVertices;
@end

//The clue library.
@interface BGTangramClues : NSObject { }
+(BGTangramClue*) clueFromPack: (int) requestedPack level: (int) requestedLevel;
@end

BGTangramClues.m

#import "BGTangramClues.h"

@implementation BGPoint
@synthesize xCoord, yCoord;
+(BGPoint*) pointWithX: (float) sentX andY: (float) sentY
{
    BGPoint *allocatedPoint = [[BGPoint alloc] init];
    [allocatedPoint autorelease];
    allocatedPoint.xCoord = sentX;
    allocatedPoint.yCoord = sentY;
    return allocatedPoint;
}
@end

@implementation BGTangramClue
@synthesize  cluePolyVertices;
//Allocating.
+(BGTangramClue*) clueWithPolyVertices: (NSArray*) sentPolyVertices
{
    BGTangramClue *allocatedClue = [[BGTangramClue alloc] init];
    [allocatedClue autorelease];
    allocatedClue.cluePolyVertices = sentPolyVertices;
    return allocatedClue;
}
@end

@implementation BGTangramClues
+(BGTangramClue*) clueFromPack: (int) requestedPack level: (int) requestedLevel;
{   

    //Will receive an autorelease flag from the class method.
    BGTangramClue *currentClue;

    switch (requestedPack)
    {

//--------------------
//-------------------- CluePack 1
//--------------------
case 1 :

    switch (requestedLevel)
    {

    case 1 :
    //-------------------- Clue 1. The logo "T".

     currentClue = [BGTangramClue 
     clueWithPolyVertices: [NSArray arrayWithObjects:

      //Poly 1.
      [NSArray arrayWithObjects:
      [BGPoint pointWithX: 40 andY: 150],
      [BGPoint pointWithX: 280 andY: 150],
      [BGPoint pointWithX: 280 andY: 210],
      [BGPoint pointWithX: 220 andY: 210],
      [BGPoint pointWithX: 220 andY: 330],
      [BGPoint pointWithX: 100 andY: 330],
      [BGPoint pointWithX: 100 andY: 210],      
      [BGPoint pointWithX: 40 andY: 210],      
      nil], //Poly 1 cap.

      nil]];//Polygons cap.

    //-------------------- End of clue
    break;

    case 2 :
    //-------------------- Clue 2. Inverse Arrow.

    currentClue = [BGTangramClue 
     clueWithPolyVertices: [NSArray arrayWithObjects:

      //Poly 1.
      [NSArray arrayWithObjects:
      [BGPoint pointWithX: 70 andY: 150],
      [BGPoint pointWithX: 250 andY: 150],
      [BGPoint pointWithX: 250 andY: 320],
      [BGPoint pointWithX: 165.148 andY: 235.148],
      [BGPoint pointWithX: 190 andY: 210],
      [BGPoint pointWithX: 130 andY: 210],
      [BGPoint pointWithX: 130 andY: 270],      
      [BGPoint pointWithX: 154.852 andY: 245.148], 
      [BGPoint pointWithX: 240 andY: 330],
      [BGPoint pointWithX: 70 andY: 330],    
      nil], //Poly 1 cap.     

      nil]];//Polygons cap.

    //-------------------- End of clue
    break;

    } //CluePack cap.

//-------------------- End of CluePack 1
//--------------------   
break;

} //Cleapacks cap.   

//Send selected clue instance.
return currentClue;

}
@end

BGTangramLevel.h

#import <UIKit/UIKit.h>
#import <Foundation/Foundation.h>
#import "BGTangramClues.h"

@interface BGTangramLevel : UIView {

    //The screens graphic context.
    CGContextRef screenContext;

    //"Local" C-Style Clue.
    int cluePolysAmount; //Max 7 of course.  
    int cluePolyVerticesAmount[7]; //Max 23 per clue poly.   
    CGPoint cluePolyVertices[7][23]; //The "worst" case is 5*3+2*4 = 23 vertex (maybe smaller). 

    //"Listener" flags.
    BOOL puzzleCompleted; 

}

//Initializers.
+(id)levelWithClue: (BGTangramClue*) clue frame: (CGRect) frame;
-(id)initWithClue: (BGTangramClue*) sentClue frame:(CGRect)frame;

//Test if the puzzle is completed.
-(void) isSolved;

@end

BGTAngramLevel.m

#import "BGTangramLevel.h"

#define CYCLE_CLUE_POLYS for (int cluePolyIndex = 0; cluePolyIndex <= cluePolysAmount-1; cluePolyIndex++)

@implementation BGTangramLevel

//Allocated instance.
+(id)levelWithClue: (BGTangramClue*) clue frame: (CGRect) frame
{
    BGTangramLevel *allocatedLevel = [[BGTangramLevel alloc] initWithClue: clue frame:frame];
    return allocatedLevel;
}

-(id)initWithClue: (BGTangramClue*) clue frame:(CGRect)frame
{
    if (self=[self initWithFrame:frame])
    {

    //Copy the point data into the C-arrays.

     //Count polygons.
     cluePolysAmount = [clue.cluePolyVertices count];

     //Count vertices. 
     for (int polyIndex = 0; polyIndex <= cluePolysAmount-1; polyIndex++)
      cluePolyVerticesAmount[polyIndex] = [[clue.cluePolyVertices objectAtIndex:polyIndex] count];

     //Import vertex data.
     for (int polyIndex = 0; polyIndex <= cluePolysAmount-1; polyIndex++)
      for (int vertexIndex = 0; vertexIndex <= cluePolyVerticesAmount[polyIndex]-1; vertexIndex++)
       cluePolyVertices[polyIndex][vertexIndex] = CGPointMake([[[clue.cluePolyVertices objectAtIndex:polyIndex] objectAtIndex:vertexIndex] xCoord],
                       [[[clue.cluePolyVertices objectAtIndex:polyIndex] objectAtIndex:vertexIndex] yCoord]);


    }
    return self;
}

//The "render" method.
-(void) drawRect: (CGRect)rect
{

    //Draw the puzzle polygon from the clue array.
    screenContext = UIGraphicsGetCurrentContext(); 
    CGContextSetRGBStrokeColor(screenContext, 1.0, 0.0, 0.0, 1.0);
    CGContextSetLineJoin(screenContext, kCGLineJoinRound);
    CGContextSetLineWidth(screenContext, 0.5);

    CYCLE_CLUE_POLYS
    {
     for (int vertexStepper = 0; vertexStepper <= cluePolyVerticesAmount[cluePolyIndex]-1; vertexStepper++)
     {
      CGContextMoveToPoint(screenContext, cluePolyVertices[cluePolyIndex][vertexStepper].x, cluePolyVertices[cluePolyIndex][vertexStepper].y);
      CGContextAddLineToPoint(screenContext, cluePolyVertices[cluePolyIndex][((vertexStepper<cluePolyVerticesAmount[cluePolyIndex]-1)?vertexStepper+1:0)].x, cluePolyVertices[cluePolyIndex][((vertexStepper<cluePolyVerticesAmount[cluePolyIndex]-1)?vertexStepper+1:0)].y);
     }
    }

    CGContextClosePath(screenContext);
    CGContextStrokePath(screenContext);

}

//Finger released.
-(void) touchesEnded: (NSSet*)touches withEvent: (UIEvent*)event
{

    //The completement condition is a simple released tap for now...
    puzzleCompleted = YES;

    //Test if puzzle is completed.
    [self isSolved]; 

}

//Test if the puzzle is completed.
-(void) isSolved
{

    if (puzzleCompleted) //Solved.
    {
     //"Notify viewController".
     [[NSNotificationCenter defaultCenter] postNotificationName:@"finishedCurrentLevel" object:nil]; 
    }

}

-(void) dealloc
{
    NSLog(@"Level dealloc invoked");
    [super dealloc];
}

@end

This is really kinda "skeleton". Anyway, as I expected, this shrinken version works now, but with the Object Allocation instrunment shows incrementing BGTangramLevel allocations.

Is it an "acceptable" way to organize my app? With this Clue-library stuff, and the others. maybe the UIImage-s, or ImageViews drives the fresh allocation crash in the extended/original app?

Nozzi, what do you suggest on redesigning?

Geri
I won't answer the "how to redesign" issue because it's very broad and belongs in a different question (with a different set of required bits of information).For this problem (why is it crashing or leaking), I see that my first assessment is correct - you're definitely not following the Cocoa memory management rules and conventions.
Joshua Nozzi
1 - Class convenience methods (+levelWithClue:...) imply autoreleased objects. 2 - Don't pay attention to retain count: that's absolutely the wrong approach. 3 - Follow memory management rules in the documentation and *only* worry about them. They're brief and relatively simple if you take them as gospel. 4 - You're releasing currentView in two separate places: use a retained property with synth'd accessors only and you don't have to worry about where to retain/release again (and shouldn't retain/release anywhere else for that accessor).
Joshua Nozzi