Is this code thread-safe with C++

Question

I have read articles about double-check which may not be safe with C++ on arbitrary CPU. I am not quite sure whether my codes are absolutely safe or not. Class Database represents database file, and the connection is dynamically created when needed. Database::GetConnection may be called from different threads simultaneously. If the code is not absolutely safe, how can I modify the code. Thank you.

#include <mutex>
#include "sqlite3.h"

class Connection
{
private:
    sqlite3* _handle;
    std::string _path;
public:
    Connection(const std::string& path) : _path(path) {
        sqlite3_open(_path.c_str(), &_handle);
        // some other initialization work..
    }
}

class Database
{
private:
    typedef enum
    {
        Void,
        Connected,
        // some other status...
    } Status;

    std::string _path;
    std::mutex _mutex;
    Status _status;
    Connection* _connection;

    void OpenDatabase()
    {
        _connection = new Connection(_path);
        _status = Connected;
    }
public:
    Connection* GetConnection()
    {
        if (_status == Connected)
            return _connection;

        std::lock_guard<std::mutex> guard(_mutex);
        if (_status != Connected)
            OpenDatabase();
        return _connection;
    }
public:
    Database(const std::string& path) : _path(path), _status(Void) {};
};

Insufficient information for an authoritative answer, but looks like this is not thread safe. It's obvious to me both that `_connection` and `_status` should only be accessed while a lock is being held on the protective mutex. However this is obviously not the case. — Sam Varshavchik, Nov 23 '16 at 03:04
You're not guarding `_status`, why would you think it's safe? — Tas, Nov 23 '16 at 03:04
I would have though OpenDatabse to return the connection instead of it operating on the class variable. Maybe what you need is a singleton? - http://stackoverflow.com/a/1008289/429476 — Alex Punnen, Nov 23 '16 at 03:15
@Tas, `_status` is modified only in `OpenDatabase`, and `OpenDatabase` is guarded by _mutex. I think that's thread-safe. But I am not sure if optimization or memory order will affect the result. — wqyfavor, Nov 23 '16 at 03:44
@wqyfavor I can very clearly see you accessing `_status` in `GetConnection`. Accessing the data requires it to be locked. In your question you said _If the code is not absolutely safe_ - I assure you: the code is not absolutely safe, despite what you may think. — Tas, Nov 23 '16 at 03:49
@Tas can you provide an example of how things might go wrong given the code as shown? I think that would be helpful as a means of demonstrating how you came to the conclusion that you did. — Jeremy Friesner, Nov 23 '16 at 03:54
For efficiency, the read access of `if (_status == Connected)` in `GetConnection` is not locked. If I lock read of `_status`, the question is meaningless. So I only wonder if what I do is **safe enough**. — wqyfavor, Nov 23 '16 at 03:59

score 1 · Answer 1 · answered Nov 23 '16 at 05:58

For efficiency ... is safe enough

You're dealing with a database, efficiency is moot and safe enough is up to you

Enough implies a level of allowable uncertainty. Are you ok with that?

What happens in that 0.001% chance it's not safe enough? Does the software crash? Does the hardware explode? Do birds turn inside out and fall from the sky?

If your OpenDatabase function is locked everywhere else in your Database class, then what's wrong with removing the first check and leaving your GetConnection function as so:

Connection* GetConnection()
{
    std::lock_guard<std::mutex> guard(_mutex);
    if (_status != Connected)
        OpenDatabase();
    return _connection;
}

You're dealing with a database, so the few milliseconds it takes to your CPU to create and store the std::lock_guard and lock/unlock the _mutex object then preform a simple boolean check is massively insignificant compared to the 100's of milliseconds building a connection and transactional state for the database can take.

Save the birds.

Is this code thread-safe with C++

1 Answers1