Trying to resolve sub-classing QThread - or not
-
I have done a search regarding the issue of sub-classing QThread or not. There are what looks almost like a theological debate as to the merits of one versus the other. What I am not being able to find is what are the pitfalls of using an object as a sub-class of QThread. I see benefits of sub-classing such as the availability of isRunning, isFinished, sleep, msleep etc. Without using a subclass, those functions are not available. So other than the statement that it essentially is "no longer necessary", is there a more functional reason?
-
There are of course still valid reasons for subclassing QThread, but for most use cases the worker object approach should be prefered. Most of the problems related to multithreading applications that are posted here can in the end be traced to mistakes related to the subclassing of QThread without having a full understanding of the implied mechanics and the resulting pitfalls (for example the fact that the QThread object is living in the context of the thread in which it was created, or the fact that the QThread object itself is not a thread but rather the manager for a thread).
-
OK, thanks. That's helpful. However documenting a changing of the way of using part of the Qt library because of mistakes in the way users are using it, seem odd. The fact that one can accomplish tasks both ways are a great testament to the flexibility of the Qt library.
-
The new approach was only made possible by the 4.4 release. Before that subclassing was the only option as far as I understood. Unfortunatly the documentation for QThread was not updated after the change to promote the new possibility.
If you haven't already stumbled upon it, this is the article that sort of first promoted the new approach: "You're doing it wrong":http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/BTW the documentation for the 5.0 release will finally be updated to promote the new approach. Have a look at the new docs.
-
@Andre if you call moveToThread(this) in the QThread subclasses constructor aren't the signals and slots handled in this thread then? I know its travesty but thats the solution you can always read in the blogs promoting subclassing. I think the main reason for the worker object approach is that it fits best to the design of the QThread class and it helps to simplify the use of it (better readability, scalability and so on).
-
[quote author="Andre" date="1354721304"]I think the approach has been possible for much longer.
The main issue with the subclassing approach is in case you are adding signals and slots on the subclassed class: they are not in the thread you expect them to be.[/quote]Thanks, that makes sense and I would imagine very soon would manifest the problem of misuse. On the other hand using the signal slot mechanism properly would allow the use of the sub-class of QThread would it not? I guess it's becoming one of consensus and what are the benefits of both approaches.
For example I submit the following article that uses a sub-class or QThread:
"here":http://blog.lugru.com/2010/05/qtthreads-are-not-scary/ -
a user in this forum (JKSH) is currently involved in updating the "documentation":http://doc-snapshot.qt-project.org/5.0/qtcore/qthread.html and other articles for QThread. Have a look at "this":http://qt-project.org/forums/viewthread/22205/, maybe it helps you understand more or you can get involved as well.
Also regarding your statement that you need to subclass to gain access to functions like msleep(), timeout() , and so forth I'd like to quote from "the new docs":http://doc-snapshot.qt-project.org/5.0/qtcore/qthread.html#managing-threads: [quote] Note: wait() and the sleep() functions should be unnecessary in general, since Qt is an event-driven framework. Instead of wait(), consider listening for the finished() signal. Instead of the sleep() functions, consider using QTimer. [/quote]
-
[quote author="KA51O" date="1354780208"]a user in this forum (JKSH) is currently involved in updating the "documentation":http://doc-snapshot.qt-project.org/5.0/qtcore/qthread.html and other articles for QThread. Have a look at "this":http://qt-project.org/forums/viewthread/22205/, maybe it helps you understand more or you can get involved as well.
Also regarding your statement that you need to subclass to gain access to functions like msleep(), timeout() , and so forth I'd like to quote from "the new docs":http://doc-snapshot.qt-project.org/5.0/qtcore/qthread.html#managing-threads: [quote] Note: wait() and the sleep() functions should be unnecessary in general, since Qt is an event-driven framework. Instead of wait(), consider listening for the finished() signal. Instead of the sleep() functions, consider using QTimer. [/quote][/quote]
Agreed, bad example. What about isRunning(), isFinished() ? Those were mentioned too. How do you access those without inheriting QThread?
-
[quote author="astodolski" date="1354798279"]
Agreed, bad example. What about isRunning(), isFinished() ? Those were mentioned too. How do you access those without inheriting QThread?[/quote]
Did you try the documentation? Yes, these are public member functions.
-
[quote author="astodolski" date="1354718950"]documenting a changing of the way of using part of the Qt library because of mistakes in the way users are using it, seem odd. [/quote]A designer must know their product users' psychology. If a design causes lots of confusion, then it is worth re-designing.
[quote]The fact that one can accomplish tasks both ways are a great testament to the flexibility of the Qt library.[/quote]That's a bit like saying "the fact that one can dynamically allocate structs using both new and malloc() is a great testament to the flexibility of C++." Both are possible, yes... but malloc is more error-prone and should be discouraged. Same with subclassing QThread.
[quote author="astodolski" date="1354798279"]What about isRunning(), isFinished() ? .... How do you access those without inheriting QThread?[/quote]Like Andre said, those are public, so anybody can access them. All QObjects know which thread they live in (Note: A QThread is NOT a thread, the same way a QFile is not a file); so you can do:
@
myObject->thread()->isRunning();
@If you create your own QThread and use worker objects, you can query the same thread through the worker object, or through the thread controller:
@
// Create worker, put worker in new thread
QThread *workerThread = new QThread();
WorkerObject *worker = new WorkerObject();
worker->moveToThread(workerThread);// Start thread and worker
connect(workerThread, SIGNAL(started()), worker, SLOT(doWork()));
workerThread->start();// Check status (two possible ways)
qDebug() << worker->thread()->isRunning();
qDebug() << workerThread->isRunning();
@[quote author="KA51O" date="1354721644"]I think the main reason for the worker object approach is that it fits best to the design of the QThread class and it helps to simplify the use of it (better readability, scalability and so on).[/quote]Yes. It also separates thread-control from thread-data-processing; all of these reasons make it harder for users to get mixed up. Thus, there's a benefit for this forum, and the mailing list, and IRC: Less people asking for help with multithreading woes!
-
@astodolski Good on you for taking the time explain QThreads in your blog, as well as for correcting it. Have you written a Part 2?
-
[quote author="JKSH" date="1354877251"]@astodolski Good on you for taking the time explain QThreads in your blog, as well as for correcting it. Have you written a Part 2?[/quote]
I didn't write a part 1. Was this comment intended for someone else?
-
[quote author="astodolski" date="1354882935"]I didn't write a part 1. Was this comment intended for someone else?[/quote]Ah, sorry, I thought the LuGRU blog was yours
-
[quote author="JKSH" date="1354899256"][quote author="astodolski" date="1354882935"]I didn't write a part 1. Was this comment intended for someone else?[/quote]Ah, sorry, I thought the LuGRU blog was yours
[/quote]
No, just posted it for reference in defence of what is being referred as the "wrong way".
-
[quote author="astodolski" date="1354902693"]No, just posted it for reference in defence of what is being referred as the "wrong way".[/quote]That post does not defend the subclassing approach. The author changed his post when he learnt of the arguments against it.
IMHO, the only benefit of the subclassing approach is fewer lines of code, and one less class. This is fine when doing quick hacks to try things out, but I'm not convinced that it's good for long-term code.
If we continue advocating it, you'll never know when some new developer comes along and thinks, "I have an idea for a new feature. Let's implement that through some new slots in our CustomThread object!" It's then time for said developer (or colleagues) to pull hair while debugging mysterious errors.
There are other, safer alternatives to subclassing QThread. Look at QRunnable and Qt Concurrent.
-
I'm just trying to build a consensus. I am not in favor of one versus the other. I am just trying to understand where the overwhelming use of one class gets replaced by a different approach. There's only a small but growing representation of the preferred use of using threads.
-
I see. Have you made good progress in your research? You can find some advocacy for the subclassing approach in the comments here: https://codereview.qt-project.org/#change,41299
I think the replacement gained traction when people realized that certain misunderstandings about QThread were more common than normal, and that these misunderstandings can be avoided if we find different ways to use QThread. Also, I'd imagine that an expert's call for change is very influential (http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/ )
-
There seems still very valid reasons for subclassing QThread. This new method is not an "end-all" replacement. If the official 5.0 release has all it's code in examples and tutorials folder updated to reflect the changed philosophy of threads, it will be easier to get a better understanding.
-
Yes, the new "worker object" method is not ideal. Its main weakness is the extra complexity and overhead. I guess we'll need to ask ourselves which poison is more tolerable for the scenario at hand: A lengthy approach, or a counter-intuitive approach?
Unfortunately, the documentation update did not pass the review process in time for the Qt 5 release -- there were more pressing issues for the reviewers to take care of, and I think not all the higher-ups are interested in making the change. Let's see if they get through for the Qt 5.0.1 release, scheduled for late January 2013.
