Help save net neutrality! Learn more.
Close

Memory leaks (Windows, VS 2005)

alexcohn
2006-03-06
2013-05-20
  • alexcohn

    alexcohn - 2006-03-06

    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

      Bastiaan Bakker - 2006-03-06

      _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

        alexcohn - 2006-03-06

        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

      alexcohn - 2006-03-08

      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;
              }
          }
       

       

Log in to post a comment.