Returning a Class that Stores on Heap

Question

I'm having a memory-related crash in my project. I managed to reduce it down to the following "toy" project:

class Foo {
public:

    Foo() : bar(nullptr) {
        bar = new int(3);
    }

    ~Foo() {
        if (bar) {
            delete bar;
            bar = nullptr;
        }
    }

private:
    int* bar;
};

Foo test() {
    Foo foo;
    return foo;
}

int main() {
    test();
             // <--- Crash!
    return 0;
}

I cannot figure out why I'm crashing at the line specified. This is what I've gathered so far, please do correct me if I'm wrong:

Basically, I'm creating foo on the stack in test(). Foo allocates some memory on the heap. All is well. Then I try to return foo. foo is returned, but it is immediately destroyed. Then, when exiting, I'm again trying to destroy foo; and hence I'm calling Foo's destructor twice, and I crash. What I do not get is why this is a problem. I'm checking for Foo::bar being null before deleting it, and if I do delete it, I set it to null afterwards.

Why should this cause a crash? How can I "fix" this? What am I doing wrong? I'm so confused! :(

Update: The main reason for why this is happening is because of lack of a copy constructor, as stated in the answers below. However, my original project did have copy constructor, or so I thought. Turns out if your class is derived, you must explicitly create a copy constructor for the derived class. Base(const Derived& other) does not count as a copy constructor!

score 9 · Accepted Answer · edited May 23 '17 at 12:12

9

You violated the rule of three.

When you're manually managing memory inside your constructor and destructor, you MUST also provide a copy constructor and assignment operator, otherwise your program will double-delete and use memory after delete.

In your particular example, you have the compiler-provided copy constructor. When foo is copied to the return value of test, you have two objects with the same bar pointer. The local foo goes out of scope and is destroyed, then its bar is set to nullptr. But the copy, the return value, still has a non-null pointer. Then it is destroyed also, and deletes the same memory again.

edited May 23 '17 at 12:12

Community

1
1

answered Oct 06 '11 at 03:21

Ben Voigt

277,958
43
419
720

Yup. This was it. And it wasn't because I didn't have a copy constructor. In my actual project, the class was derived from a base class. When I wrote it, I thought `Base(const Derived& other)` and `Base& operator=(const Derived& other)` would count as copy constructor and assignment operators respectively. Turns out they don't! And I learn something today! Thanks for pointing me in the right direction. – Zeenobit Oct 06 '11 at 03:42

score 2 · Answer 2 · answered Oct 06 '11 at 03:33

You need to implement a copy-constructor and, most likely, an assignment operator.

Really, though, your problem lies in manually managing memory. This is almost always a terrible idea, as it is one of the most common ways bugs creep into your code. Use shared_ptrs instead of manually managing memory in your code, and it is amazing how much easier your code is to maintain!

Returning a Class that Stores on Heap

2 Answers2