Unable to exit as destructor errors cause execution of main() to halt upon exit

Question

I have narrowed my problem down to passing 2 objects (which contain pointer data members) to a simple void function. The function returns clean, but when main() attempts to exit, it can not reclaim the first of the 2 objects. Here is a sample piece of code that shows the issue - along with print statements to show the address's of the objects as they are constructed, passed, and destructed.

If I only call "print1" - the program runs fine. However, if I call "printboth" - then the object "myNumbers" can not be freed. I can also make the error go away by removing the destructor statement:

 delete [] number;

but I don't think this is a good idea.

Anyone have any ideas?

class dummy
{
public:
    dummy() {
        number = new int[1];
        currentPos = -1;
        std::cout<<"default constructor called for "<<this<<std::endl;

    }
    dummy(int len) {
        number = new int[len];
        currentPos = -1;
        std::cout<<"parameterized constructor called for "<<this<<std::endl;

    }
    ~dummy() {
        cout<<"Calling destructor for "<<this<<endl;
        delete [] number;
    }
    int getNextNumber() {
        currentPos++;
        return number[currentPos];
    }
    void setNumbers(int position, int value) {
        number[position] = value;
    }
private:
    int* number;
    int currentPos;
};

void print1(dummy);
void printboth(dummy, dummy);

int main() {
dummy myNumbers(3);
myNumbers.setNumbers(0,0);
myNumbers.setNumbers(1,1);


dummy myOtherNumbers(3);
myOtherNumbers.setNumbers(0,4);
myOtherNumbers.setNumbers(1,5);

cout<<"Address of myNumbers is      "<<&myNumbers<<endl;
cout<<"Address of myOtherNumbers is "<<&myOtherNumbers<<endl;

print1(myNumbers);
printboth(myNumbers, myOtherNumbers);

system("PAUSE");
return 0;
}

void print1(dummy num) {
cout<<"Address of num is      "<<&num<<endl;
for (int i=0;i<4;i++)
    cout<<"Dummy number1 is "<<num.getNextNumber()<<endl;
return;
}
void printboth(dummy num1, dummy num2) {
cout<<"Address of num1 is      "<<&num1<<endl;
cout<<"Address of num2 is      "<<&num2<<endl;
for (int i=0;i<4;i++) {
    cout<<"Dummy number1 is "<<num1.getNextNumber()<<endl;
    cout<<"Dummy number2 is "<<num2.getNextNumber()<<endl;
    }
return;
}

Answer 1

You didn't follow rule of three

The problem is that when you call print1 or printboth the compiler calls the default copy-constructor (since you didn't provide one). That copy-constructor sets the number member variable of the copy to the same value as the original. When the destructor is called on the copy, the memory is released. Your original object now points to memory that has already been released so when its destructor is called, you crash(Nik Bougalis).

void print1(dummy);
void printboth(dummy, dummy);

You could pass dummy by const reference to avoid unnecessary copy, but strong recommand you follow rule of three

void print1(const dummy& );
void printboth(const dummy&, const dummy&);

Note: You only created size =1 array which is not necessary at all, just use int number; as member. If number holds dynamically allocated array, try use std::vector<int>.

getNextNumber is flawed, when it's called multiple times, number[currentPos]; access boundry out of number which is undefined behavior.

int getNextNumber() {
        currentPos++;
        return number[currentPos];
    }

Which implies what suggested:

int getNextNumber() const {
      return number[currentPos];
   }