0

I am really stuck on this problem. I have an application with a memory leak. To get rid of the problem I need to call destructors for classes.

For 2 layers of classes, this problem is trivial, but in this situation, there's 3 layers of classes and I cannot find any way to get this to compile:

#include<iostream>

using namespace std;

/* *MUST* be an abstract class and contains data that must be freed */
class C { 
public:
  virtual ~C()=0;
};

/* Contains concrete methods and contains data that has to be freed */
class B:public C { 
public:
  virtual ~B();
};

/* contains no data, only methods nothing needs to be freed in here */
class A:public B {
public:
  ~A();
};

int main(){
  C *c = new A;
  return 0;
}

When I compile the code I get the error:

martyn@Hades ~ $ gcc deleteme.cpp -std=c++11
/tmp/cc7nxaw5.o: In function `main':
deleteme.cpp:(.text+0x12): undefined reference to `operator new(unsigned int)'
/tmp/cc7nxaw5.o: In function `__static_initialization_and_destruction_0(int, int)':
deleteme.cpp:(.text+0x4b): undefined reference to `std::ios_base::Init::Init()'
deleteme.cpp:(.text+0x62): undefined reference to `std::ios_base::Init::~Init()'
/tmp/cc7nxaw5.o: In function `B::B()':
deleteme.cpp:(.text._ZN1BC2Ev[_ZN1BC5Ev]+0x16): undefined reference to `vtable for B'
/tmp/cc7nxaw5.o: In function `A::A()':
deleteme.cpp:(.text._ZN1AC2Ev[_ZN1AC5Ev]+0x16): undefined reference to `vtable for A'
/tmp/cc7nxaw5.o:(.rodata._ZTV1C[_ZTV1C]+0x8): undefined reference to `__cxa_pure_virtual'
/tmp/cc7nxaw5.o:(.rodata._ZTV1C[_ZTV1C]+0xc): undefined reference to `__cxa_pure_virtual'
/tmp/cc7nxaw5.o:(.rodata._ZTI1C[_ZTI1C]+0x0): undefined reference to `vtable for __cxxabiv1::__class_type_info'
collect2: error: ld returned 1 exit status

Can anyone figure out a way that I can rewrite the destructors, so that destructors for A, B and C are all called?

I am tearing my hair out with this one, any help would be appreciated. I can't however change the requirements in comments or the inheritance hierarchy, to do so would require me to rewrite the entire application.

Much appreciated.

Edit: thanks so much for the comments below to the question, the solution that works for me is:

#include<iostream>

using namespace std;

/* *MUST* be an abstract class and contains data that must be freed */
class C { 
public:
  virtual ~C()=0;
};

C::~C(){
  cout<<"destroying C"<<endl;
}

/* Contains concrete methods and contains data that have to be freed */
class B:public C { 
public:
  ~B();
};

B::~B(){
  cout<<"destroying B"<<endl;
}

/* contains no data, only methods nothing needs to be freed in here */
class A:public B {
public:
  ~A();
};

A::~A(){
  cout<<"destroying A"<<endl;
}

int main(){
  C *c = new A;
  delete c;
  return 0;
}

Thanks very much for your advice.

curiousguy
  • 8,038
  • 2
  • 40
  • 58
Owl
  • 1,446
  • 14
  • 20
  • 3
    Give your destructors definition and maybe it'll work better. `~B(){}` – Edward Strange Apr 05 '16 at 00:21
  • 1
    Also, adding `delete c;` will actually call them. Or better : use smart pointers. – O'Neil Apr 05 '16 at 00:31
  • 1
    This has me confused: `/* *MUST* be an abstract class and contains data that must be freed */` and `virtual ~C()=0;`. `C` has data to free. Only `C` should know enough about `C` to be able to do this. `C` should have a destructor. Clarifying. You can have a pure virtual destructor and still provide a destructor. Weird, but true. – user4581301 Apr 05 '16 at 00:31
  • Hey guys, I think you have partially solved my problem, if I specify the destructors as well as make it pure virtual in the abstract class, that now works, I can get B and C destructors to call, although I still cannot invoke a destructor in A. Not sure whether to change the question. Actually it may be better if i do now with that new information. – Owl Apr 05 '16 at 00:48
  • Oh wait, that has completely solved my problem. I need to have a the base, C, class pure virtual with a definition, then classes B and A non-virtual and then all the destructors are called. I thought B would have to be an impure virtual destructor or something. Thanks so much guys :) – Owl Apr 05 '16 at 00:54

1 Answers1

1

The solution here is to implement destructors in the classes that need them and let the compiler handle the rest.

First of all, C has some data that needs to be freed, so C gets a destructor like this:

class C{
    int* cp;

public:

    C(){ cp = new int;} 

    virtual ~C(){
        delete cp;
        cout << "~C()" << endl;
    }

    virtual void someMethod()=0;
};

Class B should implement the pure virtual function and have data that needs to be freed as well, so:

class B: public C{
    int* bp;
    public:

    B(){ bp = new int;} 

    virtual ~B(){
        delete bp;
        cout << "~B()" << endl;
    }

    virtual void someMethod(){//some Implementation
    }
};

Finally Class A provides just some methods, so we don't need a destructor. But i will include one here as well for demonstration:

class A: public B{
    public:
    virtual ~A(){
        cout << "~A()" << endl;
    }
};

Constructing this class and calling the destructor with delete will properly call all three destructors:

int main(){
    C *c = new A;
    delete c;
    return 0;
}

Output:

~A()
~B()
~C()

Try it online

As a note: It looks like in the code you posted you tried to use the declaration of a pure virtual destructor to make your class abstract. But if you have to free ressources in the class, you need to use another function for that purpose, if you don't have a pure virtual function anyways.

Edit:
Reading here one can learn that it is indeed valid to provide a pure-specifier and a definition for the same function - which applies to destructors as well. So you can indeed do this:

class C{
    virtual ~C() = 0;
};

C::~C(){ /*Implementation here*/}

At the end, as soon as at least one function is pure virtual (=0) your class is abstract. If this is the destructor or some other function does not matter. But if your class needs to free ressources, you need a body for your destructor.

Community
  • 1
  • 1
Anedar
  • 4,235
  • 1
  • 23
  • 41
  • Thanks for your answer, one of the questions i had remaining from this was if you could layer the virtual inheritance of the destructors, it looks like you can. Thanks very much :) I will have to test these solutions with the actual code base because it's crucial that C is an abstract class, as there's a vector of polymorphic class instances and I open up a Pandoras Box of problems if C is not abstract. – Owl Apr 05 '16 at 01:14
  • In this case C is abstract - it is just not the destructor that makes it abstract but `someMethod()` – Anedar Apr 05 '16 at 01:18
  • That works perfectly - also it fixed my memory leak. Thank you. I now have a memory leak free neural network! – Owl Apr 05 '16 at 23:09