Delete object in handle class potentially causes undefined behavior

Question

I have the following piece of code (from Koening & Moo Accelerated C++ page 255) that defines a generic handle class Handle. Handle is used to manage the memory of objects:

#include <iostream>
#include <stdexcept>

///Handle
template <class T>
class Handle
{
  public:
    Handle() : p(0) {}
    Handle &operator=(const Handle &);
    T *operator->() const;
    ~Handle() { delete p; }
    Handle(T *t) : p(t) {}

  private:
    T *p;
};

template <class T>
Handle<T> &Handle<T>::operator=(const Handle &rhs)
{
    if (&rhs != this)
    {
        delete p;
        p = rhs.p ? rhs.p->clone() : 0;
    }
    return *this;
};

template <class T>
T *Handle<T>::operator->() const
{
    if (p)
        return p;
    throw std::runtime_error("error");
};

class test_classA
{
    friend class Handle<test_classA>;

  private:
    virtual test_classA *clone() const { return new test_classA; }

  public:
    virtual void run() { std::cout << "HiA"; }
};

class test_classB : public test_classA
{
  private:
    virtual test_classB *clone() const { return new test_classB; }

  public:
    virtual void run() { std::cout << "HiB"; }
};

int main()
{

    Handle<test_classA> h;
    h = new test_classA;
    h->run();

    return 0;
}

When I compile this using g++ -o main main.cpp -Wall I get the warning:

warning: deleting object of polymorphic class type ‘test_classA’ which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor]
     ~Handle() { delete p; }

I don't quite understand the warning. The handle class automatically deletes the pointer *p in the destructor regardless of its type, so where is the potential pitfall?

Answer 1

In C++, if you have a base class (here, test_classA) that has other classes derive from it (here, test_classB), you have to be careful about deleting pointers of type test_classA if those pointers might actually point at objects of type test_classB. Notice that you're doing precisely this in the code that you've written here.

If you do something like this, you need to give your base class (test_classA) a virtual destructor, like this:

class test_classA {
public:
    virtual ~test_classA() = default;
    // ...
};

This way, when C++ tries to delete a pointer of type test_classA, it knows that the pointer in question might not actually point at a test_classA object and will correctly call the right destructor.

This issue is completely independent of your wrapper type, by the way. You could get the same issue by writing

test_classA* ptr = new test_classB;
delete ptr; // <--- Warning! Not good unless you have a virtual dtor.