Pool allocators with virtual destructors

Question

I am working on an old C++03 code base. One section looks something like this:

#include <cstddef>

struct Pool
{ char buf[256]; };

struct A
{ virtual ~A() { } };

struct B : A
{
  static void *operator new(std::size_t s, Pool &p) { return &p.buf[0]; }
  static void operator delete(void *m, Pool &p) { } // Line D1
  static void operator delete(void *m) { delete m; } // Line D2
};

Pool p;

B *doit() { return new(p) B; }

That is, B derives from A, but instances of B are allocated from a memory pool.

(Note that this example is slightly oversimplified... In reality, the pool allocator does something non-trivial, so the placement operator delete on line D1 is required.)

Recently, we enabled more warnings on more compilers, and Line D2 elicits the following warning:

warning: deleting ‘void*’ is undefined [-Wdelete-incomplete]

Well, yes, obviously. But since these objects are always allocated from a Pool, I figured there was no need for a custom (non-placement) operator delete. So I tried removing Line D2. But that resulted in a compilation failure:

new.cc: In destructor ‘virtual B::~B()’: new.cc:9:8: error: no suitable ‘operator delete’ for ‘B’ struct B : A ^ new.cc: At global scope: new.cc:18:31: note: synthesized method ‘virtual B::~B()’ first required here B *doit1() { return new(p) B; }

A little research determined that the problem is B's virtual destructor. It needs to invoke the non-placement B::operator delete, because someone somewhere might try to delete a B via an A *. Thanks to name hiding, line D1 renders the default non-placement operator delete inaccessible.

My question is: What is the best way to handle this? One obvious solution:

static void operator delete(void *m) { std::terminate(); } // Line D2

But this feels wrong... I mean, who am I to insist that you must allocate these things from the pool?

Another obvious solution (and what I am currently using):

static void operator delete(void *m) { ::operator delete(m); } // Line D2

But this also feels wrong, because how do I know I am calling the right deletion function?

What I really want, I think, is using A::operator delete;, but that does not compile ("no members matching ‘A::operator delete’ in ‘struct A’").

Related but distinct questions:

Why is delete operator required for virtual destructors

Clang complains "cannot override a deleted function" while no function is deleted

[Update, to expand a bit]

I forgot to mention that the destructor for A does not really need to be virtual in our current application. But deriving from a class with a non-virtual destructor causes some compilers to complain when you crank up the warning level, and the original point of the exercise was to eliminate such warnings.

Also, just to be clear on the desired behavior... The normal use case looks like this:

Pool p;
B *b = new (p) B;
...
b->~B();
// worry about the pool later

That is, just like most uses of placement new, you invoke the destructor directly. Or call a helper function to do it for you.

I would not expect the following to work; in fact, I would consider it an error:

Pool p;
A *b_upcast = new (p) B;
delete b_upcast;

Detecting and failing on such erroneous usage would be fine, but only if it can be done without adding any overhead to the non-erroneous cases. (I suspect this is not possible.)

Finally, I do expect this to work:

A *b_upcast = new B;
delete b_upcast;

In other words, I want to support but not require using the pool allocator for these objects.

My current solution mostly works, but I am concerned that the direct call to ::operator delete is not necessarily the right thing.

If you think you have a good argument that my expectations for what should or should not work are wrong, I would like to hear that, too.

Answer 1

Interesting problem. If I understood it correctly, what you want to do is to chose the right delete operator depending on whether it was allocated via the pool or not.

You could store some extra information about that at the beginning of the allocated block from the pool.

Since B can't be allocated wihout a Pool, you just have to forward to the placement deleter inside the normal delete(void*) operator using the bit of extra information about the pool.

Operator new will do store that part at the beginning of the allocated block.

UPDATE: Thanks for the clarification. The same trick still works with some minor modification. Updated code below. If thats still not what you want to do, then please provide some positive and negative test cases to define what should and what shouldn't work.

struct Pool
{
    void* alloc(size_t s) {
        // do the magic... 
        // e.g. 
        //    return buf;
        return buf;
    }
    void dealloc(void* m) {
        // more magic ... 
    }
private:

    char buf[256];
};
struct PoolDescriptor {
    Pool* pool;
};


struct A
{
    virtual ~A() { }
};

struct B : A
{
    static void *operator new(std::size_t s){
        auto desc = static_cast<PoolDescriptor*>(::operator new(sizeof(PoolDescriptor) + s));
        desc->pool = nullptr;
        return desc + 1;
    }

    static void *operator new(std::size_t s, Pool &p){
        auto desc = static_cast<PoolDescriptor*>(p.alloc(sizeof(PoolDescriptor) + s));
        desc->pool = &p;
        return desc + 1;
    }
    static void operator delete(void *m, Pool &p) {
        auto desc = static_cast<PoolDescriptor*>(m) - 1;
        p.dealloc(desc);
    }
    static void operator delete(void *m) {
        auto desc = static_cast<PoolDescriptor*>(m) - 1;
        if (desc->pool != nullptr) {
            throw std::bad_alloc();
        }
        else {
            ::operator delete (desc);
        } // Line D2
    }
};


Pool p;
void shouldFail() { 
    A* a = new(p)B;
    delete a;
}
void shouldWork() { 
    A* a = new B;
    delete a;
}

int main()
{
    shouldWork();
    shouldFail();
    return 0;
}

Answer 2

It's really difficult to understand what are you going to achieve with this code, since you stripped important bits of it.

Do you aware that static void operator delete(void *m, Pool &p) { } is called only if constructor of B throws an exception?

15) If defined, called by the custom single-object placement new expression with the matching signature if the object's constructor throws an exception. If a class-specific version (25) is defined, it is called in preference to (9). If neither (25) nor (15) is provided by the user, no deallocation function is called.

It means that in the current example this operator delete (D1) will never be called.

To me it looks pretty strange to have a base class A with virtual destructor, and insist that the semantics of the delete call is different, depending on the way the object was created.

If you really need the base class A, and added virtual destructor just to silence the warnings, you can make the destructor protected in A instead of making it virtual. Like this -

struct A
{
protected:
  ~A() { }
};

struct B final : public A
{
  ~B() = default;

  static void *operator new(std::size_t s, Pool &p) { return &p.buf[0]; }
  static void operator delete(void *m, Pool &p) {} // Line D1

  static void operator delete(void *m) {} // Line D2

};