Destructor segfault in simple LinkedList program

Question

I'm creating a basic linked list in C++, but for some reason my destructor segfaults when I run the program with a signal 11 (which I found using Valgrind). My Link object only has two variables, string value and Link* next

This is the destructor.

Link::~Link() {
  Link* curr = this;
  while (curr != NULL) {
    Link* nextLink = curr->next;
    delete curr;
    curr = nextLink;
 }
}

This is main.cpp

int main() {

  string temp;
  getline(cin, temp);
  Link* head = new Link(temp, NULL);
  Link* tempHead = head;

  for(int i = 1; i < 5; i++) {
    getline(cin, temp);
    Link* newLink = new Link(temp, head);
    head = newLink;
  }
  head->printAll(head);
  head->~Link();

  return 0;

}

EDIT: For link.cpp, I did this-

Link::~Link() {
  Link* curr = this;
  delete curr;
}

And then for main.cpp, I changed head->~Link() to

  Link* curr = tempHead;
  while(curr!=NULL) {
    Link* nextLink = curr->getNext();
    curr->~Link();   //replacing with delete curr gives the same segfault
    curr = nextLink;
  }

1.: `head->~Link();` don't do this. 2.: `delete curr;` you are deleting `this` from inside the destructor. — tkausl, Apr 26 '17 at 01:25
Explicitly calling the destructor is not usually the right path. — Mikel F, Apr 26 '17 at 01:42
Please note that as @tkausl states, you should never explicitly call the destructor unless you really know what you are doing. To keep your pattern, have an empty destructor and then use `delete curr;` rather than `curr->~Link();`. — Ken Y-N, Apr 26 '17 at 01:43
I tried `Link* curr = head`; `while(curr!=NULL)` { `Link* nextLink = curr->getNext();` `~Link();` `delete curr;` `curr = nextLink; }` but it gives me errors, and removing `~Link()` and only using delete still gives me the same segfault :( — fraiser, Apr 26 '17 at 01:45

Mikel F · Accepted Answer · 2017-04-26T01:46:17.343

When you execute delete curr; in your destructor, it will call the destructor again for that node, resulting in an endless recursive loop that is probably overflowing your stack.

You should not be calling what is effectively 'delete this' in your destructor.

Instead consider:

Link::~Link() {
      delete next;
}

And in main, simply use delete head;

This is still recursive, which can be problematic with a long list and a small stack. Alternatively, you can put the loop in your main function that walks the list rather than in the destructor. If you want to encapsulate the delete loop in the class itself, add a clear method to perform the loop. Just don't call it from the destructor.

And the solution is (I think) to call `delete head;` in `main()`, then just `delete next;` in the destructor. Note that `delete nullptr;` [is safe](https://stackoverflow.com/questions/4190703/is-it-safe-to-delete-a-null-pointer). — Ken Y-N, Apr 26 '17 at 01:37

Destructor segfault in simple LinkedList program

1 Answers1