tst_QMutex: fix a race

The code uses a QSignalSpy to check whether the thread started,
but the signal emission (and subsequent appending to the spy) and
the check for spy.count() before the final thr.wait() are not
synchronized:

The signal emission happens-after the thr.start() and -before the
final thr.wait(). Likewise, the spy.count() happens-after thr.start()
and -before thr.wait(), but neither one happens-before the other.
Thus, there is a data race.

The wait(200) between thr.start() and mutex.unlock() doesn't help,
either, because we check only that it doesn't return true, iow, we
check that it timed out. But it will happily do that if the thread
has not yet started executing, so there's no happens-before relation
to be had via that avenue, either.

I first fixed by moving the spy.count() check to after thr.wait().
In that case:
  signal emission happens-before thread finishing
  happens-before thr.wait() returning
  happens-before spy.count()
so no race.

Arguably, that makes the check rather useless, so I decided to remove
it completely.

Change-Id: I6bb47c4114961ee6e9251cfebeb4b7794ba674a9
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2013-09-21 15:25:57 +02:00 committed by The Qt Project
parent 4f563cf093
commit c0251f3041

View File

@ -610,14 +610,13 @@ void tst_QMutex::tryLockNegative()
QMutex mutex;
TrylockThread thr(mutex, timeout);
QSignalSpy spy(&thr, SIGNAL(started()));
mutex.lock();
thr.start();
// the thread should have stopped in tryLock(), waiting for us to unlock
// the mutex. The following test can be falsely positive due to timing:
// tryLock may still fail but hasn't failed yet. But it certainly cannot be
// a false negative: if wait() returns, tryLock failed.
// a false negative: if wait() returns true, tryLock failed.
QVERIFY(!thr.wait(200));
// after we unlock the mutex, the thread should succeed in locking, then
@ -625,7 +624,6 @@ void tst_QMutex::tryLockNegative()
// ~QThread waiting forever on a thread that won't exit.
mutex.unlock();
QCOMPARE(spy.count(), 1);
QVERIFY(thr.wait());
QCOMPARE(thr.tryLockResult, 1);
}