[Development] QThread::finished() race → fixing QThread

[Olivier Goffart]

On Thursday 03 November 2011 11:37:25 Thiago Macieira wrote:
> Hello
> 
> First of all, required reading:
> http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/
> 
> == Background ==
> 
> Today, I was told that there was a timing sensitive test in
> QDBusPendingCall's unit testing. Looking at the code, it was something like
> this:
> 
> 	connect(&thread, SIGNAL(finished()), &loop, SLOT(quit()));
> 	thread.start();
> 	loop.exec();
> 	QVERIFY(!thread.isRunning());
> 
> Technically speaking, there's a race condition in *starting* the event loop
> above, but that's an error of the code above. Let's ignore it for the
> moment.
> 
> The problem we saw was QVERIFY would fail. If we look at the apparent causal
> sequence of events, we have:
> 
> 	event loop exited
> because
> 	finished() signal was emitted
> because
> 	thread's run() function exited
> 
> But isRunning() is still true. The reason for this is that there's a race
> condition in QThreadPrivate::finish, which emits the finished() signal
> before it sets the isRunning variable to false. The fix looks simple: move
> the code around.

Now that you mention it, this is a regression against Qt 4.7, 
(isRunning and isFinished used to lock)
hence the patch: http://codereview.qt-project.org/#change,8063 
(that should be applied to Qt 4.8 please)


That is not related to the order in which  finished() is emit.