Skip to content

Commit 1ce3fd0

Browse files
mikeashgottesmm
authored andcommitted
[Tests] Make Mutex.cpp tests more reliable.
Many of these tests would fail if one of the test threads didn't begin execution within 100 milliseconds. They would hit while (!done) the very first time and never execute the loop body. That does happen from time to time, and the result would be a spurious failure. Change the loops to do {} while(!done) to ensure they always execute at least one iteration. Many tests also had the test threads concurrently write results to a std::vector, which appeared to be causing some failures in my local testing when I had extra sleeps inserted to simulate pathological scheduling. Change the results vectors to be vectors of std::atomic to ensure that this concurrent writing is safe. These changes shouldn't affect its ability to test the functionality it intends to test. rdar://problem/49386389 (cherry picked from commit 6172f72)
1 parent 57f31fd commit 1ce3fd0

File tree

1 file changed

+42
-31
lines changed

1 file changed

+42
-31
lines changed

unittests/runtime/Mutex.cpp

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -555,20 +555,21 @@ TEST(StaticReadWriteLockTest, ScopedWriteUnlockThreaded) {
555555
template <typename RW> void readLockWhileReadLockedThreaded(RW &lock) {
556556
lock.readLock();
557557

558-
std::vector<bool> results;
559-
results.assign(10, false);
558+
std::vector<std::atomic<bool>> results(10);
560559

561560
std::atomic<bool> done(false);
562561
threadedExecute(10,
563562
[&](int index) {
564-
while (!done) {
563+
// Always perform at least one iteration of this loop to
564+
// avoid spurious failures if this thread is slow to run.
565+
do {
565566
lock.withReadLock([&] {
566567
results[index] = true;
567568
std::this_thread::sleep_for(
568569
std::chrono::milliseconds(5));
569570
});
570571
std::this_thread::sleep_for(std::chrono::milliseconds(1));
571-
}
572+
} while (!done);
572573
},
573574
[&] {
574575
std::this_thread::sleep_for(std::chrono::milliseconds(100));
@@ -577,7 +578,7 @@ template <typename RW> void readLockWhileReadLockedThreaded(RW &lock) {
577578

578579
lock.readUnlock();
579580

580-
for (auto result : results) {
581+
for (auto &result : results) {
581582
ASSERT_TRUE(result);
582583
}
583584
}
@@ -595,28 +596,29 @@ TEST(StaticReadWriteLockTest, ReadLockWhileReadLockedThreaded) {
595596
template <typename RW> void readLockWhileWriteLockedThreaded(RW &lock) {
596597
lock.writeLock();
597598

598-
std::vector<int> results;
599-
results.assign(10, 0);
599+
std::vector<std::atomic<int>> results(10);
600600

601601
std::atomic<bool> done(false);
602602
threadedExecute(10,
603603
[&](int index) {
604-
while (!done) {
604+
// Always perform at least one iteration of this loop to
605+
// avoid spurious failures if this thread is slow to run.
606+
do {
605607
lock.withReadLock([&] {
606608
results[index] += 1;
607609
std::this_thread::sleep_for(
608610
std::chrono::milliseconds(5));
609611
});
610612
std::this_thread::sleep_for(std::chrono::milliseconds(1));
611-
}
613+
} while (!done);
612614
},
613615
[&] {
614616
std::this_thread::sleep_for(std::chrono::milliseconds(100));
615617
done = true;
616618
lock.writeUnlock();
617619
});
618620

619-
for (auto result : results) {
621+
for (auto &result : results) {
620622
ASSERT_EQ(result, 1);
621623
}
622624
}
@@ -636,28 +638,29 @@ template <typename RW> void writeLockWhileReadLockedThreaded(RW &lock) {
636638

637639
const int threadCount = 10;
638640

639-
std::vector<int> results;
640-
results.assign(threadCount, 0);
641+
std::vector<std::atomic<int>> results(threadCount);
641642

642643
std::atomic<bool> done(false);
643644
threadedExecute(threadCount,
644645
[&](int index) {
645-
while (!done) {
646+
// Always perform at least one iteration of this loop to
647+
// avoid spurious failures if this thread is slow to run.
648+
do {
646649
lock.withWriteLock([&] {
647650
results[index] += 1;
648651
std::this_thread::sleep_for(
649652
std::chrono::milliseconds(5));
650653
});
651654
std::this_thread::sleep_for(std::chrono::milliseconds(1));
652-
}
655+
} while (!done);
653656
},
654657
[&] {
655658
std::this_thread::sleep_for(std::chrono::milliseconds(100));
656659
done = true;
657660
lock.readUnlock();
658661
});
659662

660-
for (auto result : results) {
663+
for (auto &result : results) {
661664
ASSERT_EQ(result, 1);
662665
}
663666
}
@@ -677,28 +680,29 @@ template <typename RW> void writeLockWhileWriteLockedThreaded(RW &lock) {
677680

678681
const int threadCount = 10;
679682

680-
std::vector<int> results;
681-
results.assign(threadCount, 0);
683+
std::vector<std::atomic<int>> results(threadCount);
682684

683685
std::atomic<bool> done(false);
684686
threadedExecute(threadCount,
685687
[&](int index) {
686-
while (!done) {
688+
// Always perform at least one iteration of this loop to
689+
// avoid spurious failures if this thread is slow to run.
690+
do {
687691
lock.withWriteLock([&] {
688692
results[index] += 1;
689693
std::this_thread::sleep_for(
690694
std::chrono::milliseconds(5));
691695
});
692696
std::this_thread::sleep_for(std::chrono::milliseconds(1));
693-
}
697+
} while (!done);
694698
},
695699
[&] {
696700
std::this_thread::sleep_for(std::chrono::milliseconds(100));
697701
done = true;
698702
lock.writeUnlock();
699703
});
700704

701-
for (auto result : results) {
705+
for (auto &result : results) {
702706
ASSERT_EQ(result, 1);
703707
}
704708
}
@@ -719,10 +723,12 @@ template <typename RW> void tryReadLockWhileWriteLockedThreaded(RW &lock) {
719723
std::atomic<bool> done(false);
720724
threadedExecute(10,
721725
[&](int) {
722-
while (!done) {
726+
// Always perform at least one iteration of this loop to
727+
// avoid spurious failures if this thread is slow to run.
728+
do {
723729
ASSERT_FALSE(lock.try_readLock());
724730
std::this_thread::sleep_for(std::chrono::milliseconds(1));
725-
}
731+
} while (!done);
726732
},
727733
[&] {
728734
std::this_thread::sleep_for(std::chrono::milliseconds(100));
@@ -747,19 +753,20 @@ template <typename RW> void tryReadLockWhileReadLockedThreaded(RW &lock) {
747753

748754
const int threadCount = 10;
749755

750-
std::vector<bool> results;
751-
results.assign(threadCount, false);
756+
std::vector<std::atomic<bool>> results(threadCount);
752757

753758
std::atomic<bool> done(false);
754759
threadedExecute(threadCount,
755760
[&](int index) {
756-
while (!done) {
761+
// Always perform at least one iteration of this loop to
762+
// avoid spurious failures if this thread is slow to run.
763+
do {
757764
ASSERT_TRUE(lock.try_readLock());
758765
results[index] = true;
759766
std::this_thread::sleep_for(std::chrono::milliseconds(5));
760767
lock.readUnlock();
761768
std::this_thread::sleep_for(std::chrono::milliseconds(1));
762-
}
769+
} while (!done);
763770
},
764771
[&] {
765772
std::this_thread::sleep_for(std::chrono::milliseconds(100));
@@ -768,7 +775,7 @@ template <typename RW> void tryReadLockWhileReadLockedThreaded(RW &lock) {
768775

769776
lock.readUnlock();
770777

771-
for (auto result : results) {
778+
for (auto &result : results) {
772779
ASSERT_TRUE(result);
773780
}
774781
}
@@ -789,10 +796,12 @@ template <typename RW> void tryWriteLockWhileWriteLockedThreaded(RW &lock) {
789796
std::atomic<bool> done(false);
790797
threadedExecute(10,
791798
[&](int) {
792-
while (!done) {
799+
// Always perform at least one iteration of this loop to
800+
// avoid spurious failures if this thread is slow to run.
801+
do {
793802
ASSERT_FALSE(lock.try_writeLock());
794803
std::this_thread::sleep_for(std::chrono::milliseconds(1));
795-
}
804+
} while (!done);
796805
},
797806
[&] {
798807
std::this_thread::sleep_for(std::chrono::milliseconds(100));
@@ -818,10 +827,12 @@ template <typename RW> void tryWriteLockWhileReadLockedThreaded(RW &lock) {
818827
std::atomic<bool> done(false);
819828
threadedExecute(10,
820829
[&](int) {
821-
while (!done) {
830+
// Always perform at least one iteration of this loop to
831+
// avoid spurious failures if this thread is slow to run.
832+
do {
822833
ASSERT_FALSE(lock.try_writeLock());
823834
std::this_thread::sleep_for(std::chrono::milliseconds(1));
824-
}
835+
} while (!done);
825836
},
826837
[&] {
827838
std::this_thread::sleep_for(std::chrono::milliseconds(100));

0 commit comments

Comments
 (0)