CODEWITHSUNDEEP
c++destructor
Plog
Plog

Reputation: 9622

C++: Destructor being called before it should go out of scope?

I am having a problem with a destructor being called for a class at the end of a subroutine even though it should be defined outside of the scope of the subroutine.

Here is the smallest piece of code I have that displays my problem:

#include <iostream>
using namespace std;

class Foo {
private:

    double *array;

public:

Foo(int N) {
   array = new double[N];
   for (int i=0; i<N; i++) {
       array[i]=0;
   }
}

~Foo() {
   delete[] array;
}
};

void subroutine(Foo x) {
   cout << "Hello!" << endl;
}

int main() {
   Foo bar(10);
   subroutine(bar);
   subroutine(bar);
}

Now the destructor for the object bar here gets called after the first subroutine finishes even though it's scope should be the whole of the main() function? This means that when I call the second subroutine the destructor is called again and I get a memory leak.

I have found I can fix this by calling by reference in the subroutine but I am not very satisfied with this fix since I dont understand why it didn't work in the first place. Can anyone shed some light on this for me?

Thanks.

Upvotes: 6

Views: 2927

Answers (4)

user213313
user213313

Reputation:

You are passing bar by value to subroutine so a copy is being created. To avoid making a copy pass it by reference:

void subroutine(Foo& x)
{
    cout << "Hello!" << endl;
}

You can prevent accidental copies of your class by declaring the copy constructor and copy assignment operator private like this:

class Foo {
private:

    double *array;

    Foo(const Foo&);
    Foo& operator=(const foo&);

public:
    ...
};

Then you get a compilation error instead. If you really need to be able to make a copy of your class then you will actually need to implement these functions to perform a "deep copy" (or better yet use std::vector<float> and let that manage the memory for you including safe copying).

Upvotes: 6

Vyktor
Vyktor

Reputation: 20997

When you call void subroutine(Foo x) { your object bar is copied (thus destructor is called after function finishes).

Try using: void subroutine(Foo &x) {, it should work just fine.

Upvotes: 3

Benj
Benj

Reputation: 32398

The problem you're having is that you're passing your object by value:

void subroutine(Foo x) {

This is creating a temporary object and invoking the copy constructor/destructor of your object every time you call it.

Upvotes: 1

juanchopanza
juanchopanza

Reputation: 227370

You are passing a Foo by value to the subroutine function. This means it has it's own copy, which gets destroyed on exiting it's scope.

void subroutine(Foo x) {
   // x is a new Foo here. It will get destroyed on exiting scope,
   // resulting in a destructor call
}

Your main problem here is that you have not implemented a copy constructor, so the dynamically allocated array is not copied (only the pointer that points to it is). So, when you copy Foo objects, you have each copy referring to the same array. And each copy will try to destroy it.

You should follow the rule of three and implement an assignment operator and a copy constructor that make a "deep copy" of the array, such that each Foo object owns its own array.

Upvotes: 21

Related Questions