Govan
Govan

Reputation: 2129

Shall I clear a vector of unique_ptr in destructor in c++11 even if valgrind doesn't show memory leak

Giving the Holder class below:

    class Holder {
        string  name;
        std::vector<std::unique_ptr<Object>> objects;
    public:
        Holder(string name): name(name){
        }   

        ~Holder(){};
        Holder & operator=(const Holder & holder) = delete;  

    vector<unique_ptr<Object>> const& Holder::getContent()const{
        return this->objects;
    }

    void Holder::add(unique_ptr<Object> objPtr){
       this->objects.push_back(move(objPtr));
    }


    };

If I am calling my Holder object in the method below:

void HolderTest::addObject(){
    Holder *holder = new Holder("bag");
    holder->add(unique_ptr<Object>(new Object("test")));
    vector<unique_ptr<Object>> const& objects = holder->getContent();
    const std::string name = objects[0].get()->name();
    CPPUNIT_ASSERT_EQUAL((string)"test", name);
    delete holder;
}

My question is: should I call my vector of unique_ptr's clear method in the Holder destructor to avoid memory leak like below?

~Holder(){
  this->objects.clear();
};

My other question can I still use "Valgrind Tools Integration" version 3.0.0.201502180018 for finding memory leaks in a c++11 application or it is not able to find memory leak in c++11 programs?

Upvotes: 4

Views: 3564

Answers (2)

CoffeDeveloper
CoffeDeveloper

Reputation: 8317

You do not have technically any memory leak. When you delete your holder, all elements in the vector are destroyed (because the destructor of Holder already clears the vector), and the destructor of unique_ptr actually release the memory allocated for added objects.

What it seems strange to me is that probably for your situation the most simple solution is using

std::vector< Object> objects;
objects.emplace_back("test");

wich is more efficient and simpler to read.

EDIT: 2nd part of the question, Valgrind may have a false positive (warn about a leak when you have no leak), but I never heard of Valgrind reporting a false negative (signaling no leak at all when you have a leak).

EDIT2:

#include <utility>  // std::forward
#include <vector>

//...

template< typename... Args>
void Holder::add( Args&&... args){
   objects.emplace_back( std::forward< Args>(args)...);
}

usage:

holder.add("Test");

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726559

You do not have to call clear manually. The destructor of std::vector<T> will call destructors of std::unique_ptr<T> automatically.

The major advantage of smart pointers over built-in pointers is that you don't have to deal with manual clean-up.

Upvotes: 7

Related Questions