Memory Leak in Custom Linked List C++

Question

For an assignment I cannot use a STL list, it must be a custom list. As the title states, I have memory leaks even though I am calling delete on the nodes \ items. I would appreciate any help on this.

List Source

template <typename T>
class DLinkList
{
private:
    struct Node
    {
        T data;
        Node *nextNode;
        Node *prevNode;
        Node(T data, Node *nextNode = nullptr, Node *prevNode = nullptr)
        {
            this->data = data;
            this->nextNode = nextNode;
            this->prevNode = prevNode;
        }
        ~Node() { delete data; }
    };

    Node *head;
    Node *tail;

public:
    DLinkList();
    ~DLinkList();

    void push_back(T data);   
};

template <typename T>
inline void DLinkList<T>::push_back(T data)
{
    if (isEmpty())
    {
        head = new Node(data);
        tail = head;
    }
    else
    {
        tail->nextNode = new Node(data, nullptr, tail);
        tail = tail->nextNode;
    }
}

template <typename T>
DLinkList<T>::DLinkList()
{
    head = nullptr;
    tail = nullptr;
}

template <typename T>
DLinkList<T>::~DLinkList()
{
    Node *ptr = head;
    while (ptr->nextNode != nullptr)
    {
        Node *garbage = ptr;
        ptr = ptr->nextNode;
        delete garbage;
    }
}

Foo Class and main

class Foo
{
public:
    Foo() { i = 0; d = 0.0; }
    Foo(int i, double d) { this->i = i; this->d = d; }

    int getInteger() { return i; }
    double getDouble() { return d; }

private:
    int i;
    double d;
};


int main()
{
    DLinkList<Foo*> f1;
    f1.push_back(new Foo());
    f1.push_back(new Foo(2, 5.5));

    cout << "1st Values: " << f1.at(0)->getInteger() << ", " << f1.at(0)->getDouble() << endl;
    cout << "2nd Values: " << f1.at(1)->getInteger() << ", " << f1.at(1)->getDouble() << endl;

    return 0;
}

From valgrind

==12125== 40 (24 direct, 16 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==12125==    at 0x4C29203: operator new(unsigned long) (vg_replace_malloc.c:334)
==12125==    by 0x400FD8: DLinkList<Foo*>::push_back(Foo*) (DLinkList.hpp:138)
==12125==    by 0x400CF3: main (Source.cpp:28)

I am not sure how the memory is being lost here, I want to say that it is because it is making a copy of it and the original is being lost. If this is the case, I am unfamiliar with how to handle it.

Again, I appreciate any help in understanding this. I have tried to look through all related questions, but I did not see anything that covered this or at least I did not understand it. Thank you!

Answer 1

Given the other issues pointed out in the comments such as making an erroneous usage of explicitly calling the DLinkList destructor, you are doing this in main():

f1.push_back(new Foo());
f1.push_back(new Foo(2, 5.5));

You are creating an instant memory leak here on those two lines of code that can not be recovered. You are calling new but nowhere do you save the address returned from new so that you can call delete later on for that address.

So it is main that has to manage these dynamically allocated Foo objects. Something like this:

Foo* p1 = new Foo();
Foo *p2 = new Foo(2, 5.5);
f1.push_back(p1);
f1.push_back(p2);
//...
delete p1;
delete p2;

This should address the memory leak, but this is poor programming practice in this day and age of C++. You would more than likely either use

DLinkList<Foo>

and place Foo objects in the linked list, thus not requiring any manual memory management in main, or use a smart pointer and have a linked list of smart pointers, i.e.

DLinkList<std::unique_ptr<Foo>>