none
Deallocating static member variable

    Pergunta

  • 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

    quarta-feira, 22 de fevereiro de 2012 15:06

Respostas

  • 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;
    }
    ...
    • Sugerido como Resposta Helen Zhao sexta-feira, 24 de fevereiro de 2012 06:46
    • Marcado como Resposta Helen Zhao quinta-feira, 1 de março de 2012 01:38
    quinta-feira, 23 de fevereiro de 2012 12:03

Todas as Respostas

  • 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

    • Sugerido como Resposta Helen Zhao quinta-feira, 1 de março de 2012 01:38
    quarta-feira, 22 de fevereiro de 2012 15:26
  • 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

    quarta-feira, 22 de fevereiro de 2012 15:34
  • 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;
    }
    ...
    • Sugerido como Resposta Helen Zhao sexta-feira, 24 de fevereiro de 2012 06:46
    • Marcado como Resposta Helen Zhao quinta-feira, 1 de março de 2012 01:38
    quinta-feira, 23 de fevereiro de 2012 12:03
  • FYI, std::auto_ptr is now deprecated if you are using Visual Studio 2010 or above.  Use std::unique_ptr instead.

    Jose R. MCP

    quinta-feira, 23 de fevereiro de 2012 13:52