Raw pointer or std::shared_ptr in std::stack for a db connection pool

Question

I searched google and stackoverflow but did not find an answer for my particular usage. There is much good advice regarding raw pointers and the advantage of std::shared_ptr but not related to a std::stack.

I watched Herb Sutters talk on youtube about writing good C++14 code (https://www.youtube.com/watch?v=hEx5DNLWGgA) and one topic is why unique and shared pointers are useful.

So I rewrote my simple db connection pool from raw pointers to std::shared_ptr instead. After I top()/pop() the stack for a connection and interact with the db I push() it back.

With raw pointers in the constructor

Database::Database(const unsigned int connections) {
    for (int i = 0; i < connections; ++i) {
        try {
            auto* dbconn = new pqxx::connection(connectionString);
            dbpool.push(dbconn);
        } catch (const std::exception& e) {
            std::cerr << e.what() << std::endl;
        }
    }
}

And later on

auto *D = dbpool.top();
dbpool.pop();

std::string query = "select * from races order by racestart_at desc";
pqxx::nontransaction N(*D);
pqxx::result R(N.exec(query));

dbpool.push(D);

Using shared_ptr in constructor

auto* dbconn = new pqxx::connection(connectionString);
std::shared_ptr<pqxx::connection> s_dbconn(dbconn);
dbpool.emplace(s_dbconn);

And in the program

auto D = dbpool.top();
dbpool.pop();
<query db etc.>
dbpool.push(D);

Are raw pointers as good as shared pointers in this case? I don't see that a raw pointer can go astray that many places.

Answer 1

It's all about ownership. Use shared pointers where multiple pieces of code (objects) can own the pointer at the same time.

For your example with the pool it might be a good idea to just create an array (either std::array or a std::vector) of connection objects, and have another array (or stack in your case) of pointers (or indexes) to the objects in the first array. Then when a connection object is requested from the pool you return one as a raw pointer, and then you need a function to "return" the connection back to the pool so that it can be reused.

In this way only the pool actually owns the connection objects so it should be the only one deleting them. The other code just uses the object but should not delete it, and instead should return it to the pool once it has finished using it.

One way to enhance this would be to return a special object/class from the pool instead of the raw pointer. This object would be like a little RAII wrapper that managed the loan of the connection object. So ince it is destroyed it returns the connection object back to the pool. I believe you could use a unique_ptr with a custom deleter for this purpose.

Answer 2

Since you actually removes the pointer from the pool when someone request a resource, smart pointers is for sure what you should do. The reason is that you can't delete all resources in the destructor as you don't have all pointers available all the time.

If you want to use raw pointers, you should keep all pointer in the class all the time so that you can delete them when Database is destructed. This would require a redesign with a way of marking the individual resources as occupied/free.

That said - remember, using a pool is just the same as dynamic memory allocation.

If someone forgets to give the resource back to the pool, your program will crash sooner or later.

If someone forgets to delete dynamic allocated memory, your program will crash sooner or later.

Same story...

In other words - the pool is just as dangerous as the dynamic memory allocation.

Answer 3

Raw pointers can go astray in that they are not automatically deleted and there are many, many ways they can leak eventhough you think you handled all the cases.