8

In the following code when I add the the line which is specified with an arrow gives error:

Error in `./a.out': double free or corruption (fasttop): 0x00000000007a7030 * Aborted (core dumped)

The code works if I do not use destructor. Any idea?

#include<iostream>
#include<vector>

struct Element
{
    int *vtx;

    ~Element ()
    {
        delete [] vtx;
    }
};

int main ()
{
    Element *elm = new Element [2];
    elm[0].vtx = new int [2]; // <----- adding this gives error

    std::vector <Element> vec;
    vec.push_back (elm[0]);
    vec.push_back (elm[0]);

    return 0;
}
Shibli
  • 5,879
  • 13
  • 62
  • 126
  • 1) http://klmr.me/slides/modern-cpp/ 2) http://flamingdangerzone.com/cxx11/2012/08/15/rule-of-zero.html 3) http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29 *in this order*. – Griwes May 09 '14 at 05:44
  • 2
    @juanchopanza, I am not sure - this is asking a question where "follow the Rule of Three" (or now, the Rule of Zero) is an answer, not one about what the Rule of Three is (since OP didn't hear the term prior to asking). – Griwes May 09 '14 at 05:46
  • There is no good way for this code to work without an overhaul. The simplest solution would be to change `vtx` to `std::vector` and get rid of the destructor. Even a copy constructor won't help you as-is. – Ryan Haining May 09 '14 at 05:46
  • @Griwes The information in the duplicate would answer this question. I'm just following common practice, but if that isn't right I'm happy to remove the close vote. – juanchopanza May 09 '14 at 05:48
  • @juanchopanza, I am just wondering if the one I chose as dupe isn't the right-er dupe than the one you chose. – Griwes May 09 '14 at 05:50

2 Answers2

5

When you add elm[0] to vec, copies of elm[0] are stored in vec. Since you haven't defined a copy constructor, the compiler used the default one -- which performs a member by member copy. In this case, it keeps a copy of the pointer vtx. Now you have three objects pointing to the same memory.

When vec gets destructed, it calls the destructor on two of those objects. They each try delete on the same pointer. Hence the error.

If you want to avoid errors like these, check out Rule of Three.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Basically the problem is Shallow Copy here. Write a copy constructor and assignment operator and do a deep copy whenever you have pointer members. ``` Element(const Element& e) { cout << "Copy constructor\n"; vtx = new int [2]; } ``` – sudheerbb Apr 05 '20 at 06:06
3

This is because you make copies of your Element when you push it in the vector, but the vtx is not duplicated on copy, so an the end of main(), you will have three Elements pointing to the same vtx. When the program terminates, all three of them will try to delete the same int array.

Maurice Perry
  • 32,610
  • 9
  • 70
  • 97