Clobbering automatic implicit objects

Question

Let's assume the following scenario. I have an implicitly shared class as defined below:

class MyClass
{
public:
    MyClass (void) {
        data = new Data();
        data->refs = 1;
        data->dummy = 32;
    }

    ~MyClass (void) {
        if (--data->refs == 0)
            delete data;
    }

    MyClass (const MyClass& c) {
        ++(data = c.data)->refs;
    }

    MyClass& operator = (const MyClass& c) {
        ++(data = c.data)->refs;
        return *this;
    }

private:
    struct Data {
        int refs;
        int dummy;
    } *data;
};

The idea is that when this class gets copied, the internal pointer to the data gets copied and the number of references to that data is incremented. However, consider the following:

int main (void)
{
    MyClass c1;
    c1 = MyClass();
    c1 = MyClass();
    return 0;
}

My understanding is that there are three instances of MyClass and only the last instance gets deallocated. If this is the case, what can I do to avoid these scenarios and ensure that every instance of MyClass gets cleaned up?

Answer 1

In your assignment operator you need to make sure that the assigned to object is cleaned up before overwriting its value by a new value. After all, this left hand side of the assignment already references a value. The easiest way to write an assignment operator is to leverage the existing copy constructor and destructor and use a function swap() to exchange the two objects:

MyClass& MyClass::operator = (MyClass c) {
    this->swap(c);
    return *this;
}

void MyClass::swap(MyClass& other) {
    std::swap(this->data, other.data);
}

This way the value is already copied in the value c and the current value is exchanged with the one held by c. When c gets destroyed the reference count gets decremented as needed.

Note, that std::share_ptr<T> already does reference counting using a rather fancy mechanism: you might want to use the standard class rather than rolling your own.

Answer 2

MyClass& operator = (const MyClass& c) {
    ++(data = c.data)->refs;
    return *this;
}

This is broken. Before you clobber this.data, you need to decrement its reference and delete it if the reference drops to zero.