Share QString between threads
-
Hello.
Can anybody explain me how to share QString between threads? Under "share" I mean that one thread is writing to QString and another thread is reading from it. I know, that QString uses implicit sharing, so it's reentrant only, but not thread-safe. I guess that I have to use some locking when reading from or writing to a QString. Here is example of my code:
// Task.h class Task : public QObject { Q_OBJECT public: QString errorText() const { QReadLocker locker(&m_lock); return m_errorText; } void setErrorText(const QString &errorText) { QWriteLocker locker(&m_lock); if (errorText != m_errorText) { m_errorText = errorText; locker.unlock(); emit errorTextChanged(); } } signals: void errorTextChanged(); private: QString m_errorText; QReadWriteLock m_lock; }; // MainWindow.h class MainWindow : public QMainWindow { Q_OBJECT public: void executeTask(int taskIndex) { auto worker = new Worker(m_tasks[taskIndex]); QObject::connect(worker, &Worker::finished, worker, &QObject::deleteLater); worker->start(); } private: QList<QSharedPtr<Task>> m_tasks; }; // Worker.h class Worker : public QThread { Q_OBJECT public: explicit Worker(QSharedPtr<Task> task, QObject *parent = Q_NULLPTR) : QObject(parent) , m_task(task) {} protected: void run() override { auto exitStatus = 0; // some code if (1 == exitStatus) m_task->setErrorText("Failed to access input file"); // some oode } private: QSharedPtr<Task> m_task; };
My question is: do I really have to use locking when using QString in different threads and is the above example code correct?
Thanks.
-
@Max-Logvinov said in Share QString between threads:
do I really have to use locking when using QString in different threads
Yes, not just for QString, it's a general rule to avoid race conditions
@Max-Logvinov said in Share QString between threads:
is the above example code correct
QWriteLocker locker(&m_lock); if (errorText != m_errorText) { m_errorText = errorText; locker.unlock(); emit errorTextChanged(); }
is a bit redundant
if (errorText != m_errorText) { m_lock.lockForWrite(); m_errorText = errorText; lm_lock.unlock(); emit errorTextChanged(); }
works already.
For the rest I cannot see anything too obviously wrong
-
@VRonin, thank you for your answer.
Yes, not just for QString
The C++11 Concurrency Library provides std::atomic, which I can use with any TriviallyCopyable type. But not with QString.
is a bit redundant
You right. In the example above, there is no need to lock before comparing strings. But let me clarify one thing: if there will be more than one thread, that can simultaneously write to the string, then my example above will not be redundant, isn't it?
-
@Max-Logvinov said in Share QString between threads:
The C++11 Concurrency Library provides std::atomic, which I can use with any TriviallyCopyable type. But not with QString.
That's because QString is not trivially copyable, as are a lot of classes, see http://en.cppreference.com/w/cpp/types/is_trivially_copyable
@Max-Logvinov said in Share QString between threads:
hat can simultaneously write to the string, then my example above will not be redundant, isn't it?
You are correct, I missed it.
-
@VRonin, once again, thank you for your clear answer.
-
I have two remarks that aren't explicitly linked to the question.
- Are you convinced you need to use a new thread for each task and then delete it. This isn't the best of ideas as on some OSes (like my lovely Linux) threads are pretty heavy to create/destroy. Perhaps you should consider moving to the worker object idiom or to using a thread pool.
- If
QSharedPtr
is what I think, I strongly advise you not to shoot yourself in the foot by delegating responsibility for memory management to a black box.QObject
doesn't work that well with shared pointers, as it has a special ownership (and organizational) model.
Here are some thoughts and a little bit more from the mailing list. If you need to track lifetime ofQObject
s when you don't own them useQPointer
, if you own them and want to be throw-safe useQScopedPointer
(or its STL equivalent).
-
@kshegunov, thank you for your notes.
- Of course, I'm using a thread pool and the example above is just for understanding content of my question.
- Yes, it's a typo. I mean a QSharedPointer. But I really don't see any reasons to don't use QSharedPointer with a QObject if you do it wisely. Here is a part of my real code:
class Task : public QObject { Q_OBJECT friend class TaskManager; private: Task(const QString &inputFile); ~Task() {} public: void setParent(QObject *parent) = delete; }; typedef QSharedPointer<Task> TaskPtr; class TaskManager : public QObject { Q_OBJECT public: qint64 addTask(const QString &inputFile) { static auto deleteTask = [] (Task *task) { delete task; }; _tasks << TaskPtr(new Task(inputFile), deleteTask); emit taskAdded(); } signals: void taskAdded(); private: QList<TaskPtr> m_tasks; };
As you can see, you can't set the parent for the Task. Also, instances of the Task is created/deleted only by the TaskManager. This looks good to me.
-
@Max-Logvinov said in Share QString between threads:
As you can see, you can't set the parent for the Task.
Are you sure about this? Suppose the following code:
QObject * coolParent; QObject * task = new Task(inputFile); task->setParent(coolParent);
Also, instances of the Task is created/deleted only by the TaskManager.
They are created by the
TaskManager
but there's no guarantee about deletion, or in which thread it happens. It all depends on who (which object) loses the last reference to the task and in which thread context this is happening.This looks good to me.
Just read the mailing list threads. It's up to you in the end how to manage the memory, but you've been warned.
-
@kshegunov, thanks for your notes, I'll keep in mind.