Sign in to follow this  
Scanmaster_k

singleton memory leak

Recommended Posts

Hi, i have a problem with a class that is a singleton, when ever I use it ogre wich I use reports a memory leak, it doesn't state exactly where it occurs but since I only get it when I use this class it more or less have to be it. Here is the code.
#pragma once

#include <cstdlib>

#include "define.h"

class CGameState
{
public:
	~CGameState(void);

	static CGameState *getSingleton(void);
	//const int getCurrentState(void) const {return(mInstance->mCurrentState);}
	//void setCurrentState(const int gameState) {mInstance->mCurrentState = gameState;}
	const int getCurrentState(void) const {return(mCurrentState);}
	void setCurrentState(const int gameState) {mCurrentState = gameState;}

protected:
	CGameState(void);
	CGameState(const CGameState&);

private:
	static CGameState *mInstance;
	int mCurrentState;
};

Implementation.
#include "GameState.h"

CGameState* CGameState::mInstance = NULL;

CGameState::CGameState(void) : mCurrentState(GAME_MAIN_MENU)
{
}

CGameState::~CGameState(void)
{
	if(mInstance != NULL)
	{
		delete mInstance;
		mInstance = NULL;
	}
}

CGameState* CGameState::getSingleton(void)
{
	if(mInstance == NULL)
	{
		mInstance = new CGameState;
	}
	return(mInstance);
}
Any ideas what I've done wrong? PS. This is what I get from ogre. 001409 0x00D1BBF0 0x00000004 0x00D1BBE0 0x00000024 0x00000000 new N N ??(0) ?? DS. Thankful for responses Scanmaster_K

Share this post


Link to post
Share on other sites
I don't know too much about ogre, so I'm not sure when it checks for memory leaks, but I know with windows crt memory leak checking globals can cause "false positives" on memory leak reports.

From what I understand, this is because globals exist outside of main(), and therefor, can still be around after main() terminates. Now I know with the crt memory leak checking, it looks for unfreed memory when main terminates, and will typically find globals, so it thinks they are memory leaks, even though they get cleaned up properly, just not until after main().

I can't say for sure if this is what ogre is picking up on with your singleton, but its something to keep in mind.

Share this post


Link to post
Share on other sites
Do you delete your singleton at the end of the programm? mInstance is a pointer, so the cGameState pointed by mInstance won't be automatically be destroyed at the end of your programm.

and if it was, it would crash your programm. when you call delete mInstance in your destructor, it will call the destructor, creating an infinite loop.

Share this post


Link to post
Share on other sites
A better way to make singeltons (my opinion) :

class Single {
public:
static Single *create(/*parameters*/) {
assert(inst == 0);
inst = new Single(/*parameters*/);
return inst;
}
static void destroy() {
assert(inst != 0);
delete inst;
inst = 0;
}
static Single *get() {
assert(inst != 0);
return inst;
}
private:
Single(/*parameters*/);
~Single();

Single(const Single&);
Single &operator=(const Single&);

Single *inst;
};

Single *Single::inst = 0;



Now you have control over when to create and when to destroy to avoid the problems above.

Share this post


Link to post
Share on other sites
Singletons?

1> Write a "singleton manager", whose job it is to create and destroy all of your program's singletons. The singleton manager is not created on the heap.

Your main() function is responisble for telling the singleton manager to "create singletons" before it does anything else, and "destroy singletons" as it's last action.

2> Write a "singleton provider", who stores a pointer to a singleton (ideally templated). The provider itself is not heap-allocated, but the singleton it wraps is. It registers itself with the "singleton manager" so it can be created and destroyed at a reasonable time.

Such a pattern will help you a hell of alot when you start creating singletons with state that needs to be saved to disk. Relying remembering to destroy each singleton, or static destruction, causes problems.

smurfkiller, I would leave the asserts in, but also include the "if". The extra stability to your code in the case that something doesn't go the way you expect is worth the teeny tiny performance hit. Write good, solid code, THEN optimize it.

Share this post


Link to post
Share on other sites
Quote:
Original post by NotAYakk
Write good, solid code, THEN optimize it.

It's hard to take you seriously when you say that just after endorsing over-engineered singletons as a design solution. [rolleyes]

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this