Why memory leak only happens in case of assignment operator overloading but not in copy constructor and how copy and swap idiom resolves it

Question

P.S: I am new to programming so please answer my doubts in simpler terms. I have found a couple of answers but could not understand them. Below is the copy constructor and assignment operator overload.

template <class T>
Mystack<T>::Mystack(const Mystack<T> &source)            // copy constructor
{
    input = new T[source.capacity];
    top = source.top;
    capacity = source.capacity;
    for (int i = 0; i <= source.top; i++)
    {
        input[i] = source.input[i];
    }
}


template <class T>
Mystack<T> & Mystack<T>::operator=(const Mystack<T> &source)       // assignment operator overload 
{
    input = new T[source.capacity];
    top = source.top;
    capacity = source.capacity;

    for (int i = 0; i <= source.top; i++)
    {
        input[i] = source.input[i];
    }
    return *this;
}

main function snippet

   Mystack <int> intstack = tempstack; (copy constructor)
    Mystack <int> floatstack, temp_1;
    floatstack = temp_1;  (assignment operator)

Understanding : I understand that we need copy and assignment operator so that we can have deep copying in case if we are using heap memory and thus there will be no dangling pointer issue when we delete one of the objects.

Can some one please answer below questions.

1. : Is my understanding correct?

2. : I have been suggested by developers that I have memory leak in assignment operator. If yes, can some please explain me how?

3. Copy constructor has more or less same code as assignment operator then how come I have memory leak only in case of assignment operator but not in the copy constructor function.

4. : In case I really have memory leak. What magic copy and swap idiom does that mem leak gets resolved.

P.S: Its not the complete running code. In actual code objects does contain some data. Kindly bear!

Answer 1

"Is my understanding correct?"

Yes you seem to understand. The full reasons are best described by the Rule of Three concept. If you find yourself having to implement any of the three (copy-ctor, assignment op, or destructor) to manage dynamic memory, you very likely need all three (and maybe more, see the article).

---

"I have been suggested by developers that I have memory leak in assignment operator. If yes, can some please explain me how?"

You've not posted your default constructor, but I assume it looks something like this:

Mystack<T>::Mystack(size_t size = N)
{
    input = new T[size];
    top = 0;
    capacity = size;
}

or something similar. Now, Lets see what happens in your assignment operator:

input = new T[source.capacity]; 

Um, what just happened to the old value in input for this object? It is no longer reachable and with it the memory therein is no longer reclaimable. It is leaked.

---

"Copy constructor has more or less same code as assignment operator then how come I have memory leak only in case of assignment operator but not in the copy constructor function."

There is no previous value for input assigned in the target of the copy construction in your copy-ctor. I.e. input doesn't point to anything yet (how could it? you're just-now creating the target object). Thus, no leak.

---

"In case I really have memory leak. What magic copy and swap idiom does that mem leak gets resolved."

The copy-swap idiom uses the copy-constructor to create a temporarily-held value-copy, then uses the assignment operator to swap object "guts" with that copy. in doing so, the out-going temporary will destroy the target object's original content when its destructor fires, while the target object has taken ownership of the temporary's incoming content as its own.

This provides multiple benefits (and yes, one drawback), and is brilliantly described here. A simple example in your code is:

template <class T>
void Mystack<T>::swap(Mystack<T>& src)
{
    std::swap(input, src.input);
    std::swap(top, src.top);
    std::swap(capacity, src.capacity);
}

and your assignment operator becomes:

template <class T>
Mystack<T> & Mystack<T>::operator=(Mystack<T> src) // NOTE by-value intentional,
                                                   // invokes copy-ctor.
{
    this->swap(src);
    return *this;
}

Now you have one implementation of copying (in the copy-ctor) to manage. Further, if any exceptions happen they will do so during the construction of the value-copy, not here. The chances of this object being polluted to indeterminate state are reduced (a good thing)

If you're curious of the drawback I mentioned earlier, consider how self-assignment (x = x;) would play out with a paradigm such as this. Honestly, I personally don't consider self-assignment inefficiency a drawback. If your code frequents the likes of x = x; you probably have a putrid smell in your design to begin with.

---

I strongly suggest you read the article for other info on the concept. It is one of those may-change-how-you-think things that you'll remember the rest of your career.

Answer 2

The purpose of the copy constructor is so that the input pointer of two instances of the class do not end up pointing to the same buffer in the heap. If they do, modifying one stack will affect the other and the destructor of one will free the memory of the other, resulting in use-after-free errors.

Your assignment operator does result in a memory leak because it does not free the memory that was previously allocated to that instance of the stack. Therefore, the buffer that input was pointing to does not end up being de-allocated when the destructer was called. This is not a problem with the copy constructor because it is only called on a new instance of the class, which does not have any memory allocated to it from before. In order to solve the problem, add the following line to the beginning of the assignment operator:

delete [] input;