4

After reading about "possibly lost" block message with Valgrind it seems they are bad.

I am getting the error for a static pointer class member. I want to verify there is nothing wrong with our code.

I am getting this from Valgrind:

==27986== 76 bytes in 1 blocks are possibly lost in loss record 370 of 1,143
==27986==    at 0x4C247F0: operator new(unsigned long) (vg_replace_malloc.c:319)
==27986==    by 0x107CFEE8: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (new_allocator.h:94)
==27986==    by 0xDDCE21F: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (basic_string.tcc:140)
==27986==    by 0x107D19B2: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (basic_string.h:1722)
==27986==    by 0xD381F19: MyNamespace::MyClass::MyMethod() (MyClass.cpp:189)
==27986==    by 0xD1A6E17: __static_initialization_and_destruction_0(int, int) (IProperty.cpp:520)
==27986==    by 0xD1B2B97: _GLOBAL__sub_I_IProperty.cpp (IProperty.cpp:551)
==27986==    by 0x400D4F2: call_init (in /lib64/ld-2.5.so)
==27986==    by 0x400D5B4: _dl_init (in /lib64/ld-2.5.so)
==27986==    by 0x4000AA9: ??? (in /lib64/ld-2.5.so)

My class (Simplified) to which this error is referring to: I have simplified the code to post it here, but I can add more detail if required.

MyClass.h

class MyClass
{
   private:
     double _p1, _p2, _p3, _p4;
     std::string _p5, _p6, _p7;
   public:
      MyClass(double p1, double p2, double p3, double p4, std::string p5, std::string p6, std::string p7) 
      {
         _p1 = p1;
         _p2 = p2;
         _p3 = p3;
         _p4 = p4;
         this->_p5 = p5;
         this->_p6 = p6;
         this->_p7 = p7;
      }

      static  MyClass& MyMethod();

}

MyClass.cpp

    static MyClass* _myPtr = NULL;
    MyClass&  MyClass::MyMethod()
    {
        if (!_myPtr )
        {
            _myPtr  = new MyClass(1, 2.1, 3, 4, "xxxx", "yyyyy", "zzzzz");
        }
        return *_myPtr ;
    }

I think we are using the static pointer properly. However, the documentation says that this normally is a memory leak unless you are doing something funny with your pointers. I don't think we are doing anything funny.

Is this error actually referring to the internal pointers of the string class that the constructor receives as parameters?

Should we worry about this possibly lost block error?

Dzyann
  • 5,062
  • 11
  • 63
  • 95
  • 1
    You are returning the value a pointer created by `new`. How are you ever supposed to `delete` that memory once you leave the function? Are you trying to make a [Singleton](http://stackoverflow.com/questions/1008019/c-singleton-design-pattern)? – NathanOliver May 18 '15 at 14:41
  • That can´t be real compilable code. – deviantfan May 18 '15 at 14:43
  • You need to `delete _myPtr;` during application exit – M.M May 18 '15 at 14:43
  • @deviantfan - the code is not mine, and it is simplified, but it does build. And we haven't had any issues, just the valgrind message. – Dzyann May 18 '15 at 14:50
  • `and it is simplified` That´s the problem. Eg. there is no p7 declared in the first code, but you´re using it. – deviantfan May 18 '15 at 15:24
  • @deviantfan - Sorry for that, I would have normally removed all parameters from constructor, but since in this case they could have been relevant, I wanted to leave them, and I forgot to add p7 in the declaration. I just added it. Thanks for pointing it out! – Dzyann May 18 '15 at 15:30

2 Answers2

3

It's a memory leak, not important one, as this object should live as long as program itself, yet destructor of MyClass will never be called and pointer freed. If MyClass uses some resources external to program that might be a problem.

try

MyClass&  MyClass::MyMethod()
{
    static MyClass instance(1, 2.1, 3, 4, "xxxx", "yyyyy", "zzzzz");
    return instance;
}

Destructor will be called (when all static objects will be destructed - after exiting main).

In C++11 it's even granted to be thread-safe.

Hcorg
  • 11,598
  • 3
  • 31
  • 36
  • It is not my code, but I understand there is a memory leak although when the application dies, it should go away right? Sadly, we are not using C++11, would you advice using a mutex to make it safe? Resources such as? The class it is really simple, doesn't have much more than what I showed in the sample. – Dzyann May 18 '15 at 14:55
  • When application dies every memory allocated by it goes away. It's a matter how 'clean' it goes. When you do not explicitly call `delete` then destructor is not called and memory is released when process dies, by OS, not by process itself. So Valgrind will report it. It's not huge error, but changing code will suppress it. 'External resources' - something like connection to database etc, which would be nice to be cleanly released. Your example is not thread-safe but if it works and your not planning to use it in concurrent code - don't bother with mutex. – Hcorg May 18 '15 at 14:59
  • You are right, I don't know why even said thread-safe, never mind that. Thanks for the clarification it seems I was right on my assessment, we will try to change the code to what you have suggested. – Dzyann May 18 '15 at 15:02
1

Your program has a memory leak by definition, but a benign one because the amount of heap memory that is not freed is bounded. Apart from that it is good practice to ensure that all destructors are called on program exit. E.g. consider that, at some time, you want to extend MyClass by logging to a file stream. If you do not destroy the stream object, the file handle will still be closed on every major operating system with no resource leak, but any data buffered inside the process will be discarded. Also, it is very easy to avoid this leak. Even if you want to create the object lazily, you can use std::unique_ptr or boost::optional, for instance, to ensure that it is properly destroyed on exit.

That said, if you are positive that you want to keep the code as is, you can create a suppression for Valgrind, and the block will no longer be listed in subsequent runs that use the corresponding suppressions file.

Arne Vogel
  • 6,346
  • 2
  • 18
  • 31