C++ double free error from function copy

Question

I'm working through the Stroustrup C++ 11 book, and I ran into a double free exception. I understand that it's freeing the memory twice, but what I don't understand is why it's happening for a function that's passing by copy:

#include <iostream>

using namespace std;

namespace ALL_Vector { 

  class Vector {
    public:
      // Intitialize elem and sz before the actual function
      Vector(int size) :elem {new double[size]}, sz {size} {};
      ~Vector() {delete[] elem;};

      double& operator[](int i) {
        return elem[i];
      };
      int size() {return sz;};
    private:
      double* elem;
      int sz;
  };


  void print_product(Vector& y) {
    double result {1};

    for (auto x = 0; x < y.size() ; x++){
      if (y[x] > 0) {result *= y[x]; };
    }

    cout << "The product of Vector y is: " << result << ", or so it would appear ;)\n";
  } 

}


/*
  Self test of the Vector class.  
*/

int main(){  
    ALL_Vector::Vector myVector(15);
    cout << "The size of Vector y is: " << myVector.size() << "\n"; 
    myVector[0] = 12;
    myVector[2] = 7;
    myVector[3] = 19;
    myVector[4] = 2;

    ALL_Vector::print_product(myVector);

  return 0;
}

print_product() is taking the Vector class and creating a new Vector with duplicated contents? Why would this cause a double free? I'm assuming that RIIA in this instance is somehow interacting with the Vector::~Vector(), something like a race condition?

I know if I change this to pass it's argument by reference it will avoid the double free. I'm trying to better understand the issue with passing by copy.

Thanks!

@LeFlou: It has a bug that the test does not detect, namely it violates the rule of three. It's interesting that your comment says the code works fine then your answer explains why it's not ^_^ — Lightness Races in Orbit, Jan 22 '16 at 10:28

Richard Dally · Accepted Answer · 2016-01-22T10:22:16.810

5

Actually you are calling print_product with a reference to myVector, so everything is fine.
Troubles begin with passing myVector by value because default copy constructor will copy elem pointer instead of duplicating the whole array.
Both ALL_Vector::Vector elem pointer will refer to same memory storage and thus be deleted twice.
To tackle this issue, you have to implement copy constructor to create a new array and copy all elements.

edited Jan 22 '16 at 10:22

answered Jan 22 '16 at 10:12

Richard Dally

1,432
2
21
38

Great explanation, and it explains why passing by reference would work. When my new copy is destroyed at the end of the function call it frees the memory. So I would expect any further references to the original object would core dump since the memory allocated has already been destroyed. – MrMowgli Jan 22 '16 at 11:25
C++ does shallow copy by default. That means that references and pointers are copied correctly, but what they refer to and point to respectively are not. Deepcopy of dynamically allocated objects must be coded manually. This would include both copy constructor and copy assignment operator. Sometimes a special private member function that does the allocation and copying of data is warranted. – Chris Reid Oct 17 '21 at 01:10

score 1 · Answer 2 · answered Jan 22 '16 at 10:20

If you pass Vector per value, the copy constructor is called, not the constructor you have implemented. In this case, elem is not duplicated but the pointer is copied in the new object and then deleted twice by the destructor. You have to implement a copy constructor that allocate a new elem and copy all elements.

C++ double free error from function copy

2 Answers2