Thread: Singleton leak. Let us finish this (aka Partial Solution)

  1. #1
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446

    Singleton leak. Let us finish this (aka Partial Solution)

    Hello everyone,

    Well, I kinda solved the leak I was having with a singleton for those of you who remember this issue. I did because I had to build another one to maintain my application configuration settings. It was leaking too. But it also had a non-related bug that, when solved, made the leak go away.

    Anyways, I'll try and make this as simple as possible...

    Here's a simple example of a singleton with a heap based data member...

    Code:
    class MySingleton {
    public:
    
        static MySingleton& GetInstance() {
        
            if (instance_ == NULL) instance_ = new MySingleton();
    
            return *instance_;
        
        }
        
        ~MySingleton() {}
        
    private:
    
        static MySingleton* instance_;
    
        MySingleton() {
        
            heap_.push_back("hello");
        
        }
    
        std::vector<std::string> heap_;
    
    };
    
    /* implementation file */
    MySingleton* MySingleton::instance_ = NULL;
    As it is, this singleton is reported to leak by Microsoft Visual C++ 2005 Express. Go ahead and try it.

    - It leaks inside the vector allocator and in the allocation inside GetInstance().
    - It leaks only if you do more than one call to GetInstance().
    - But it leaks only once no matter how many calls to GetInstance().

    The call code can look something like this, for simplicity :

    Code:
    int main() {
        
        MySingleton& a = MySingleton::GetInstance();
        MySingleton& b = MySingleton::GetInstance();
        MySingleton& c = MySingleton::GetInstance();
    
    }
    If it is really leaking or this is an erroneous report ,I'm yet to know. But I strongly suspect the latter.

    However, if I make heap_ a static data member, the leak goes away. All I'm left with is a leak report of 1 byte pointing to instance_ allocation inside GetInstance(). It's the pointer. This one definitely has to be an erroneous report.

    Help me figure this out.
    Last edited by Mario F.; 09-13-2007 at 05:20 AM. Reason: fixed the class code. The constructor had a bug
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  2. #2
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    What about keeping a static count of the number of instances requested in your mySingleton Class? Then in the destructor, you can check whether all instances were destroyed and then delete the last instance.

    QuantumPete
    "No-one else has reported this problem, you're either crazy or a liar" - Dogbert Technical Support
    "Have you tried turning it off and on again?" - The IT Crowd

  3. #3
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    That is indeed a possibility Pete. Thanks. I'm however confused as to why the static pointer is not destroyed, regardless of there being no delete.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  4. #4
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Code:
    int main() {
        
        MySingleton& a = MySingleton::GetInstance();
        MySingleton& b = MySingleton::GetInstance();
        MySingleton& c = MySingleton::GetInstance();
    
    }
    If it's a singleton why would you want to do this? Or are you just demonstrating how to get the leak to happen?

  5. #5
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    Yup. Just to demonstrate the leak.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  6. #6
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    It says the vector is leaking too? That seems a bit odd. Have you checked the addresses of the instance(s) you are referring to in the debugger?
    Last edited by VirtualAce; 09-13-2007 at 05:10 AM.

  7. #7
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    No. Not the actual addresses, just the allocation number. However, no matter the number of instances only one leaks. My guess, the first.

    Unfortunately I can't be any more precise at this point. I don't have Studio here. But if the need arises, I can further investigate your question later on.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  8. #8
    The superhaterodyne twomers's Avatar
    Join Date
    Dec 2005
    Location
    Ireland
    Posts
    2,273
    Why don't you do the allocation in the constructor, Mario? It seems a bit odd to put it in a method.

  9. #9
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    Are you reall not deleting MySingleton::instance_ anywhere? Or did you just omit that part?

    Assuming you are deleting instance_ somwhere, it's just a false positive because of global object destruction.

    http://msdn2.microsoft.com/en-us/lib...tx(VS.80).aspx

    gg

  10. #10
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Quote Originally Posted by Mario F. View Post
    That is indeed a possibility Pete. Thanks. I'm however confused as to why the static pointer is not destroyed, regardless of there being no delete.
    Because the language doesn't work like that. Nothing is ever deleted unless you delete it yourself. The fact that it is "static" has nothing to do with it.

  11. #11
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    But...

    How can I delete that static pointer?

    - I can't do it on the destructor. GetInstance() would make no sense if I did it. Everytime I called it, the pointer would be null. The singleton would lose its one-time instantiation and globality properties.

    - I cannot keep a running count of instances, as suggested, and delete it on the destructor when the number of instances becomes 0. This would mean the same as above with the only difference I would be deleting it less times.

    What am I missing here regarding the singleton pattern usage? Deleting the pointer is not part of the pattern description on any of my readings.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  12. #12
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,820
    Check this out. At the bottom using a static non pointer that will be cleaned up.
    http://gethelp.devx.com/techtips/cpp.../10min0200.asp

  13. #13
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    Gosh! That simple.

    Thanks prog-bman, and everyone.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  14. #14
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    Quote Originally Posted by http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp
    Because operator new is thread-safe, you can use this design pattern in multi-threaded applications.
    This is bad information. operator new may or may not be thread safe (depending on the std library you're using). Either way Singleton::Instance() definately isn't thread safe. However, that code can be used in a thread safe manner as long as Singleton::Instance() is called prior to any additional threads calling it.

    gg

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 6
    Last Post: 06-02-2006, 08:32 AM