3
\$\begingroup\$

I'm implementing a object pool with shared_ptr to access with multiple thread. Any suggestions to improve my implementation

template <typename T>
class Section : public std::enable_shared_from_this<Section<T>>
{
public:

template <typename... E>
Section(std::size_t size, E... args)
    : std::enable_shared_from_this<Section<T>>()
    , m_mem(size,T(args...))
{
    for(auto& itr: m_mem)
    {
        destroy(&itr);
        m_objs.push(&itr);
    }
}
virtual ~Section() = default;

void isEmpty() const
{
    std::unique_lock<std::mutex> lk(m_lock);
    return m_objs.empty();
}
template <typename... E>
std::shared_ptr<T> create(E... args)
{
    T* obj = getObject();       
    if(obj != nullptr)
    {
        new (obj) T(args...);
    }
    return std::shared_ptr<T>(obj,[this](T* ptr)->void
    {
        destroy(ptr);
        std::unique_lock<std::mutex> lk(m_lock);
        m_objs.push(ptr);
    });
}

private:
std::list<T> m_mem;
std::queue<T*> m_objs;
mutable std::mutex m_lock;


T* getObject()
{
    std::unique_lock<std::mutex> lk(m_lock);
    if (!m_objs.empty())
    {
        T* obj = m_objs.front();
        m_objs.pop();
        return obj;
    }
    return nullptr;
}

void destroy(T* obj)
{
    obj->~T();
}       
};
\$\endgroup\$
4
\$\begingroup\$

Small nitpicks

  • The constructor and create could profit from perfect forwarding args (so references of all kinds can be used.

    template<typename... E>
    Section(size_t size, E&&... args) :
        std::enable_shared_from_this<Section<T>>(),
        m_mem(size, T(std::forward<E>(args)...))
        { /* ... */ }
    
  • Do you have any specific reason to prefer std::list<T> over std::vector<T> for m_mem? std::vector<T> would use less memory (no pointers from each element to the next), and the memory used would be contiguous (so iterating over it is very likely faster due to cache effects).

Correctness

Every std::shared_ptr<T> returned by Section<T>::create() might be dangling. This can happen if the Section<T> gets destroyed while there are still std::shared_ptr<T> pointing to contained objects.

This is a result of the std::shared_ptr<T> not owning the pointer.

If you can guarantee that there won't be any std::shared_ptr<T> pointing into the Section<T> when the Section<T> is destroyed, this problem easily disappears.

If this can't be guaranteed, there are two options:

  1. Introducing your own smart pointer type that checks the Section<T> for validity every time it's dereferenced, as the standard smart pointers (std::unique_ptr, std::shared_ptr and std::weak_ptr) are not designed for this task.
  2. (Ab-)using the returned std::shared_ptr<T> to keep the section alive. This can be done in the lamda init capture of the custom deleter.

    return std::shared_ptr<T>(obj, [section = shared_from_this()](T *ptr) -> void
    {
        section->destroy(ptr);
        std::unique_lock<std::mutex> lk(section->m_lock);
        section->m_objs.push(ptr);
    });
    

    Note: This might keep the section alive far longer than intended! (Some might also say that this is an ugly hack...)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.