HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Implement a blocking counter with a reader/writer lock

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementwithwriterblockingcounterreaderlock

Problem

I'm trying to implement a blocking counter that may have several readers and writers at the same time:

class BlockingCounter {
public:
    BlockingCounter(int count) : count_(count) {
        // init lock_;
    }
    ~BlockingCounter() {
        // destroy lock_;
    }

    void Increment() {
        pthread_rwlock_wrlock(&lock_);
        count_++;
        pthread_rwlock_unlock(&lock_);

    }

    void Decrement() {
        pthread_rwlock_wrlock(&lock_);
        count_--;
        pthread_rwlock_unlock(&lock_);

    }

    int GetCount() {
        pthread_rwlock_rdlock(&lock_);
        int count = count_;
        pthread_rwlock_unlock(&lock_);
        return count;
    }

private:
    pthread_rwlock_t lock_;
    int count_;

};


Does this code have problems under multi-threading circumstances?

I intended to use it like this:

Main thread:

BlockingCounter *counter = new BlockingCounter(10);

// Here 10 threads are started. See code below.
while (counter->GetCount() > 0) {
  ConditionVar.wait();
}
// Main thread resumes execution.


Other 10 thread(s):

// Do stuff
counter.Decrement();
ConditionVar.notify();

Solution

You are really writing C

C++ has its own language specific features to handle multi threading.
Your interface is broken.

The only way to test the value of the counter is via GetCount() but by the time you can use the value the counter may have changed.

Thread1                      Thread2                    Thread3
 if (b.GetCount() == 0) {     if (b.GetCount() == 0) {   if (b.GetCount() == 0) {
     b.Increment();                b.Increment();             b.Increment();
     // I have the lock.
     Do stuff                      Do stuff                   Do stuff
 }                            }                          }


As you can see your mechanism does not stop all three threads from doing something. The only real use for the counter is to print the count (and even that may not be accurate but at least it wont break anything).

Code Snippets

Thread1                      Thread2                    Thread3
 if (b.GetCount() == 0) {     if (b.GetCount() == 0) {   if (b.GetCount() == 0) {
     b.Increment();                b.Increment();             b.Increment();
     // I have the lock.
     Do stuff                      Do stuff                   Do stuff
 }                            }                          }

Context

StackExchange Code Review Q#74493, answer score: 4

Revisions (0)

No revisions yet.