Storage access via reinterpret_cast and std::launder

Question

I created a memory pool in order to use it as placement new for objects and I use the free slots to have a free-list in order to reuse the slots:

template<class T>
class ObjectPool {
public:
    ObjectPool( std::size_t cap );
    virtual ~ObjectPool();
    void* allocate() noexcept(false);
    void deallocate( void* ptr );
private:
    template<typename M>
    static constexpr const M& max( const M& a, const M& b ) {
        return a < b ? b : a;
    }
    using storage = typename std::aligned_storage<max(sizeof(T), sizeof(T*)), std::alignment_of<T>::value>::type;
    storage* pool;
    std::mutex mutex;
    std::size_t capacity;
    std::size_t counter;
    T* deletedItem;
};

template<class T>
ObjectPool<T>::ObjectPool( std::size_t cap ) :
        capacity(cap), counter(0), deletedItem(nullptr) {
    if ( capacity > 0 )
        pool = ::new storage[cap];
    else
        pool = nullptr;
}

template<class T>
ObjectPool<T>::~ObjectPool() {
    ::delete[] pool;
}

template<class T>
void* ObjectPool<T>::allocate() noexcept(false) {
    std::lock_guard<std::mutex> l(mutex);
    if ( deletedItem ) {
        T* result = deletedItem;
        deletedItem = *(reinterpret_cast<T**>(deletedItem)); //<-----undefined behavior??
        return result;
    }

    if ( counter >= capacity ) {
        throw std::bad_alloc();
    }

    return std::addressof(pool[counter++]);
}

template<class T>
void ObjectPool<T>::deallocate( void* ptr ) {
    std::lock_guard<std::mutex> l(mutex);
    *(reinterpret_cast<T**>(ptr)) = deletedItem;  //<-----undefined behavior??
    deletedItem = static_cast<T*>(ptr);
}

I'm trying to understand if this class has an undefined behavior under C++17 standard. Is it needed to use std::launder? From my understanding it isn't since only void pointers and T pointers are involved. In addition when in the deallocate the object has already been destroyed, so it should be safe.

Answer 1

Your code exhibits UB under every C++ standard, even C++20 which allows (under certain circumstances) implicit object creation. But not for reasons having to do with launder.

You can't just take a piece of memory, pretend there's a T* there, and treat it like one exists. Yes, even for fundamental types like pointers. You have to create objects before you use them. So if you have a disused piece of memory, and you want to shove a T* into it, you need to create one with placement-new.

So let's rewrite your code (note that this compiles, but is otherwise untested; the main point is that you must create the elements of your linked list):

template<class T>
class ObjectPool {
public:
    ObjectPool( std::size_t cap );
    virtual ~ObjectPool();
    void* allocate() noexcept(false);
    void deallocate( void* ptr );
private:

    using empty_data = void*; //The data stored by an empty block. Does not point to a `T`.
    using empty_ptr = empty_data*; //A pointer to an empty block.

    static constexpr size_t entry_size = std::max(sizeof(T), sizeof(empty_data));
    static constexpr std::align_val_t entry_align =
      std::align_val_t(std::max(alignof(T), alignof(empty_data))); //Ensure proper alignment

    void* pool;
    std::mutex mutex;
    std::size_t capacity;
    //std::size_t counter;  //We don't need a counter; the pool is empty if `freeItem` is NULL
    empty_ptr freeItem; //Points to the first free item.
};

template<class T>
ObjectPool<T>::ObjectPool( std::size_t cap ) :
    pool(::operator new(entry_size * cap, entry_align)),
    capacity(cap)
{
    //Build linked-list of free items, from back to front.
    empty_data previous = nullptr; //Last entry points to nothing.
    std::byte *byte_pool = reinterpret_cast<std::byte*>(pool); //Indexable pointer into memory
    auto curr_ptr = &byte_pool[entry_size * capacity]; //Pointer to past-the-end element.
    do
    {
        curr_ptr -= entry_size;

        //Must *create* an `empty_data` in the storage.
        empty_ptr curr = new(curr_ptr) empty_data(previous);
        previous = empty_data(curr); //`previous` now points to the newly-created `empty_data`.
    }
    while(curr_ptr != byte_pool);

    freeItem = empty_ptr(previous);
}

template<class T>
ObjectPool<T>::~ObjectPool()
{
    ::operator delete(pool, entry_size * capacity, entry_align);
}

template<class T>
void* ObjectPool<T>::allocate() noexcept(false) {
    std::lock_guard<std::mutex> l(mutex);
    if(!freeItem) { throw std::bad_alloc(); } //No free item means capacity full.

    auto allocated = freeItem;
    freeItem = empty_ptr(*freeItem); //Next entry in linked list is free or nullptr.
    return allocated;
}

template<class T>
void ObjectPool<T>::deallocate( void* ptr ) {
    std::lock_guard<std::mutex> l(mutex);

    //Must *create* an `empty_data` in the storage. It points to the current free item.
    auto newFree = new(ptr) empty_data(freeItem);
    freeItem = newFree;
}