Please critique: solution to database connection and multiple threads

Hello,

As many of you are probably aware, Qt docs are quite clear about instances of QSqlDatabase and threads:

Quote:

---

A connection can only be used from within the thread that created it. Moving connections between threads or creating queries from a different thread is not supported.

---

After migrating from raw MySQL libraries to a QSql-based approach, my application started behaving oddly. Evidently the raw MySQL does not have the cited constraint: my application indeed uses exactly two threads: the main thread, and a thread that receives sensor data. Under the current implementation, both threads do work with the database.

The following illustrates the current solution to the problem, which takes a lazy-loading approach. Everything appears to work, but I would welcome any and all constructive criticism.

Code:

---

// Controller.h
 
private:
    CustomDatabase * databaseConnectionForThread(const Qt::HANDLE threadID = QThread::currentThreadId());
 
    QHash<Qt::HANDLE, CustomDatabase *> dbConnectionsByThread;
    QMutex threadMutex, sqlMutex;

---

Code:

---

// Controller.cpp
 
CustomDatabase * Controller::databaseConnectionForThread(const Qt::HANDLE threadID) {
    QMutexLocker locker(&threadMutex);
    static int numberForConnectionName = 0;
    CustomDatabase *dbConnection;
    if (dbConnectionsByThread.contains(threadID))
        dbConnection = dbConnectionsByThread.value(threadID);
    else {
        dbConnection = CustomDatabase::create(QString::number(++numberForConnectionName), this); // create() initializes and opens the connection it returns
        if (!dbConnection->isOpen())
            exitWithError(QString("Controller::databaseConnectionForThread() - Failed: %1!").arg(dbConnection->lastError()));
        dbConnectionsByThread.insert(threadID, dbConnection);
    }
    return dbConnection;
}
 
void Controller::doStuffWithDatabase() {
    QMutexLocker locker(&sqlMutex);
    CustomDatabase *dbConnection = databaseConnectionForThread();
    // do stuff
}

---

Thank you very much for your time.

That looks ok, maybe use the pointer of QThread::currentThread() instead though.
The docs for Qt::HANDLE say it can't be used for numeric comparison, which I think QHash does use.

I am not sure you need the sqlMutex, you have per thread connections now anyway.

An alternative approach would be to use different connectionNames for QSqlDatabase::addDatabase(), e.g. the pointer of the thread printed into a string.

Cheers,
_

Quote:

---

Originally Posted by anda_skoa

That looks ok, maybe use the pointer of QThread::currentThread() instead though.
The docs for Qt::HANDLE say it can't be used for numeric comparison, which I think QHash does use.

---

Ah, good catch, thank you!

Generally speaking, is this a more or less standard way to address the original problem? I mean, it works fine but for the sake of argument, how else might one get around it? Are there certain situations in which certain approaches are best?

Quote:

---

how else might one get around it? Are there certain situations in which certain approaches are best?

---

I'm going to partially answer my own questions, for posterity.

It just so happens that code executed in the sensor thread (which is not created by the application) uses an instance of a custom class that has-a QTimer. This resulted in the following warning:

Quote:

---

QObject::startTimer: Timers can only be used with threads started with QThread

---

To get around this, I wrap the sensor input and pass it to the main thread instead of the lazy-load-and-cache database connections per thread approach:

Code:

---

void Controller::onNotification(const Notification *notification, void *caller) {
        Controller *ref = static_cast<Controller *>(caller);
        NotificationWrapper wrapper(notification);
        QMetaObject::invokeMethod(ref, "doStuff", Qt::AutoConnection, Q_ARG(NotificationWrapper, wrapper));
}

---

Note that there is some legwork involved in calling QMetaObject::invokeMethod() with a custom type. See http://doc.qt.io/qt-5/qmetaobject.html#invokeMethod.

Since all the work is now on the main thread, only a single database connection is required. In addition, calls to QMutexLocker that were peppered throughout the code have been removed.

This is coming a bit late, especially since you have moved away from your one-connection-per-thread solution, but if you ever have to do something like that again, consider using thread-local storage (see QThreadStorage for Qt's implementation) instead of rolling out your own map protected by a mutex. Thread-local storage is supported by the C11 and C++11 standards, and is usually natively supported by the OS. IIRC a typical implementation consists in reserving a CPU register that points to a different memory area for each thread.