diff options
author | Marc Mutz <marc.mutz@kdab.com> | 2013-09-21 15:25:57 +0200 |
---|---|---|
committer | The Qt Project <gerrit-noreply@qt-project.org> | 2013-09-23 02:47:49 +0200 |
commit | c0251f3041b9c7d6968e34dcdb8dd1b24a2c97c6 (patch) | |
tree | 4e3c3cb04029e06d03452f6d78b73de59d895de3 /tests | |
parent | 4f563cf09329c20f82d88ddfad7c0b3f3d0411f9 (diff) |
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>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/auto/corelib/thread/qmutex/tst_qmutex.cpp | 4 |
1 files changed, 1 insertions, 3 deletions
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); } |