user1313867
user1313867

Reputation: 31

could you tell me why does this code crash?

So I'm curious of the reason the following code crashes. Will appreciate help.

#include <iostream>
using namespace std;

class temp
{    
  public:

    temp(int i)
    {
        intPtr = new int(i);
    }

    ~temp()
    {
        delete intPtr;
    }

  private:
    int* intPtr;
};

void f (temp fInput)
{
    cout << "f called" << endl;
}

int main()
{
    temp x = 2;
    f(x);
    return 0;
}

Upvotes: 2

Views: 159

Answers (4)

user1203803
user1203803

Reputation:

The pointer is copied when x is passed (implicit copy constructor) and the destructor is called twice (before the function returns and before main returns), thus the memory is deleted twice.

Use std::shared_ptr<int> here instead instead of a raw int pointer (assuming you want the behavior to be the same, i.e. reference the same int from both temps; otherwise, implement the copy constructor, move constructor and assignment operator yourself).

#include <memory>

class temp {    
public:
  temp(int i) : intPtr(std::make_shared<int>(i)) {

  }

private:
  std::shared_ptr<int> intPtr; // reference counting ftw
};

Upvotes: 5

josephthomas
josephthomas

Reputation: 3276

The crash occurs because of the way you are passing x.

After the scope of the f function, x's destructure will be called and will delete intPtr.

However, that will delete memory that is still in scope for main. Therefor, after return 0 is called, it will try to delete memory that already exists as you are calling delete twice on the same pointer.

To fix this error, change

void f (temp fInput)

to

void f (const temp& fInput)

Alternatively, you could look into using a std::shared_ptr.

Upvotes: 5

JaredPar
JaredPar

Reputation: 754645

The problem you're hitting here is a double delete. Because you didn't define any copy constructors here C++ is happily doing so for you. The implementation does this by performing a shallow copy of all of the contents.

f(x);  

This line works by creating a copy of x and passing it to f. At this point there are 2 temp instances which own a single intPtr member. Both instances will delete that pointer and this is likely what is causing your crash.

To work around this you can take a number of steps

  1. Use a pointer type meant for sharing like shared_ptr<T>
  2. Create an uncallable copy constructor which forces the instance to be passed by ref

An example of #2 is

class temp {
  temp(const temp& other);
  temp& operator=(const temp& other);
public:
  // Same
};

Now the f(x) line simply won't compile because it can't access the necessary copy constructor. It forces it to instead redefine f to prevent a copy.

void f(const temp& fInput) {
  ...
}

Upvotes: 1

Ed Swangren
Ed Swangren

Reputation: 124642

You are violating The Rule of Three

You maintain a pointer member and you pass a copy of the object to the function f. So, the end result is that you call delete twice on the same pointer.

Upvotes: 5

Related Questions