How to clean up resources that have been allocated by a virtual member function

Question

Let's say I have the following code:

class BaseMember
{
};

class DerivedMember : public BaseMember
{
};

class Base
{
private:
    BaseMember* mpMember;
protected:
    virtual BaseMember* initializeMember(void)
    {
        return new BaseMember[1];
    }

    virtual void cleanupMember(BaseMember* pMember)
    {
        delete[] pMember;
    }
public:
    Base(void)
        : mpMember(NULL)
    {
    }

    virtual ~Base(void)
    {
        cleanupMember(mpMember);
    }

    BaseMember* getMember(void)
    {
        if(!mpMember)
            mpMember = initializeMember();
        return mpMember;
    }
};

class Derived : public Base
{
protected:
    virtual BaseMember* initializeMember(void)
    {
        return new DerivedMember;
    }

    virtual void cleanupMember(BaseMember* pMember)
    {
        delete pMember;
    }
};

Base and BaseMember are parts of an API and may be subclassed by the user of that API, like it is done via Derived and DerivedMember in the example code.

Base initializes mpBaseMember by a call to it's virtual factory function initializeMember(), so that the derived class can override the factory function to return a DerivedMember instance instead of a BaseMember instance.

However, when calling a virtual function from within a base class constructor, the base implementation and not the derived class override gets called. Therefor I am waiting with the initialization of mpMember until it gets accessed for the first time (which of course implies, that the base class and any derived class, that may could get derived further itself, are not allowed to access that member from inside the constructor).

Now the problem is: Calling a virtual member function from within the base base destructor will result in a call of the base class implementation of that function, not of the derived class override. That means that I can't simply call cleanupMember() from within the base class destructor, as that would call it's base class implementation, which may not be able to correctly cleanup the stuff, that the derived implementation of initializeMember() has initialized. For example the base class and the derived class could use incompatible allocators that may result in undefined behavior when getting mixed (like in the example code - the derived class allocates the member via new, but the base class uses delete[] to deallocate it).

So my question is, how can I solve this problem? What I came up with is: a) the user of the API has to explicitly call some cleanup function before the Derived instance gets destructed. That can likely be forgotten. b) the destructor of the (most) derived class has to call a cleanup function to cleanup stuff which initialization has been triggered by the base class. That feels ugly and not well designed as ownership responsibilities are mixed up: base class triggers allocation, but derived class has to trigger deallocation, which is very counter-intuitive and can't be known by the author of the derived class unless he reads the API documentation thoroughly enough to find that information. I would really like to do this in a more fail-proof way than relying on the users memory or his reliability to thoroughly read the docs.

Are there any alternative approaches?

Note: As the derived classes may not exist at compile time of the base classes, static polymorphism isn't an option here.

Answer 1

Inspired by the ideas from https://stackoverflow.com/a/19033431/404734 I have come up with a working solution :-)

class BaseMember
{
};

class DerivedMember : public BaseMember
{
};

class BaseMemberFactory
{
public:
    virtual ~BaseMemberFactory(void);

    virtual BaseMember* createMember(void)
    {
        return new BaseMember[1];
    }

    virtual void destroyMember(BaseMember* pMember)
    {
        delete[] pMember;
    }
};

class DerivedMemberFactory : public BaseMemberFactory
{
    virtual BaseMember* createMember(void)
    {
        return new DerivedMember;
    }

    virtual void destroyMember(BaseMember* pMember)
    {
        delete pMember;
    }
};

class Base
{
private:
    BaseMemberFactory* mpMemberFactory;
    BaseMember* mpMember;
protected:
    virtual BaseMemberFactory* getMemberFactory(void)
    {
        static BaseMemberFactory fac;
        return &fac;
    }
public:
    Base(void)
        : mpMember(NULL)
    {
    }

    virtual ~Base(void)
    {
        mpMemberFactory->destroyMember(mpMember);
    }

    BaseMember* getMember(void)
    {
        if(!mpMember)
        {
            mpMemberFactory = getMemberFactory();
            mpMember = mpMemberFactory->createMember();
        }
        return mpMember;
    }
};

class Derived : public Base
{
protected:
    virtual BaseMemberFactory* getMemberFactory(void)
    {
        static DerivedMemberFactory fac;
        return &fac;
    }
};

Answer 2

What about a modification of the factory pattern that would include the cleanup method? Meaning, add a attribute like memberFactory, an instance of a class providing creation, cleanup, as well as access to the members. The virtual initialization method would provide and initialize the right factory, the destructor ~Base would call the cleanup method of the factory and destruct it.

(Well, this is quite far from the factory pattern... Perhaps it is known under another name?)

Answer 3

If you really want to do this sort of thing you can do it like this:

class Base {
    BaseMember* mpMember;

  protected:
    Base(BaseMember *m) : mpMember(m) {}

    virtual void doCleanupMember(BaseMember *m) { delete [] m; }

    void cleanupMember() {
      // This gets called by every destructor and we only want
      // the first call to do anything. Hopefully this all gets inlined.
      if (mpMember) {
        doCleanupMember(pmMember);
        mpMember = nullptr;
      }
    }

  public:
    Base() : mpMember(new BaseMember[1]) { }
    virtual ~Base(void) { cleanupMember(); }
};

class Derived : public Base {
  virtual void doCleanupMember(BaseMember *m) override { delete m; }

  public:
    Derived() : Base(new DerivedMember) {}
    ~Derived() { cleanupMember(); }
};

However there are reasons this is a bad idea.

First is that the member should be owned an exclusively managed by Base. Trying to divide up responsibility for Base's member into the derived classes is complicated and just asking for trouble.

Secondly the ways you're initializing mpMember mean that the member has a different interface depending on who initialized it. Part of the problem you've already run into is that the information on who initialized the member has been destroyed by the type you get to ~Base(). Again, trying to have different interfaces for the same variable is just asking for trouble.

We can at least fix the first problem by using something like shared_ptr which lets up specify a deleter:

class Base {
    std::shared_ptr<BaseMember> mpMember;
  public:
    Base(std::shared_ptr<BaseMember> m) : mpMember(m) { }
    Base() : mpMember(std::make_shared<BaseMember>()) { }
    virtual ~Base() {}
};

class Derived : virtual public Base {     
  public:
    Derived()
      : Base(std::shared_ptr<BaseMember>(new DerivedMember[1],
                                         [](BaseMember *m){delete [] m;} ) {}
};

This only hides the difference in the destruction part of the member's interface. If you had an array of more elements the different users of the member would still have to be able to figure out if mpMember[2] is legal or not.

Answer 4

First of all, you must use RAII idiom when developing in C++. You must free all your resources in destructor, of course if you don't wish to fight with memory leaks.

You can create some cleanupMember() function, but then you should check your resources in destructor and free them if they are not deleted (as cleanupMember can be never called, for example because of an exception). So add destructor to your derived class:

virtual ~Derived()
{
    Derived::cleanupMember(mpMember);
}

and manage the member pointer in the class itself.

I also recommend you to use smart pointers here.

Answer 5

Never never never call virtual methods in constructor/destructor because it makes strange results (compiler makes dark and weird things you can't see).

Destructor calling order is child and then parent

You can do like this (but there is probalby a better way) :

private:
    // private destructor for prevent of manual "delete"
    ~Base() {}

public:
    // call this instead use a manual "delete"
    virtual void deleteMe()
    {
        cleanupMember(mpMember);
        delete this; // commit suicide
    }

More infos about suicide : https://stackoverflow.com/a/3150965/1529139 and http://www.parashift.com/c++-faq-lite/delete-this.html

PS : Why destructor is virtual ?

Answer 6

Let mpMember be protected and let it be initialized in derived class constructor and deallocated in derived destructor.