HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++11 Blocking connection pool with auto release

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withautoblockingreleasepoolconnection

Problem

Relatively new to C++. Please help me understand the potential issues with the following blocking object pool.

#pragma once
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

template 
struct DefaultDeleter {
    DefaultDeleter() {};
    void operator()(T* p) const {
        delete p;
    }
};

/// \brief A blocking object pool containing objects of type T.
/// Objects burrowed from the pool are automatically returned to the pool when they
/// are out of scope.
/// \n
/// When the pool is empty calls to borrow objects blocks until a resource is returned to the pool.
template >
class ObjectPool {
public:

    /// \brief Construct an object pool of size n.
    ///
    /// \param n The size of the pool.
    /// \param factory A factory method that returns a pointer to a resource.
    ObjectPool(size_t n, std::function factory) {
        for(size_t i = 0; i

    /// \brief burrow an object from the pool.
    /// When the borrowed pointer is out of scope the resource is returned to the pool.
    std::unique_ptr borrow() {
        std::unique_lock lock(mu);
        if (resources.empty()) {
            this->condition.wait(lock, [this]{return !this->resources.empty();});
        }
        auto resource = std::unique_ptr(this->resources.front(), [this](T* t){
            std::unique_lock lock(mu);
            resources.emplace_back(t);
            condition.notify_one();
        });
        this->resources.pop_front();
        return std::move(resource);
    }

private:
    std::deque resources;

    // synchronization
    std::mutex mu;
    std::condition_variable condition;
};


EDIT: Maybe ObjectPool is a bad term. The intention in here is that this is a ConnectionPool. A common use case looks like this

```
struct CloseDb
{
void operator()(sqlite3* db) const
{
sqlite3_close(db);
}
};

struct Resource {
std::shared_ptr> sqlitePool;
Resource() {

Solution

This is not what I would normally call an Object Pool (in the traditional sense of the term). That term I would use to refer to a pool of memory that returns "Uninitialized" memory (via a pointer) usually via an allocator so that a new operation could build the object (and the memory is returned after it is deleted).

I do like the term borrow() though as that is an accurate description of what is happening. You are temporarily giving ownership of objects that can be returned (in potentially a new state).

Your ownership semantics are wrong though. You return a unique_ptr<> indicating that ownership has been transferred to the borrower. But that is not actually true. The "pool" actually retains ownership as it calls the destructor on the object when it is destroyed (ONLY the owner of an object should try and delete it (reclaim the resource)).

Also if the "pool" is destroyed (thus destroying all the owned pointers); then the "borrowers" are left holding unqiue_ptr<> to objects that no longer exist (bad point 1). But when they do release them the "deleter" tries to put them back into a "pool" that no longer exists (bad point 2).

As it stands this code is just ill-advised.

You need to think some more about your ownership semantics.

Context

StackExchange Code Review Q#157687, answer score: 5

Revisions (0)

No revisions yet.