mathripper
mathripper

Reputation: 451

Best approach for making a thread safe singleton class derived from QObject

In my Qt program, I have a class called MySettings which stores the application's settings (things like UI parameters). The class is a wrapper around QSettings and it looks like this:

class MySettings : public QObject {

    Q_PROPERTY(QString myProp1 READ myProp1 WRITE setMyProp1 NOTIFY myProp1Changed)
    Q_OBJECT
public:
    MySettings() : m_settings(new QSettings) {}
    
    // MySettings is a singleton
    static MySettings *getInstance()
    {
        if(!instance)
            instance = new MySettings;
        return instance;
    }

    
    QString myProp1 () const { return m_myProp1; }
    void setMyProp1(const QString &prop1) { m_myProp1 = prop1; m_settings->setValue("myProp1", prop1); }
    
    // other properties are of type float, int, bool or QString
    
signals:
    void myProp1Changed();
    
private:
    static MySettings* instance;
    QSettings *m_settings;
    QString m_myProp1;
    // other properties...
}

I'm using this class as a singleton by calling MySettings::getInstance() and then accessing the properties I need. Now, I would like to use this class from different threads. My first thought was that in order to make it thread-safe I could wrap the private member variables using std::atomic, but this would only work for numbers and boolean values and QString is not trivially copyable. Also the QSettings class is reentrant but not thread-safe, so I would need to make sure that I created different instance of QSettings instead of using a single instance as I'm doing above. The other approach is to use mutexes inside each member function, but I would prefer to avoid that. Is there a better solution to this? Is there a programming pattern I can follow to create a thread-safe singleton of MySettings class?

Upvotes: 0

Views: 403

Answers (1)

hyde
hyde

Reputation: 62777

Do not create your own class for this, just use QSettings. Here's how:

In main function, have this:

QCoreApplication::setOrganizationName("mathripper");
QCoreApplication::setApplicationName("fancypantsapp");

This will determine where the settings file goes and with what name, and unless you have a specific reason to do it differently, I'd do it that way.

Then anywhere you need your settings, you just do

QSettings settings;

And that's it. Normally there is no need to mess with new, pointers and setting parent, just create a local QSettings object and use it. You can do that as a function local parameter, or you can put that as a member variable of any class if you want that created only once (and the object with that is going to be used only from 1 thread).


Alternatively, if you want a different settings file or scope than the default, you could create a helper function somehwere, something like:

QSettings settingsInstance() {
    return QSettings(QCoreApplication::applicationDirPath() + "/settings.ini", 
                     QSettings::IniFormat);
}

(Note that returning QSettings by value like this works here because of guaranteed copy elision in C++, even though QObject it is not copyable.)

And then when ever you need an instance, you just do

auto settings = settingsInstance();

If you have performance concerns about having too many QSettings instances, the next step would be to create a thread local instance for any thread that needs these. But that's a different question, "How can I have thread local QSettings instances?".


If you really want to do this yourself, instead of using QSettings directly, put the settings in a class that is not QObject subclass, and make it fully thread safe. If you need QObject, do like it is done with QSettings: every thread needs its own instance, even if they access the same data (using that thread-safe non-QObject class mentioned above).

Upvotes: 1

Related Questions