CODEWITHSUNDEEP
c++linked-listsegmentation-faultsingly-linked-list
ADisplayName
ADisplayName

Reputation: 29

Destructor of a SingleLinkedList Causing a Seg Fault

I am working on coding a singly linked list in c++, and the destructor of my singly linked list is causing a segfault when it is called on a list with more than one node in it.

I am running tests on my linked list class to make sure it is functioning properly, and I am running into the problem when testing the PushFront method. I realized the destructor was causing the seg fault when I removed the delete list line from the testing function, and it ran fine without segfaulting (the PushFront testing function is just one function in a series of several testing functions to test all aspects of the linked list. With the delete line removed, the testing program calling this series of testing functions finishes execution perfectly fine, but with the delete line, it causes a segfault.

This is the destructor of the linked list (with cout statements for debugging purposes):

// CSingleLinkedList Destructor
CSingleLinkedList::~CSingleLinkedList()
{
    std::cout << "In Destructor" << std::endl;

    CSingleLinkedList::CSingleLinkedNode* temp = head_;

    std::cout << "temp = " << temp << std::endl;

    while(temp != nullptr)
    {
        CSingleLinkedList::CSingleLinkedNode* toDelete = temp;
        temp = temp->GetNext();

        std::cout << "toDelete = " << toDelete << std::endl;
        std::cout << "temp = " << temp << std::endl;

        delete toDelete;
    }
}

This is the destructor of the linked node (which only has data members value_ (an int) and next_ (a pointer to the next CSingleLinkedNode):

// CSingleLinkedNode Destructor
CSingleLinkedList::CSingleLinkedNode::~CSingleLinkedNode()
{
    delete next_;
}

This is the testing function I am running to test the PushFront function:

void TestListPushFront()
{
    CSingleLinkedList* list = new CSingleLinkedList();

    list->PushFront(1);

    assert(list->GetFrontValue() == 1);
    assert(list->GetBackValue() == 1);
    assert(list->GetSize() == 1);

    list->PushFront(2);
    list->PushFront(3);

    assert(list->GetFrontValue() == 3);
    assert(list->GetBackValue() == 1);
    assert(list->GetSize() == 3);

    std::cout << "TestListPushFront Passed!" << std::endl;

    delete list;
}

And this is the trace I see when I run the function:

TestListPushFront Passed!
In Destructor
temp = 0x55ce050332e0
toDelete = 0x55ce050332e0
temp = 0x55ce050332c0
toDelete = 0x55ce050332c0
temp = 0x55ce050332a0
Segmentation fault

Anyone have any ideas as to why this seg fault is occuring?

Upvotes: 2

Views: 152

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 596713

Your CSingleLinkedNode destructor has the following statement:

delete next_;

As soon as your CSingleLinkedList class delete's a node, that node AND ALL SUBSEQUENT NODES are freed, because you are invoking recursive destruction.

As such, when your CSingleLinkedList destructor destroys the head_ node and then tries to access the next node, it crashes since that next node was already destroyed. That is where your segfault is coming from.

Instead, your CSingleLinkedList destructor would need to be a single delete statement by itself:

CSingleLinkedList::~CSingleLinkedList()
{
    std::cout << "In Destructor" << std::endl;

    delete head_;
}

However, using a recursive destructor inside a linked list is never a good idea, especially if the list has a large number of nodes. This is likely to cause a stack overflow, because each recursive call to the CSingleLinkedNode destructor will push more and more data onto the call stack, until either the end of the list is reached, or the call stack runs out of space.

Always use iterative loops when handling nodes in a linked list - like your CSingleLinkedList destructor is trying to do. In order for that loop to work properly, you need to remove the delete next_; statement from your CSingleLinkedNode destructor. Nodes have no business destroying other nodes. That is the responsibility of their parent list class to manage instead.

Upvotes: 5

Related Questions