Skip to content

Commit 7d14964

Browse files
arosenzweig3arosenzweig3
committed
Correctly set writer in ReadWriteMutex::tryUpgradeToWrite(), add tests
Signed-off-by: Adam M. Rosenzweig <arosenzweig3@bloomberg.net>
1 parent 087555c commit 7d14964

File tree

2 files changed

+185
-6
lines changed

2 files changed

+185
-6
lines changed

‎quantum/impl/quantum_read_write_mutex_impl.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,12 @@ void ReadWriteMutex::upgradeToWrite(ICoroSync::Ptr sync)
100100
inline
101101
bool ReadWriteMutex::tryUpgradeToWrite()
102102
{
103-
return _spinlock.tryUpgradeToWrite();
103+
bool rc = _spinlock.tryUpgradeToWrite();
104+
if (rc)
105+
{
106+
_taskId = local::taskId();
107+
}
108+
return rc;
104109
}
105110

106111
inline
@@ -112,11 +117,9 @@ void ReadWriteMutex::unlockRead()
112117
inline
113118
void ReadWriteMutex::unlockWrite()
114119
{
115-
if (_taskId != local::taskId()) {
116-
//invalid operation
117-
assert(false);
118-
}
120+
assert(_taskId == local::taskId());
119121
_taskId = TaskId{}; //reset the task id
122+
120123
_spinlock.unlockWrite();
121124
}
122125

‎tests/quantum_locks_tests.cpp‎

Lines changed: 177 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,29 @@ TEST(ReadWriteMutex, SingleLocks)
543543
EXPECT_FALSE(mutex.isReadLocked());
544544
EXPECT_TRUE(mutex.isWriteLocked());
545545
EXPECT_EQ(0, mutex.numReaders());
546+
547+
mutex.unlockWrite();
548+
EXPECT_FALSE(mutex.isLocked());
549+
EXPECT_FALSE(mutex.isReadLocked());
550+
EXPECT_FALSE(mutex.isWriteLocked());
551+
EXPECT_EQ(0, mutex.numReaders());
552+
553+
mutex.lockRead();
554+
EXPECT_TRUE(mutex.isLocked());
555+
EXPECT_TRUE(mutex.isReadLocked());
556+
EXPECT_FALSE(mutex.isWriteLocked());
557+
EXPECT_EQ(1, mutex.numReaders());
558+
mutex.upgradeToWrite();
559+
EXPECT_TRUE(mutex.isLocked());
560+
EXPECT_FALSE(mutex.isReadLocked());
561+
EXPECT_TRUE(mutex.isWriteLocked());
562+
EXPECT_EQ(0, mutex.numReaders());
563+
564+
mutex.unlockWrite();
565+
EXPECT_FALSE(mutex.isLocked());
566+
EXPECT_FALSE(mutex.isReadLocked());
567+
EXPECT_FALSE(mutex.isWriteLocked());
568+
EXPECT_EQ(0, mutex.numReaders());
546569
}
547570

548571
TEST(ReadWriteMutex, TryLocks)
@@ -562,6 +585,18 @@ TEST(ReadWriteMutex, TryLocks)
562585
EXPECT_TRUE(mutex.isWriteLocked());
563586
EXPECT_FALSE(mutex.tryLockRead());
564587
EXPECT_FALSE(mutex.isReadLocked());
588+
589+
mutex.unlockWrite();
590+
591+
mutex.lockRead();
592+
EXPECT_TRUE(mutex.isReadLocked());
593+
EXPECT_FALSE(mutex.tryLockWrite());
594+
EXPECT_TRUE(mutex.isReadLocked());
595+
EXPECT_TRUE(mutex.tryUpgradeToWrite());
596+
EXPECT_TRUE(mutex.isWriteLocked());
597+
EXPECT_FALSE(mutex.tryLockRead());
598+
EXPECT_FALSE(mutex.isReadLocked());
599+
565600
}
566601

567602
TEST(ReadWriteMutex, Guards)
@@ -574,36 +609,130 @@ TEST(ReadWriteMutex, Guards)
574609
{
575610
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
576611
EXPECT_TRUE(mutex.isReadLocked());
612+
EXPECT_TRUE(guard.ownsLock());
613+
EXPECT_TRUE(guard.ownsReadLock());
614+
EXPECT_FALSE(guard.ownsWriteLock());
577615
}
578616
EXPECT_FALSE(mutex.isLocked());
579617

580618
// Guard, TryLock
581619
{
582620
ReadWriteMutex::Guard guard(mutex, lock::acquireRead, lock::tryToLock);
583621
EXPECT_TRUE(mutex.isReadLocked());
622+
EXPECT_TRUE(guard.ownsLock());
623+
EXPECT_TRUE(guard.ownsReadLock());
624+
EXPECT_FALSE(guard.ownsWriteLock());
584625
}
585626
EXPECT_FALSE(mutex.isLocked());
586627

587628
// WriteGuard
588629
{
589630
ReadWriteMutex::Guard guard(mutex, lock::acquireWrite);
590631
EXPECT_TRUE(mutex.isWriteLocked());
632+
EXPECT_TRUE(guard.ownsLock());
633+
EXPECT_FALSE(guard.ownsReadLock());
634+
EXPECT_TRUE(guard.ownsWriteLock());
591635
}
592636
EXPECT_FALSE(mutex.isLocked());
593637

594638
// WriteGuard, TryLock
595639
{
596-
ReadWriteMutex::Guard writeGuard(mutex, lock::acquireWrite, lock::tryToLock);
640+
ReadWriteMutex::Guard guard(mutex, lock::acquireWrite, lock::tryToLock);
597641
EXPECT_TRUE(mutex.isWriteLocked());
642+
EXPECT_TRUE(guard.ownsLock());
643+
EXPECT_FALSE(guard.ownsReadLock());
644+
EXPECT_TRUE(guard.ownsWriteLock());
598645
}
599646
EXPECT_FALSE(mutex.isLocked());
600647

601648
// Guard, WriteGuard, TryLock (fails)
602649
{
603650
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
604651
EXPECT_TRUE(mutex.isReadLocked());
652+
EXPECT_TRUE(guard.ownsLock());
653+
EXPECT_TRUE(guard.ownsReadLock());
654+
EXPECT_FALSE(guard.ownsWriteLock());
605655
ReadWriteMutex::Guard writeGuard(mutex, lock::acquireWrite, lock::tryToLock);
606656
EXPECT_FALSE(mutex.isWriteLocked());
657+
EXPECT_FALSE(writeGuard.ownsLock());
658+
EXPECT_FALSE(writeGuard.ownsReadLock());
659+
EXPECT_FALSE(writeGuard.ownsWriteLock());
660+
}
661+
EXPECT_FALSE(mutex.isLocked());
662+
663+
// Guard, adoptLock (read)
664+
{
665+
mutex.lockRead();
666+
ReadWriteMutex::Guard guard(mutex, lock::adoptLock);
667+
EXPECT_TRUE(guard.ownsLock());
668+
EXPECT_TRUE(guard.ownsReadLock());
669+
EXPECT_FALSE(guard.ownsWriteLock());
670+
}
671+
EXPECT_FALSE(mutex.isLocked());
672+
673+
// Guard, adoptLock (write)
674+
{
675+
mutex.lockWrite();
676+
ReadWriteMutex::Guard guard(mutex, lock::adoptLock);
677+
EXPECT_TRUE(guard.ownsLock());
678+
EXPECT_FALSE(guard.ownsReadLock());
679+
EXPECT_TRUE(guard.ownsWriteLock());
680+
}
681+
EXPECT_FALSE(mutex.isLocked());
682+
683+
// Guard, tryLockRead
684+
{
685+
mutex.lockWrite();
686+
ReadWriteMutex::Guard guard(mutex, lock::acquireRead, lock::tryToLock);
687+
EXPECT_FALSE(guard.ownsLock());
688+
EXPECT_FALSE(guard.tryLockRead());
689+
EXPECT_FALSE(guard.ownsLock());
690+
691+
mutex.unlockWrite();
692+
mutex.lockRead();
693+
EXPECT_FALSE(guard.tryLockWrite());
694+
EXPECT_FALSE(guard.ownsLock());
695+
mutex.unlockRead();
696+
}
697+
698+
// Guard, upgrade
699+
{
700+
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
701+
EXPECT_TRUE(mutex.isReadLocked());
702+
EXPECT_TRUE(guard.ownsLock());
703+
EXPECT_TRUE(guard.ownsReadLock());
704+
EXPECT_FALSE(guard.ownsWriteLock());
705+
guard.upgradeToWrite();
706+
EXPECT_TRUE(mutex.isWriteLocked());
707+
EXPECT_TRUE(guard.ownsLock());
708+
EXPECT_FALSE(guard.ownsReadLock());
709+
EXPECT_TRUE(guard.ownsWriteLock());
710+
}
711+
EXPECT_FALSE(mutex.isLocked());
712+
713+
// Guard, tryUpgrade (success)
714+
{
715+
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
716+
EXPECT_TRUE(mutex.isReadLocked());
717+
EXPECT_TRUE(guard.ownsLock());
718+
EXPECT_TRUE(guard.tryUpgradeToWrite());
719+
EXPECT_TRUE(mutex.isWriteLocked());
720+
EXPECT_TRUE(guard.ownsLock());
721+
EXPECT_FALSE(guard.ownsReadLock());
722+
EXPECT_TRUE(guard.ownsWriteLock());
723+
}
724+
725+
// Guard, tryUpgrade (failure)
726+
{
727+
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
728+
EXPECT_TRUE(mutex.isReadLocked());
729+
EXPECT_TRUE(guard.ownsLock());
730+
mutex.lockRead();
731+
EXPECT_FALSE(guard.tryUpgradeToWrite());
732+
EXPECT_TRUE(guard.ownsLock());
733+
EXPECT_TRUE(guard.ownsReadLock());
734+
EXPECT_FALSE(guard.ownsWriteLock());
735+
mutex.unlockRead();
607736
}
608737
EXPECT_FALSE(mutex.isLocked());
609738

@@ -613,8 +742,14 @@ TEST(ReadWriteMutex, Guards)
613742
EXPECT_TRUE(mutex.isReadLocked());
614743
guard.unlock();
615744
EXPECT_FALSE(mutex.isLocked());
745+
EXPECT_FALSE(guard.ownsLock());
746+
EXPECT_FALSE(guard.ownsReadLock());
747+
EXPECT_FALSE(guard.ownsWriteLock());
616748
ReadWriteMutex::Guard writeGuard(mutex, lock::acquireWrite, lock::tryToLock);
617749
EXPECT_TRUE(mutex.isWriteLocked());
750+
EXPECT_TRUE(writeGuard.ownsLock());
751+
EXPECT_FALSE(writeGuard.ownsReadLock());
752+
EXPECT_TRUE(writeGuard.ownsWriteLock());
618753
}
619754
EXPECT_FALSE(mutex.isLocked());
620755

@@ -624,12 +759,53 @@ TEST(ReadWriteMutex, Guards)
624759
EXPECT_TRUE(mutex.isReadLocked());
625760
guard.unlock();
626761
EXPECT_FALSE(mutex.isLocked());
762+
EXPECT_FALSE(guard.ownsLock());
763+
EXPECT_FALSE(guard.ownsReadLock());
764+
EXPECT_FALSE(guard.ownsWriteLock());
627765
ReadWriteMutex::Guard writeGuard(mutex, lock::acquireWrite, lock::tryToLock);
628766
EXPECT_TRUE(mutex.isWriteLocked());
767+
EXPECT_TRUE(writeGuard.ownsLock());
768+
EXPECT_FALSE(writeGuard.ownsReadLock());
769+
EXPECT_TRUE(writeGuard.ownsWriteLock());
629770
writeGuard.unlock();
630771
EXPECT_FALSE(mutex.isLocked());
772+
EXPECT_FALSE(writeGuard.ownsLock());
773+
EXPECT_FALSE(guard.ownsReadLock());
774+
EXPECT_FALSE(guard.ownsWriteLock());
631775
}
632776
EXPECT_FALSE(mutex.isLocked());
777+
778+
// Guard (read), release
779+
{
780+
ReadWriteMutex::Guard guard(mutex, lock::acquireRead);
781+
EXPECT_TRUE(mutex.isReadLocked());
782+
EXPECT_TRUE(guard.ownsLock());
783+
EXPECT_TRUE(guard.ownsReadLock());
784+
EXPECT_FALSE(guard.ownsWriteLock());
785+
guard.release();
786+
EXPECT_TRUE(mutex.isReadLocked());
787+
EXPECT_FALSE(guard.ownsLock());
788+
EXPECT_FALSE(guard.ownsReadLock());
789+
EXPECT_FALSE(guard.ownsWriteLock());
790+
}
791+
EXPECT_TRUE(mutex.isReadLocked());
792+
mutex.unlockRead();
793+
794+
// Guard (write), release
795+
{
796+
ReadWriteMutex::Guard guard(mutex, lock::acquireWrite);
797+
EXPECT_TRUE(mutex.isWriteLocked());
798+
EXPECT_TRUE(guard.ownsLock());
799+
EXPECT_FALSE(guard.ownsReadLock());
800+
EXPECT_TRUE(guard.ownsWriteLock());
801+
guard.release();
802+
EXPECT_TRUE(mutex.isWriteLocked());
803+
EXPECT_FALSE(guard.ownsLock());
804+
EXPECT_FALSE(guard.ownsReadLock());
805+
EXPECT_FALSE(guard.ownsWriteLock());
806+
}
807+
EXPECT_TRUE(mutex.isWriteLocked());
808+
mutex.unlockWrite();
633809
}
634810

635811
TEST(ReadWriteMutex, MultipleReadLocks) {

0 commit comments

Comments
 (0)