Memory leaks (Windows, VS 2005)

[alexcohn]

We have noticed three objects that are not released and display memory leaks with log4cpp library:

*Appender::_allAppenders
*HierarchyMaintainer::_defaultMaintainer
*(threading::ThreadLocalDataHolder<NDC> _nDC)

I believe that the easy solution like this:

    HierarchyMaintainer* HierarchyMaintainer::_defaultMaintainer = NULL;
    static HierarchyMaintainer _defaultMaintainer_;

    HierarchyMaintainer& HierarchyMaintainer::getDefaultMaintainer() {
        if (!_defaultMaintainer)
        _defaultMaintainer = &_defaultMaintainer_;
//            _defaultMaintainer = new HierarchyMaintainer();

could painlessly resolve the leaks for Appenders and for HierarchyMaintainer.

The NDC is trickier because it should take care of all the threads (see http://sourceforge.net/tracker/index.php?func=detail&aid=707529&group_id=15190&atid=315190 and the suggested change there).

[Bastiaan Bakker]

_defaultMaintainer cannot be a static object because that would break static initialisation of Category objects, e.g.

class Foo {
   static log4cpp::Category& _log;
};

Unfortunately, C++, unlike Java doesn't make any promises about the initialisation order of static obejcts :-(

[alexcohn]

Bastiaan, maybe you have got a test case for this scenario? To be on the safe side, you can use an envelope solution similar to the one I suggested for the NDC list:

    namespace {
        template<class T> class ownersingleton
        {
            T* pObj;
        public:
            ownersingleton(): pObj(NULL) {};
            ~ownersingleton() { if (pObj) delete pObj; };
            operator T*() { return pObj; };
            T* operator =(T* ptr) { pObj = ptr; return ptr; };
        };
        ownersingleton<HierarchyMaintainer> _defaultMaintainer;
    }

Do you think the same is true for the _allAppenders AppenderMap?

[alexcohn]

OK, let's use the atexit() function which should be portable. No changes to header files are needed.

***** NDC.cpp ****** (don't forget to add #include <list>)

    namespace {
        threading::ThreadLocalDataHolder<NDC> _nDC;
        std::list<const NDC*>* _listNDC = NULL;
        void clear_listNDC()
        {
            while (!_listNDC->empty())
            {
                const NDC* pObj = _listNDC->front();
                delete pObj;
                _listNDC->pop_front();
            };
            delete _listNDC;
            _listNDC = NULL;
        }
    }

**** and the getNDC() method becomes:

    NDC& NDC::getNDC() {
        NDC* nDC = _nDC.get();

        if (!nDC) {
            if (!_listNDC)
            {
                _listNDC = new std::list<const NDC*>;
                atexit(clear_listNDC);
            }
            nDC = new NDC();
            _listNDC->push_back(nDC);
            _nDC.reset(nDC);
        }

        return *nDC;
    }

*** HierarchyMaintainer.cpp ****

    namespace {
        void cleanupHierarchyMaintainer()
        {
            HierarchyMaintainer* _defaultMaintainer = &(HierarchyMaintainer::getDefaultMaintainer());
            delete _defaultMaintainer;
            _defaultMaintainer = NULL;
        }
    }

*** and getDefaultMaintainer() becomes:
        if (!_defaultMaintainer)
        {
            _defaultMaintainer = new HierarchyMaintainer();
            atexit(cleanupHierarchyMaintainer);
        }

        return *_defaultMaintainer;
    }

*** finally, in Appender.cpp _removeAppender(Appender* appender) becomes:

{
        threading::ScopedLock lock(_appenderMapMutex);
        _getAllAppenders().erase(appender->getName());
        if (_getAllAppenders().empty())
        {
            delete _allAppenders;
            _allAppenders = NULL;
        }
    }