0

I use doubly linked list to implement Deque in C++.

Destructor:

Deque::~Deque() 
{
    while (this->left_p) 
    {
        node *temp = this->left_p;
        this->left_p = this->left_p->next;
        delete temp;
    }

    this->right_p = NULL;
}

when i use valgrind --leak-check=full ./a.out to check memory leak just to test my destructor` I got the following output:

==2636== 
==2636== HEAP SUMMARY:
==2636==     in use at exit: 72,704 bytes in 1 blocks
==2636==   total heap usage: 1,003 allocs, 1,002 frees, 97,760 bytes allocated
==2636== 
==2636== 72,704 bytes in 1 blocks are still reachable in loss record 1 of 1
==2636==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2636==    by 0x4EC3EFF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==2636==    by 0x40106B9: call_init.part.0 (dl-init.c:72)
==2636==    by 0x40107CA: call_init (dl-init.c:30)
==2636==    by 0x40107CA: _dl_init (dl-init.c:120)
==2636==    by 0x4000C69: ??? (in /lib/x86_64-linux-gnu/ld-2.23.so)
==2636== 
==2636== LEAK SUMMARY:
==2636==    definitely lost: 0 bytes in 0 blocks
==2636==    indirectly lost: 0 bytes in 0 blocks
==2636==      possibly lost: 0 bytes in 0 blocks
==2636==    still reachable: 72,704 bytes in 1 blocks
==2636==         suppressed: 0 bytes in 0 blocks
==2636== 
==2636== For counts of detected and suppressed errors, rerun with: -v
==2636== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

I can't figure out why there is still ONE out of 1003 allocs not being free.

Why do i have one memory leak? what is wrong with my destructor?

Test code here:

/* Deque Test Program 6 */
#include <cstring>
#include <iostream>
#include "Deque.h"

using namespace std ;

int main (int argc, char * const argv[]) {
    cout << "\n\nDeque Class Test Program 6 - START\n\n";

    // Make a Deque
    Deque * dq1 = new Deque();
    for( int i = 0 ; i<1 ; i++ ){
        dq1->push_left(1);
        // dq1->display();
    }
    cout << "Size=" << dq1->size() << endl ;

    // The destructor should delete all the nodes.
    delete dq1 ;

    cout << "\n\nDeque Class Test Program 6 - DONE\n\n";
    return 0;
}

edit: remove implementation code.

齐天大圣
  • 1,169
  • 5
  • 15
  • 36
  • 1
    Please extract an MCVE and put that inline into your question. External links are too volatile and they tend to change. – Ulrich Eckhardt Mar 11 '18 at 07:56
  • Without the knowledge of Deque.cpp there is no way to tell what is wrong here. – Rudi Mar 11 '18 at 08:05
  • @Rudi Sorry. updated my question – 齐天大圣 Mar 11 '18 at 08:07
  • 1
    I can't reproduce the bug. Either you have something modified locally or you are experiencing a bug in the C standard library. When you use valgrind you also should compile and link all your programs with the `-g` switch, since valgrind gives much better output when it can find debug information. (As a side note, I also suggest to remove the explicit `g++ -c` calls when you transform .cpp into .o, since make has a built in rule for exactly this case.) – Rudi Mar 11 '18 at 08:25
  • After deleting all nodes towards the left, you then move on to set `this->right_p = NULL;`. Doesn't look very prudent to me. – IInspectable Mar 11 '18 at 08:47
  • While I'd use two locals/temporaries to walk/"delete" a linked list, [Setting a member to NULL in a destructor is uncalled for](https://stackoverflow.com/a/4666535) (in contrast to invalidating each and every reference to the object deleted). Your top snippet doesn't mention `cursor`; `deque.cpp`'s `~Deque()` does. – greybeard Mar 11 '18 at 09:24
  • The Deque (a list, really) contains `Node`s with a single `int` member. The node cannot possibly use 72000 bytes. That must be something else. – Bo Persson Mar 11 '18 at 09:55
  • @IInspectable - The OP is tricking us by using `left` and `right` instead of `begin` and `end` in the linked list (called Deque :-). Setting the end-pointer to NULL is harmless (and useless). – Bo Persson Mar 11 '18 at 09:59
  • @BoPersson: That's what I thought, too: It's either a bug, or useless. But then, if code is this tricky (read: unreadable), it will inevitably turn into a bug at some point. – IInspectable Mar 11 '18 at 10:28

2 Answers2

3

Essentially, it's not your code's fault, it's valgrind's.

Check this other question that has had the same problem: Valgrind: Memory still reachable with trivial program using <iostream>

Quoting from the post:

First of all: relax, it's probably not a bug, but a feature. Many implementations of the C++ standard libraries use their own memory pool allocators. Memory for quite a number of destructed objects is not immediately freed and given back to the OS, but kept in the pool(s) for later re-use. The fact that the pools are not freed at the exit of the program cause Valgrind to report this memory as still reachable. The behaviour not to free pools at the exit could be called a bug of the library though.

Hope that helps :)

2

The memory leak reported by valgrind does not appear to be in your code:

==2636== 72,704 bytes in 1 blocks are still reachable in loss record 1 of 1
==2636==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2636==    by 0x4EC3EFF: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==2636==    by 0x40106B9: call_init.part.0 (dl-init.c:72)
==2636==    by 0x40107CA: call_init (dl-init.c:30)
==2636==    by 0x40107CA: _dl_init (dl-init.c:120)

This appears to be a heap allocation from within a constructor of a global object. (In theory, it could still come from your code if operator new is called as a tail call, so that it does not show up in the backtrace, but I don't see such an object declaration in your cdoe.)

It is also not an actual leak, it is just some data allocated on the heap at program start. If you install debugging information for libstdc++, then you might get a hint of what is actually being allocated. Then you could also set a breakpoint on call_init and step through the early process initialization, to see the constructors that are called.

Florian Weimer
  • 32,022
  • 3
  • 48
  • 92