C++ Memory Leak, Valgrind Message Misleading?

Question

I'm a bit new to C++, and I'm able to run my code without errors, however, I'm running into memory leaks when I run it through Valgrind, and I can't for the life of me seem to figure out where I'm leaking! Here's my error message:

==22902==     in use at exit: 72,728 bytes in 2 blocks
==22902==   total heap usage: 4 allocs, 2 frees, 73,816 bytes allocated
==22902==
==22902== 24 bytes in 1 blocks are definitely lost in loss record 1 of 2
==22902==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22902==    by 0x401086: Bar::Bar(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int) (Bar.cpp:10)
==22902==    by 0x400F76: main (Foo_main.cpp:20)

I have a class, Foo, which are used as "attachments" to the Bar class. Bar inherits from foo. It has two private members, id_ and name_, and the destructor and use() are declared as virtual in the header file.

#include "Foo.h"

#include<iostream>

using std::cout;
using std::endl;
using std::string;

Foo::Foo(int id, const string& name) :
        id_(id),
        name_(name){}

Foo::~Foo() {}

int Foo::id() const { return id_; }

string Foo::name() const { return name_; }

void Foo::use() const {
    cout << "Using Foo #" << id_ << ", " << name_ << endl;
}

Bar has two private members, num_attachments_ (unsigned int) and attachments_ (Foo**)

#include "Bar.h"
#include <iostream>

using std::cout;
using std::endl;

Bar::Bar(int id, const std::string& name, unsigned int num_attachments) :
        Foo(id, name),
        attachments_(new Foo*[num_attachments]),
        num_attachments_(num_attachments) {
    // explicity null out each entry in the new array
    for (unsigned int i=0; i<num_attachments; ++i) {
        attachments_[i] = NULL;
    }
}

void Bar::use() const {
    cout << "Using Bar #" << id() << endl;
    for (unsigned int i=0; i<num_attachments_; ++i) {
        if (attachments_[i] != NULL) {
            attachments_[i]->use();
        }
    }
}

(Note: Some of the code that I know isn't causing leaks is commented out). I suspect that the issue is in Bar's use() function, but I'm not quite sure what's missing!

Finally, here's the main function:

Foo* f = new Bar(1, "foobar", 3);
f->use();
delete f;

I can, of course, upload the entire program at request (although I feel like the problem is probably obvious and I'm just missing something completely). Any help would be awesome!

You're missing a destructor that `delete`s the `new`ed array. Fairly basic. — Sam Varshavchik, Mar 22 '17 at 01:53

Santiago Varela · Accepted Answer · 2017-03-22T02:21:20.160

You need to declare and implement an explicit destructor for Bar

The memory leak is probably must be happening, because you have not freed your attribute attachments_'s memory, which you have to allocated in your Bar constructor.

It destructor should be implemented similar to this:

Bar::~Bar() {
    for (unsigned int i = 0; i < num_attachments_; i++) {
        if (attachments_[i] != NULL) {
            delete attachments_[i];
        }
    }
    if(attachments_) { // Just a safeguard good practice for defensive programming. You could omit this statement. This if statement is the same as if (attachments_ != NULL)
        delete [] attachments_;
    }
}

In addition, since Bar inherits from Foo, in Bar.h it is already good that your destructor has been declared as virtual so that the derived class destructor gets called before your base's class destructor. Since the the derived class has dynamic memory, you want that scenario to take place. So in Bar.h:

class Bar: public Foo {
   /* ...other class members... */
   public:
       /* ...other class operations... */
       virtual ~Bar(); // the virtual keyword here forces the program to visit Bar's (the derived class) destructor before Foo's (the base class)destructor. It is necessary, otherwise it only invokes the Foo's destructor (leaving leaked dynamic memory from Bar)
};

Wow, I can't believe I missed that, thank you so much! I'm no longer getting a leak, however, in a more complex problem, would it be more appropriate to add "delete [] attachments_" after the for loop since it is of type Foo**? — gmooney8, Mar 22 '17 at 02:17
You're spot on. I rushed it and missed that. I've now edited my answer. — Santiago Varela, Mar 22 '17 at 02:19
BTW: there is no reason to check for NULL before delete (see http://stackoverflow.com/questions/8004495/deleting-a-null-pointer and http://stackoverflow.com/questions/615355/is-there-any-reason-to-check-for-a-null-pointer-before-deleting) — Frunsi, Mar 22 '17 at 02:19
@Frunsi plus1, that's true if you really know what you're doing. However, it's a good practice. It doesn't hurt applying it, especially when starting out with C++ memory management. — Santiago Varela, Mar 22 '17 at 02:22

score 0 · Answer 2 · answered Mar 22 '17 at 02:25

0

try to use std::unique_ptr for Foo Bar objects.

http://en.cppreference.com/w/cpp/memory/unique_ptr

answered Mar 22 '17 at 02:25

sailfish009

2,561
1
24
31

C++ Memory Leak, Valgrind Message Misleading?

2 Answers2

You need to declare and implement an explicit destructor for Bar