Updating QThread docs (finally!)
-
[quote author="Andre" date="1348740754"]I think the simplest code example is something like this:[/quote]Thanks; updated, along with the explanation just below the code.
[quote author="Andre" date="1348740754"]And, yes, I think there should be something about the proper use of subclassing QThread in the detailed section of the QThread docs. It should explicitly warn against adding signals and slots there though.[/quote] Do you mean discourage making connections from within the subclassed QThread object? Or discourage adding new signals/slots to the QThread subclass? What's the reason?
The only warnings I can think of regarding subclassing, are:
- A QThread is not meant to "be" the actual code that runs in another thread
- A QThread should not be set as a parent of an object that's constructed inside its own implementation of run(), as they'll have different thread affinities
- To ensure that an event loop is created (thus ensure that event handling and signals/slots are enabled for objects in the thread), the reimplemented run() must call exec() at the end
Are these correct?
[quote author="Andre" date="1348740754"]
I'm not sure how relevant this is. I think this belongs in the setStackSize() method itself. It is not a main feature, and not used all that often I think.[/quote]Those lines were in the latest snapshot. I've removed them now. -
[quote author="JKSH" date="1348744139"]Do you mean discourage making connections from within the subclassed QThread object? Or discourage adding new signals/slots to the QThread subclass? What's the reason?
The only warnings I can think of regarding subclassing, are:
- A QThread is not meant to "be" the actual code that runs in another thread
- A QThread should not be set as a parent of an object that's constructed inside its own implementation of run(), as they'll have different thread affinities
- To ensure that an event loop is created (thus ensure that event handling and signals/slots are enabled for objects in the thread), the reimplemented run() must call exec() at the end
Are these correct?
[/quote]I was refering to discouraging adding new slots and signals to the QThread subclass. The problem is, that these almost never end up doing what the deverloper intends. Usually, these slots or signals are used to keep tabs on progress or some other state of the threaded process, or to give input to that threaded progress. However, with the slots and signals being on the QThread subclass itself, and the QThread subclass instance living in the main thread, we have a problem. Slots are invoked in the context of the main thread, and not in the context of the thread that is actually managed by the object. Signals may work though.
If you know what you are doing, then the above may be fine for your situation. But only if you understand the consequences. Judging by the posts on this forum and in other channels: people don't understand these consequences.
-
@Andre:
Very enlightening observations, thank you for sharing them. I'll work out how to weave it into the body of the text, and update this thread (no pun intended)@Beacon11:
Glad to hear that you worked it out :) Would you mind sharing what difficulties you faced, and what kind of information was most helpful to you? -
[quote author="JKSH" date="1348757707"]@Beacon11:
Glad to hear that you worked it out :) Would you mind sharing what difficulties you faced, and what kind of information was most helpful to you?[/quote]I followed the docs! I always subclassed QThread, and it always got messy. One day I decided there had to be a better way, and came across http://blog.qt.digia.com/2006/12/04/threading-without-the-headache/ . If that was just in the docs I would have never started down the subclassing road. Your doc update will be a simplified version of that blog post, which will be very helpful to those wanting to learn how to do it right.
-
Hi all, new text added:
- Big spiel on subclassing QThread
- Remind developers that basic multithreading precautions may still be needed
Any critique would be much appreciated!
@Beacon11: It is unfortunate that docs continued to advocate subclassing for years after the approach became unnecessary. Well, that's the purpose for this project: To help developers get it right the first time. If there's anything else you feel should be in the docs, please let me know!
-
I've just added subheadings and re-worded bits and pieces for improved clarity. I think that covers all the common and important use cases, but does anyone have any more suggestions? (be it for content, structure, or expressions)
In the meantime, I'll start working on the member function documentation.
Questions
- Does a QThread subclass need the Q_OBJECT macro, to start an event loop?
- For QThread::sleep(), the docs says "Forces the current thread to sleep for secs seconds." Is this correct? I'm wondering if it forces the thread being managed to sleep, not the current thread (which can be the thread that the QThread object lives in, depending on where sleep() is called)
-
Many thanks for your efforts!
[quote author="JKSH" date="1348894702"]I've just added subheadings and re-worded bits and pieces for improved clarity. I think that covers all the common and important use cases, but does anyone have any more suggestions? (be it for content, structure, or expressions)
In the meantime, I'll start working on the member function documentation.
Questions
- Does a QThread subclass need the Q_OBJECT macro, to start an event loop?
[/quote]
No. Q_OBJECT has the usual purposes (adding signals, slots (beware!), tr(), qobject_cast, etc.).
[quote]
- For QThread::sleep(), the docs says "Forces the current thread to sleep for secs seconds." Is this correct? I'm wondering if it forces the thread being managed to sleep, not the current thread (which can be the thread that the QThread object lives in, depending on where sleep() is called)[/quote]
It's correct -- it's a static member. You don't call it on a object (and it would make no sense to call aQThreadObject->sleep(10) and have an arbitrary thread blocked for 10 seconds); the thread that invokes that method is the one who sleeps.
Note that in 4.x those functions were protected, now they're finally public.
- Does a QThread subclass need the Q_OBJECT macro, to start an event loop?
-
@peppe: You're welcome, and thanks for your clarifications too. I can't believe I didn't notice the 'static' modifier!
-
Guys, is it necessary to include the following qmake statement:
@
CONFIG += thread
@or
@
CONFIG *= thread
@... when creating other threads?
The "thread":http://qt-project.org/doc/qt-4.8/qmake-variable-reference.html#config CONFIG variable's member does the following as stated by the qmake variable reference:
bq. The target is a multi-threaded application or library. The proper defines and compiler flags will automatically be added to the project.
As far as I can see by:
@
message($$CONFIG)
@... on Windows, the printed output contains:
@
thread_off
@When I add:
@
CONFIG += thread
@... the CONFIG variable contains both "thread_off" and "thread".
Is it necessary to add this statement and if so, is the "thread_off" CONFIG member a standard one that, for instance, explicitly removes the helper utilities added by "thread"?
Is the following code correct:
@
CONFIG -= thread_off
CONFIG *= thread
@ -
Minor edit: Followed a suggestion by Quentin on the Development mailing list, and switched to Qt5-style signal-slot connections, to promote their use.
[quote author="napajejenunedk0" date="1348917475"]Guys, is it necessary to include the following qmake statement.... when creating other threads?[/quote] Hmm... I've never used that flag before, but my multithreaded Qt programs worked fine. If I'm not mistaken, multithreading support is already enabled by default since Qt 4.0.
By the way, you might get more responses if you start a new forum topic, with descriptive title that matches your question. It's also considered good etiquette to keep forum replies relevant to the original post; otherwise it makes it harder for others to find relevant information.
-
Yes, I thought about posting a new forum thread before posting a response in this one. I will.
-
Hi all, some more updates made, based on suggestions from the mailing list and this forum. Comments and suggestions would be appreciated, as usual.
Changes:
- Added fact: Connecting finished() to deleteLater() became possible in Qt 4.8
- Added fact: sleep functions became public in Qt 5
- Added: Member function documentation (below, due to length constraints. I think the drafts for other pages shall be going on the wiki :P)
Questions:
- QThread::event() currently has a blank doc entry; what do developers need to know about it? From the source code, it provides handling of quit events, on top of QObject::event(). Does this need to be published?
=========================================================
DRAFT STARTS
Only modified member functions are shown: -Strikethroughs- = Deletions; Italics = Additionsh3. Member Function Documentation
h2. QThread::QThread(QObject * parent = 0)
Constructs a new -thread with the given parent.- QThread to manage a new thread. The parent takes ownership of the QThread. The thread does not begin executing until start() is called.
See also start().h2. QThread::~QThread()
Destroys the -thread- QThread.
Note that deleting a QThread object will not stop the execution of the thread it -represents- manages. Deleting a running QThread (i.e. isFinished() returns false) will probably result in a program crash. -You can wait() on a thread to make sure that it has finished.- Wait for the finished() signal before deleting the QThread.
h2. QThread * QThread::currentThread() [static]
Returns a pointer to a QThread which -represents- manages the currently executing thread.
h2. void QThread::run() [virtual protected]
The starting point for the thread. After calling start(), the newly created thread calls this function. The default implementation simply calls exec().
You can reimplement -ed- this function to -do other useful work- facilitate advanced thread management. Returning from this method will end the execution of the thread.
See also start() and wait().
h2. void QThread::start(Priority priority = InheritPriority) [slot]
Begins execution of the thread by calling run() -, which should be reimplemented in a QThread subclass to contain your code- . The operating system will schedule the thread according to the priority parameter. If the thread is already running, this function does nothing.
The effect of the priority parameter is dependent on the operating system's scheduling policy. In particular, the priority will be ignored on systems that do not support thread priorities (such as on Linux, see http://linux.die.net/man/2/sched_setscheduler for more details).
See also run() and terminate().
=========================================================
DRAFT ENDSTODO:
~ Add version info for functions introduced in Qt 5
~ Add "\override" to QThread::event()
~ Give sleep(), msleep() and usleep() the same wording -
Random thoughts:
- I wouldn't use the word "thread manager". I do understand what you try to stress, but a thread manager sounds more like an object managing more than one thread.
- For event, just put \override without any further explaination.
- "Every thread has its own stack." is a quite obvious statement to me... people failing to grasp thread basics probably shouldn't be playing with QThread at all
- Instead of stressing that various sleep methods are public in 5.0, I would stress that their usage is very likely a symptom of bad programming :-P cf. http://qt-project.org/wiki/Threads_Events_QObjects#92dd35cc61ffc41d02defdcef071856d
-
[quote author="peppe" date="1349024940"]
- I wouldn't use the word "thread manager". I do understand what you try to stress, but a thread manager sounds more like an object managing more than one thread.[/quote]Good point. So "thread" implies that QThread IS-A thread, but "thread manager" implies a one-to-many relationship. What's a good alternative? I can think of:
"[The constructor] constructs a new..."
- "... thread controller."
- "... thread control interface."
- "... object that manages a thread."
- "... QThread."
- "... QThread that manages a new thread."
- -"... QThread, along with the thread that it will manage."- Edit: Incorrect
(Side note: Maybe the name of the class itself should be changed for the next version of Qt. But then again, people don't seem to struggle much when using QDir, which is-not-a dir)
[quote author="peppe" date="1349024940"]
- For event, just put \override without any further explaination.
- "Every thread has its own stack." is a quite obvious statement to me... people failing to grasp thread basics probably shouldn't be playing with QThread at all[/quote]Done, and removed.
[quote author="peppe" date="1349024940"]
- Instead of stressing that various sleep methods are public in 5.0, I would stress that their usage is very likely a symptom of bad programming :-P cf. http://qt-project.org/wiki/Threads_Events_QObjects#92dd35cc61ffc41d02defdcef071856d[/quote]
Good point. I've stressed that forced sleeps should be unnecessary given that Qt is an event-driven framework, and urged readers to consider QTimer instead for their needs. (Are there any other common reasons people force a thread to sleep? I've never actually used these functions myself)
Continuing the train of thought, would you say that wait()-ing is sub-optimal, too? Personally, I listen for the finished() signal, and have never used wait().
-
Updated, to encourage developers to listen for finished() instead of using wait(). Also edited the constructor/destructor descriptions.
I'm planning to submit this to Gerrit in 48 hours, if there are no more comments from here or the mailing list. Thanks for all your input so far!
-
Gerritted: (Qt5) https://codereview.qt-project.org/36301
Edit: (Qt4) https://codereview.qt-project.org/36566 -
-
I did; the announcement came a few hours after my original patch set. I didn't pay much attention to it at first as I thought it was for the qtdoc repo only (QThread is in qtbase). I've asked Eskil for clarification though.
-
Thnaks a lot for your effort, mate.
