1

My class have member function that take pointer of it's own type as it's argument.

When I do this:

Object* obj1 = new Object();
Object* obj2 = new Object();

obj1->add_child(obj2)

delete obj1;
delete obj2;
obj1 = NULL;
obj2 = NULL;

and run valgrind, the report says:

HEAP SUMMARY:
    in use at exit: 72,704 bytes in 1 blocks
  total heap usage: 6 allocs, 5 frees, 73,098 bytes allocated

LEAK SUMMARY:
   definitely lost: 0 bytes in 0 blocks
   indirectly lost: 0 bytes in 0 blocks
     possibly lost: 0 bytes in 0 blocks
   still reachable: 72,704 bytes in 1 blocks
        suppressed: 0 bytes in 0 blocks

I read an answer says that still reachable is fine, no leak. Then, when I try this:

Object* obj = new Object();

obj1->add_child(new Object());

delete obj;
obj = NULL;

valgrind's report says:

HEAP SUMMARY:
    in use at exit: 72,877 bytes in 3 blocks
  total heap usage: 6 allocs, 3 frees, 73,098 bytes allocated

LEAK SUMMARY:
   definitely lost: 144 bytes in 1 blocks
   indirectly lost: 29 bytes in 1 blocks
     possibly lost: 0 bytes in 0 blocks
   still reachable: 72,704 bytes in 1 blocks
        suppressed: 0 bytes in 0 blocks

It's obvious that I didn't delete the new Object() pointer that passed as an argument. So, how do I delete that pointer?

DETAILED UPDATE

definition of add_child(Object* obj):

void add_child(Object* obj) {

    container.add_child(obj);
}

container is a member of Object which is an instance of template class.

Container<Object> container;

The Container definition is:

template<class T>
class Container {
public:
    void add_child(const std::string& key, T* child) {
        childrens.insert(std::pair<std::string,T*>(key, child));
    }
private:
    std::multimap<std::string,T*> childrens;
}

Then, the childrens is actually a std::multimap.

I hope this not too long for just a question.

Improve this question
7
  • Where do you store the pointer? That's what you delete. Commented May 28, 2015 at 17:47
  • it's impossible to tell without knowing the definition of the Object class. You should really be using smart pointers (e. g. unique_ptr or shared_ptr) instead, though. Commented May 28, 2015 at 17:48
  • Notice that it says 72,704 bytes in 1 blocks, which doesn't match the size of the lost pointers in the second program. Do you have an idea what could be a such a large object in your program? (since it says "1 block", it's probably an array, given such a large size) Commented May 28, 2015 at 18:00
  • Updated. That's how I stored the pointer. Commented May 28, 2015 at 18:02
  • What version of gcc do you use? If it's 5.1, then you may suffer from this bug Commented May 28, 2015 at 18:09

3 Answers 3

Reset to default
3

You need to make a clear and consistent choice about who owns pointers. If you decide that add_child takes ownership, then the caller should expect that they don't need to delete the pointer passed in. If you decide that add_child does not take ownership, then the caller retains ownership and deletes the pointer.

You can make Object take ownership, so its destructor deletes all the child pointers that have been added to it. Then your first example should read

Object* obj1 = new Object();
Object* obj2 = new Object();

obj1->add_child(obj2);

delete obj1; // also deletes obj2 automatically

and your second:

Object* obj = new Object();

obj->add_child(new Object());

delete obj; // automatically deletes the unnamed pointer passed in on previous line

If you don't want Object to take ownership, then you have a problem, since you have no way of deleting the new Object() in the context of the caller.

If you use smart pointers, managing ownership becomes much easier. add_child can take a std::unique_ptr<Object> argument.

auto obj1 = std::make_unique<Object>();
auto obj2 = std::make_unique<Object>();
obj1->add_child(std::move(obj2)); // obj1 takes ownership
// both pointers automatically deleted at scope exit

auto obj = std::make_unique<Object>();
obj->add_child(std::make_unique<Object>());
// both pointers automatically deleted at scope exit
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

Once the function call is finished, you no longer have the pointer that new created. If the function you called doesn't delete the pointer, then you have a memory leak.

Improve this answer

Comments

1

The valgrind report about your still reachable memory block isn't your fault - it's the bug in gcc 5.1.

How did I come up with this:

The block size was very large (72704) and didn't match the size of allocated objects, so next I tried to divide 72704 by 144 and 29 to see if it's an array of these objects.

But it wasn't since neither 72704, nor 72704-4, nor 72704-8 (array, array + 32-bit integer to store the size, array + 64-bit integer to store the size) weren't divisible. So then I googled "72,704 still reachable", which showed the mentioned question.

The other answerers' suggestions to use smart pointers (like std::unique_ptr and std::shared_ptr) are sound, and I too recommend using them.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.