From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 21 Jun 2013 22:04:04 +0000 (-0600)
Subject: Make !lock.readers and !lock.writers assertions safe.
X-Git-Tag: SQUID_3_5_0_1~444^2~57
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=92e297979096e99bcb872178b03a47747dc310a5;p=thirdparty%2Fsquid.git

Make !lock.readers and !lock.writers assertions safe.

The lock class used readers level counter to count both attempts to read and
current readers. The attempts part made assertions declaring that there should
be no readers unsafe because even a writing entry may have a reading attempt.
Same for writers counter: A reading entry may have a writing attempt.

We now segragate the attempts level, which is internal information required
for shared lock to work, from counting the number of successful attempts
(i.e., actual readers and writers), which is public information useful for
assertions, stats, etc.
---

diff --git a/src/ipc/ReadWriteLock.cc b/src/ipc/ReadWriteLock.cc
index 4732df4e2b..8ee26d901b 100644
--- a/src/ipc/ReadWriteLock.cc
+++ b/src/ipc/ReadWriteLock.cc
@@ -9,49 +9,59 @@
 bool
 Ipc::ReadWriteLock::lockShared()
 {
-    ++readers; // this locks "new" writers out
-    if (!writers || appending) // there are no old writers or sharing is OK
+    ++readLevel; // this locks "new" writers out
+    if (!writeLevel || appending) { // nobody is writing, or sharing is OK
+        ++readers;
         return true;
-    --readers;
+    }
+    --readLevel;
     return false;
 }
 
 bool
 Ipc::ReadWriteLock::lockExclusive()
 {
-    if (!writers++) { // we are the first writer + this locks "new" readers out
-        if (!readers) // there are no old readers
+    if (!writeLevel++) { // we are the first writer + lock "new" readers out
+        if (!readLevel) { // no old readers and nobody is becoming one
+            writing = true;
             return true;
+        }
     }
-    --writers;
+    --writeLevel;
     return false;
 }
 
 void
 Ipc::ReadWriteLock::unlockShared()
 {
-    assert(readers-- > 0);
+    assert(readers > 0);
+    --readers;
+    --readLevel;
 }
 
 void
 Ipc::ReadWriteLock::unlockExclusive()
 {
-    appending = 0;
-    assert(writers-- > 0);
+    assert(writing);
+    appending = false;
+    writing = false;
+    --writeLevel;
 }
 
 void
 Ipc::ReadWriteLock::switchExclusiveToShared()
 {
-    ++readers; // must be done before we release exclusive control
+    assert(writing);
+    ++readLevel; // must be done before we release exclusive control
+    ++readers;
     unlockExclusive();
 }
 
 void
 Ipc::ReadWriteLock::startAppending()
 {
-    assert(writers > 0);
-    appending++;
+    assert(writing);
+    appending = true;
 }
 
 void
@@ -60,9 +70,9 @@ Ipc::ReadWriteLock::updateStats(ReadWriteLockStats &stats) const
     if (readers) {
         ++stats.readable;
         stats.readers += readers;
-    } else if (writers) {
+    } else if (writing) {
         ++stats.writeable;
-        stats.writers += writers;
+        ++stats.writers;
         stats.appenders += appending;
     } else {
         ++stats.idle;
diff --git a/src/ipc/ReadWriteLock.h b/src/ipc/ReadWriteLock.h
index 5292cad251..4deb00aaae 100644
--- a/src/ipc/ReadWriteLock.h
+++ b/src/ipc/ReadWriteLock.h
@@ -31,9 +31,13 @@ public:
     void updateStats(ReadWriteLockStats &stats) const;
 
 public:
-    mutable Atomic::Word readers; ///< number of users trying to read
-    Atomic::Word writers; ///< number of writers trying to modify protected data
+    mutable Atomic::Word readers; ///< number of reading users
+    Atomic::Word writing; ///< there is a writing user (there can be at most 1)
     Atomic::Word appending; ///< the writer has promissed to only append
+
+private:
+    mutable Atomic::Word readLevel; ///< number of users reading (or trying to)
+    Atomic::Word writeLevel; ///< number of users writing (or trying to write)
 };
 
 /// approximate stats of a set of ReadWriteLocks