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 4732df4e2b03abca4443ff33f6b34253a8adbfd1..8ee26d901b9115d2af83d0e783373bfd8d24e468 100644 (file)
--- 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 5292cad2518026c0b9f17d84c1599a18f63b6ea9..4deb00aaaefe2e8b84ef7b0f1e92dc1a03adb22a 100644 (file)
--- 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