Deallocating static member variable

Question

Hello,

I am having some problems in trying to deallocate a static member variable. I shall detail what exactly I am trying to do.

I have a  logger class, which is a singleton instance. The logger class is as follows

class Logger
{
public:
	static	Logger	*Instance();
	
	void	CloseFile();
	void	Destroy();
	void	WriteLogHeader(string directories[], int i);

	ofstream	&getOfstream();

private:
	Logger() {};
	Logger(Logger const&){};

	string		filename;	/* Name of the log file	*/

	ofstream	output;		/* Output buffer pointing to the log file */

	static	Logger	*logger;	/* Pointer to the Logger class */
};

Logger* Logger::Instance()
{
	if (!logger)
		logger	=	new Logger;

	return	logger;
}

void	Logger::Destroy()
{
	if (logger)
		delete	logger;
}

ofstream &Logger::getOfstream()
{
/*
	This function returns a reference to the output stream
*/
	return	output;
}

This logger class is used my C++ dll. Once I am done all processing within the dll, I am trying to deallocate the logger memory before returning to the calling program.

DLLMain.cpp

int dllmain()
{

...
...
...
Some C++ Code
...
...
...

// Deallocating logger object
Logger::Instance()->Destroy();

return EXIT_SUCCESS;

}

When the dll is executed a second time (i.e the dll triggered by the calling program) the dll crashes and throws a memory exception.

I think the logger object is not being released correctly and when I am creating it again, there seems to be a problem.

Any advice as to how I can deallocate the memory correctly is appreciated.

Thanks

Kaushik

Answer 1

On Destroy() you should set the pointer to NULL because the Instance() method only creates the log object if the pointer is NULL.  But why use pointers?  The whole idea of C++ is to automatically destroy objects once their scope is done.

---

Jose R. MCP

Answer 2

So should I be doing this?

void	Logger::Destroy()
{
	if (logger)
		delete	logger;
        
        logger = NULL;
}

Since I am allocating memory with new operator, I am trying to release the allocated memory with delete. I just trying to avoid any memory leaks.

Thanks

Answer 3

Singleton is not something that gets created and destroyed all the time, which is what you're trying to do.

If you really want a singleton, try this:

class Logger
{
public:
	static	Logger& Instance(); // No need for a pointer; pointer might give NULL, and that makes no sense here.
	void	WriteLogHeader(string directories[], int i);
	ostream &getOstream();
// no need for oFstream here. That exposes implementation details client likely don't need.
// Also, this way, they can't accidentaly close the stream ;-)

private:
	Logger() {};
	Logger(Logger const&){};
// You also want void operator=(const Logger& rhs) {} here.
// Or, derive your Logger from boost:noncopyable
...
	static	std::auto_ptr<Logger> logger; // auto_ptr will make sure that your object is destroyed when your 
};

Logger& Logger::Instance()
{
	if (!logger.get())
		logger = new std:auto_ptr<Logger>;
	return *logger;
}
...

Answer 4

FYI, std::auto_ptr is now deprecated if you are using Visual Studio 2010 or above.  Use std::unique_ptr instead.

---

Jose R. MCP