From c0251f3041b9c7d6968e34dcdb8dd1b24a2c97c6 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 21 Sep 2013 15:25:57 +0200 Subject: 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 --- tests/auto/corelib/thread/qmutex/tst_qmutex.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp index 55b4e0b797..b320a438fa 100644 --- a/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp +++ b/tests/auto/corelib/thread/qmutex/tst_qmutex.cpp @@ -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); } -- cgit v1.2.3