Implementation of "Rule of Three" gone wrong

Question

Below is an erroneous implementation of "The rule of three", which I try to understand.

Debugging the program, I found that the debugger has a problem cleaning up int *k, which could be resolved either by defining int *k = nullptr or simply setting it to something reasonable in the copy constructor.

However, I do not understand how the resulting error of the program, an access violation, comes into existence.

I do know, after the copy assignment constructor v1's int *kno longer points to a valid memory address.

class Vector2 {
public:
    std::string name = "default";
    int* k;

    Vector2(int s, std::string n) : k(new int[s]), name(n) {
    }

    Vector2(const Vector2 &other)  {
        std::cout<< "Copy constructor: " << name << std::endl;
    }

    ~Vector2() {
        std::cout << "Deleting: " << name << std::endl;
        delete[] k;
    }

    void swap(Vector2& other) {
        using std::swap;
        swap(k, other.k);
    }

    Vector2& operator=(Vector2 other) {
        std::cout << "Copy assignment constructor: " << name << std::endl;
        swap(other);
        return *this;
    }
};


int main() {
        Vector2 v1 = Vector2(2, "v1");
        Vector2 v2 = Vector2(4, "v2");
        v1 = v2;
        std::cout << &v1 << " " << &v2 << std::endl;
        std::cout << &v1.k << " " << &v2.k << std::endl;
        return 0;
    }

Below is the console output of above's program:

Copy constructor: default
Copy assignment constructor: v1
Deleting: default
0000001B5611FA28 0000001B5611FA78
0000001B5611FA50 0000001B5611FAA0
Deleting: v2
Deleting: v1
16:18:42: The program has unexpectedly finished.

Answer 1

The solution can be derived by laying out the exact sequence of events, e.g.: More print outs and testing, what parameters are called when:

Starting in: v1 = v2;

1. v2 calls copy constructor with the argument other (whatever other is), in particular: its int* k does not point to valid memory. For simplicity let's call this new Vector2 v3.
2. The copy assignment constructor is called now with v3.
3. Then we begin swap.

The error actually arises within the copy constructor, as v3 is not initialised properly in step 1 .

Step 2 and step 3 are basically "hiding", transferring the error from v3 onto v1.

The interesting question now is, how is v3 actually generated? Not by the default constructor!

Answer 2

Constructing other in operator= uses the copy constructor, which does not create a new copy of the pointed to value. Your copy constructor may not even copy k since it is a POD type and therefore not necessarily default constructed or default copied.

Then when it is destructed it tries to destroy it twice. Or depending on random factors like stack layout, it may not copy k at all and then it tries to delete an invalid pointer.

Answer 3

Your issue is in Vector2(const Vector2 &other)

You use this constructor in your operator = implicitly by passing by value; but you fail to assign k to any value in that constructor.

This results in swap replacing a valid k with an invalid k, and then deleting the invalid k; resulting in your crash.

Answer 4

It's actually very simple: Your copy constructor does not make a copy. In fact, it does not initialize any member, so any instance created by this constructor is filled with crap.

For the call of operator=(Vector2 other) the copy constructor is called to create other (this is the point of the rule of three), so other is filled with crap. Then you swap the valid k of this (aka v1) with the crappy k of other.

Then, when the destructor of v1 is called, it calls delete[] k on a crappy k --> access violation.

Solution

Make your copy constructor make a copy. Or at least, make it properly initialize k (e.g. to nullptr).