[Interest] Annoying Bug - Qt 4.8.1 bug?
Konrad Rosenbaum
konrad at silmor.de
Fri Jan 24 18:54:35 CET 2014
Hi Etienne,
if you are impatient: scroll at least halfway down.
On Thursday, Thursday 23 January 2014 at 17:41, Etienne Sandré-Chardonnal
wrote:
> 2014/1/23 Konrad Rosenbaum <konrad at silmor.de>
> > If some of those connections are handled outside the main thread, it
> > could explain the crash: GUI events MUST NOT be executed outside the GUI
> > thread. It also is usually a bad idea to execute methods of the same
> > object in multiple threads (unless they are specifically protected by
> > locks).
> >
> >
> >
> > Why is run() empty? As far as I understand QThread this means the thread
> > lives for only a few microseconds and then self-destructs. I have not
> > completely understood what that does to the timer and progress dialog
> > yet.
>
> Please read previous posts! This is a dummy lightweight minimalist project
> that emphasizes on a bug. It does nothing and run() is empty because you
> will not read my whole 200k lines project and it does not change the crash
> occurence to strip it out. Yes it terminates after a few µs.
So we can exclude anything that lives inside the thread as a culprit - except
the thread itself.
> > I think I have a possible reason why the progress dialog may crash: the
> > finished() signal is emitted from within the new thread, since the
> > connection is direct (automatic means direct in this specific case) the
> > close() method is called inside the wrong thread.
>
> No. QThread QObject lives in the main GUI thread and emits signals from
> there. Never from the new thread. Connecting it to the main GUI thread
> slots is a very common way to use QThread.
Yes, I also use this technique. But I always explicitly use QueuedConnection -
just to make sure. I'm paranoid this way... ;-)
> By the way, if you emit finished() from run() , that's wrong. It will call
> direct connections from the wrong thread and signal that the thread is
> finished while this is not the case. That's bad.
Neither I nor you emit finished. The implementation of QThread inside Qt emits
finished. According to documentation it is emitted last thing from the thread
before it exits. To make sure, I experimented with it (adding a few qDebug
statements outputting (qint64)QThread::currentThread()) - I could confirm the
docu. The finished() signal is DEFINITELY emitted from the new thread!
I tried to read the Unix implementation of QThread in Qt 5 (sorry, I deleted
the Qt4 sources and I can't read Windows code very well):
QThread::start causes the underlying thread facility of the OS to create a new
thread and run a private method in it, this private (already running in the
new thread) method looks approximately like this (pseudo code):
//initialize
register_cleanup_to_run(mycleanup);
create_event_dispatcher();
emit started();
//do my thing
realthread->run();
//return control to OS
return 0;
The cleanup routine is then called by the OS (inside the still running thread)
before the thread exits and does:
//write my orbituary
emit finished();
//clean up
delete_event_dispatcher();
As you can see: both the started() and finished() signals are emitted from
inside the new thread, even though they belong to an object that officially
"lives" inside the main thread.
This is a common pattern: you can emit signals from whereever you want, even
if it is not the thread that the object is supposed to live in.
However, the good news is that the AutoConnections you create seem to be
QueuedConnections after all. I haven't dug deeper into QThread to find out
why, but it's a useful property of QThread...
> > Somewhere else in this mailthread: it does not matter whether the program
> > crashes on Linux, valgrind (helgrind, drd) detects race conditions no
> > matter whether they are triggering a problem right now by detecting
> > memory access that is not protected by locks/semaphores.
>
> The new threads runs NOTHING (ie run() is empty) so it does not live, does
> not emit anything, and does not access anything. This can't be the problem,
> which very likely is in the main thread event loop.
As written above: it does. Even if you didn't believe you told it to.
The simple truth is (at least on my Linux laptop): for some odd reason
QProgressDialog::setValue gets called after the progress dialog has already
been deleted. In your case it probably decides to self-destruct because it is
at 100% and at almost the same time gets the finished() signal causing a
double-free.
Normally Qt should disconnect everything and delete queued events for this
object when a QObject destructs. I guess that in this case an event sneaks in
because it comes from a different thread - I vaguely remember having big
problems with this a few years ago - I solved them with a lot of semaphores
and mutexes and occasional delays before deleting objects (not nice, but works
most of the time).
Hint: this is a good reason to migrate to Qt5 - this problem has been solved
there (at least as far as I can tell).
In the case of your program I could solve the problem this way:
1) I extended the run method to simulate an actual storage object: sleep(2);
-> sleep blocks the thread, there is no risk of doing anything on the event
loop
2) I added a boolean variable: bool _done;
and initialized it in the constructor: _done=false;
_done==true will simply mean it is no longer safe to signal any dependent
object in the main thread from the new worker thread
3) the last thing run() does is: _done=true;
i.e.: this thread will self-destruct in a few nano-seconds
4) any method emitting signals that may cross threads (e.g.
updateProgress(int)) now contains: if(_done)return;
as the first line
5) To make sure nothing bad happens, you should also add
progress->setAutoClose(false); to startWriting()
-> otherwise the progress dialog may self-destruct prematurely
The effect is that no signals (except finished()) can cross the threads after
the thread is done doing actual work. This way it is guaranteed that
finished() is the last signal to cross over.
Hint: If you have too many exit points from run() - try a helper class:
template<class T> class MySetter{
T&var; //don't worry, the compiler will optimize these
const T val; //two variables away
public:
MySetter(T&_var,T _val):var(_var),val(_val){} //remember
~MySetter(){var=val;} //do the setting at exit
};
void BackgroundFileWorker::run()
{
//schedule _done=false for later...
MySetter<bool> donesetter(_done,true);
//do complex work
sleep(2);
}
Konrad
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20140124/9cc33494/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20140124/9cc33494/attachment.sig>
More information about the Interest
mailing list