0

I have the following code, which results in an double free or corruption:

class A{
   public:
   ~A(){
    cout<<"delete A"<<endl;
   }
};
class Test{
   public:
     A* a;
     ~Test(){
        cout<<"delete Test"<<endl;
        delete a;
     }
};
class Test2{
  public:
    A* a;
    ~Test2(){
       cout<<"delete Test2"<<endl;
       delete a;
    }
};
int main()
{
  A* a=new A();
  Test test;
  test.a=a;
  Test2 test2;
  test2.a=a;
  return 0;
}

I know why this occurs: ~Test2 is firstly called which deletes the memory, then ~Test attempts to do the same.

But how can I fix or eliminate the error? Assume that I'm unable to modify either class.

Tas
  • 7,023
  • 3
  • 36
  • 51
cyhone
  • 293
  • 1
  • 4
  • 12
  • 1
    Possible duplicate of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Richard Critten Jul 11 '17 at 02:00
  • 1
    If `Test` and `Test2` can't be modified, then you must avoid getting them both own the same instance of `A`. Create two separate `A`s, one for each. – Igor Tandetnik Jul 11 '17 at 02:01
  • Your options are to create two separate instances of `A`, or change one/both to `nullptr` before they destruct – Tas Jul 11 '17 at 02:07
  • You fix it by actually learning and understanding how memory allocation works. You are allocating one instance of an object, and shoving the pointer to that instance into two other objects, with each one of them attempting to `delete` it. This is an obvious fail, by design. Things don't work this way. A full explanation of dynamic memory allocaiton is beyond the scope of a few short paragraphs on stackoverflow.com. Open your C++ book, and keep reading it. – Sam Varshavchik Jul 11 '17 at 02:15

1 Answers1

0

These classes seem terrible, but assuming they are simply to demonstrate some real world code, you have a few options (which don't modify the classes).

As proposed in the comments by @Igor, you could simply give the classes two separate instances of A. This may or may not suit your needs however, as perhaps they are meant to work on the same instance but it wasn't made clear:

int main()
{
    A* a1 = new A();
    A* a2 = new A();
    Test test;
    Test2 test2;
    test.a = a1;
    test2.a = a2;
}

If you needed for them to operate on the same instance, you can simply set one to nullptr before it destructs. It would probably make more sense to change them both, and manage the memory yourself though:

int main()
{
    std::unique_ptr<A> a = std::make_unique<A>();
    Test test;
    Test2 test2;
    test.a = a.get();
    test2.a = a.get();
    // Use Test etc:
    ...
    test.a = nullptr;
    test2.a = nullptr;
    // a goes out of scope and cleans itself up
}

Further depending on your uses, you could make a TestWrapper class. This works for the classes provided, but the two classes you provided might provide all sorts of different functionality, in which case you could consider making T public:

template <typename T>
struct TestWrapper
{
    TestWrapper(A* a) { t.a = a; }
    ~TestWrapper() { t.a = nullptr; }
private:
    T t;
};

int main()
{
    std::unique_ptr<A> a = std::make_unique<A>();
    {
        TestWrapper<Test> test(a.get());
        TestWrapper<Test2> test2(a.get());
    }
}
Tas
  • 7,023
  • 3
  • 36
  • 51