CODEWITHSUNDEEP
c++memory-leaksmove-constructor
Andrey Chertov
Andrey Chertov

Reputation: 49

Is there a memory leak in this C++ move constructor?

In Stroustrups The C++ Programming Language Fourth Edition, on page 76, there is example of move constructor use.

The class is defined like this:

class Vector { 
private: 
  double∗ elem; // elem points to an array of sz doubles 
  int sz; 
public: 
  Vector(int s) :elem{new double[s]}, sz{s} 
  { for (int i=0; i!=s; ++i) elem[i]=0; // initialize elements } 

  ~Vector() { delete[] elem; } // destructor: release resources 
  Vector(const Vector& a); // copy constructor 
  Vector& operator=(const Vector& a); // copy assignment 
  Vector(Vector&& a); // move constructor 
  Vector& operator=(Vector&& a); // move assignment 

  double& operator[](int i); 
  const double& operator[](int i) const; 
  int size() const; 
};

The move constructor is defined:

Vector::Vector(Vector&& a) :elem{a.elem}, // "grab the elements" from a 
  sz{a.sz} 
{ 
  a.elem = nullptr; // now a has no elements 
  a.sz = 0; 
}

Example execution which, I think, will cause memory leak:

Vector f() 
{ 
  Vector x(1000); 
  Vector y(1000); 
  Vector z(1000); 
  // ... 
  z=x; //we get a copy 
  y = std::move(x); // we get a move 
  // ... 
  return z; //we get a move 
};

It seems that such move operation will cause memory leak, because y had been allocated 1000 elements in 

Vector y(1000);

and simple pointer re-assignment at line y = std::move(x); would leave these initial 1000 ints pointed by y left on its own. I assume, the move constructor has to have extra line of code to de-allocate pointer 'elem' before moving.

Upvotes: 4

Views: 1720

Answers (4)

ascotan
ascotan

Reputation: 1684

and simple pointer re-assignment at line y = std::move(x); would leave these initial 1000 ints pointed by y left on its own.

The address of the pointer to double is copied over to the new object (y) and the old object (x) has it's pointer set to nullptr. (It's happening in the move assignment method not shown)

I assume, the move constructor has to have extra line of code to de-allocate pointer 'elem' before moving.

No deallocation needed here. The pointer is just 'moved' during assignment (it would also be moved during move construction). Cleanup of the double pointer should be handled by the destructor. In the case of x, the destructor would eventually fire on the nullptr. delete[] on a nullptr is a noop. see http://www.cplusplus.com/reference/new/operator%20delete[]/

The doubles allocated in y would however need to be cleaned out during the move assignment as pointed out.

Upvotes: 0

Andrey Chertov
Andrey Chertov

Reputation: 49

Yes, agree, this is move assignment: y = std::move(x);, not the constructor. In the book they did not show it, just mentioned that move assignment is defined similarly. But, in the move assignment definition, as I understand, we would need to deallocate elem, right? Something like this.

Vector& Vector::operator=(Vector&& a)
{ 
  delete[] elem;
  elem = a.elem;
  sz = a.sz;
  a.elem = nullptr; // now a has no elements 
  a.sz = 0; 
}

Upvotes: 0

eerorika
eerorika

Reputation: 238331

Is there a memory leak in this C++ move constructor?

No.

Example execution which, I think, will cause memory leak:

The shown execution calls move assignment, not move constructor.

I assume, the move constructor has to have extra line of code to de-allocate pointer 'elem' before moving.

It does not need to, because the pointer of the newly constructed object can not have pointed to any allocated memory before it was initialized with the value of the pointer from the move argument.

Upvotes: 2

Xarn
Xarn

Reputation: 3551

Assuming the move constructor is implemented properly, no. The line you are refering to invokes move assignment, not constructor, because z already exists.

Upvotes: 2

Related Questions