I am writing a multithread project with Qt Framework. I have recently started using dynamic analysis tools as helgrind and thread sanitizer to check thread safety.
Throughout the entire application I have been writing a Qt Type called QSettings. The nominal functionality of this class is to store, in a file or registry, application settings such as windows size, company name and other types of settings (gains of a controller, timer timeouts, ...).
From the docs we read:
Accessing Settings from Multiple Threads or Processes Simultaneously
QSettings is reentrant. This means that you can use distinct QSettings object in different threads simultaneously. This guarantee stands even when the QSettings objects refer to the same files on disk (or to the same entries in the system registry). If a setting is modified through one QSettings object, the change will immediately be visible in any other QSettings objects that operate on the same location and that live in the same process.
Also from the docs we know that only one function is thread-safe.
At last, I want to keep in mind the following part:
void QSettings::sync()
Writes any unsaved changes to permanent storage, and reloads any settings that have been changed in the meantime by another application. This function is called automatically from QSettings's destructor and by the event loop at regular intervals, so you normally don't need to call it yourself.
And:
void QSettings::setAtomicSyncRequired(bool enable)
Configures whether QSettings is required to perform atomic saving and reloading (synchronization) of the settings. If the enable argument is true (the default), sync() will only perform synchronization operations that are atomic. If this is not possible, sync() will fail and status() will be an error condition. Setting this property to false will allow QSettings to write directly to the configuration file and ignore any errors trying to lock it against other processes trying to write at the same time. Because of the potential for corruption, this option should be used with care, but is required in certain conditions, like a QSettings::IniFormat configuration file that exists in an otherwise non-writeable directory or NTFS Alternate Data Streams.
So in principle QSettings is reentrant and cannot be used safely in a multithread applications but if we use setAtomicSyncRrequired to True, it can be done.
QSettings can be instantiated from different threads and be used in a isolated way so each thread has a private copy of the settings. Every certain time sync() is called and all the threads gets the same status settings in a safely way IF AND ONLY IF setAtomicSyncRequired is set to True. If setAtomicSyncRequired is set to true then the writing on the file is done atomically. So that is for writting in a good way, isn't it? If we use set(value) then we modified the local copy of Qsettings but since sync() writes to the common file the threads will end up with the same setting status.
When I started using helgrind and thread sanitizer they promp me data races problems due to the write of the same file by different threads. The file being simoultaneously accessed is the one which store the configuration set by QSettings.
Question: Is this a false positive?
Question: Is something wrong with my use of QSettings? How can it be improve in order to get a way to store common settings for all the threads in my application?
Question: Would it be a good idea to have only one pointer to the same QSettings object and set to true setAtomicSyncRequired in order to get all the thread settings synchronize every little time by sync()?
I understand the sync to be between multiple processes, not multiple threads. On Linux the settings are stored in a single file so you need to write to it atomically. QSaveFile does this for you. On Windows writing to a registry key is itself already thread-safe but writing a large set of keys likely requires further locking.
Looking at the source code it seems reasonably clear to me that Qt always tries to write atomically. It's just that depending on the file system, this can fail if lock files do not work. Then writing the settings would continue without atomicity if setAtomicSyncRequired is false or it will fail writing the settings if not. See around line 1373 in the linked file:
void QConfFileSettingsPrivate::syncConfFile(QConfFile *confFile)
{
…
/*
Use a lockfile in order to protect us against other QSettings instances
trying to write the same settings at the same time.
We only need to lock if we are actually writing as only concurrent writes are a problem.
Concurrent read and write are not a problem because the writing operation is atomic.
*/
QLockFile lockFile(lockFileName);
if (!readOnly && !lockFile.lock() && atomicSyncOnly) {
setStatus(QSettings::AccessError);
return;
}
…
}
QSettings themselves are protected by a mutex per ini file and one global mutex. I think it is safe to assume that all operations are thread-safe. This is in line with what I would expect from Qt. sync is only needed to publish data to other processes, not other threads within the same process.
When I started using helgrind and thread sanitizer they promp me data races problems due to the write of the same file by different threads. The file being simoultaneously accessed is the one which store the configuration set by QSettings.
Question: Is this a false positive?
Yes, I would assume that this is false-positive. The sanitizer likely doesn't understand the relation between the lock file and the config file.
Question: Is something wrong with my use of Qsettings? How can it be improve in order to get a way to store common settings for all the threads in my application?
I don't think there is anything wrong. However, be cautious about the atomicity. Loading and saving a single setting is atomic but accessing a set of settings is not. For example if one thread changes two settings while another thread reads those two, the reading thread could get one old and one new value.
Question: Would it be a good idea to have only one pointer to the same QSetting object and set to true setAtomicSyncRequired in order to get all the thread settings synchronize every little time by sync()?
No, I don't think this would be a good idea, if anything, it is worse. Operations such as QSettings::beginGroup, endGroup, beginArray and endArray would not work properly unless you add some external locking.
The design is intended so that the QSettings objects themselves are local to a thread or parent object and potentially short-lived. Your use is fine.
As for the sync, note that isAtomicSyncRequired is default true and sync() is automatically called by the QSettings destructor.