Readers and writers (writers-preference). I'm not sure whether my code is wrong or not [closed]

Question

I've found on the wikipedia pseudo-code, so I've decided to try it.

That's the Wiki's code:

int readcount, writecount; (initial value = 0)
semaphore mutex_1, mutex_2, mutex_3, w, r ; (initial value = 1)

READER
  P(mutex_3);
    P(r);
      P(mutex_1);
        readcount := readcount + 1;
        if readcount = 1 then P(w);
      V(mutex_1);
    V(r);
  V(mutex_3);

  reading is performed

  P(mutex_1);
    readcount := readcount - 1;
    if readcount = 0 then V(w);
  V(mutex_1);


WRITER
  P(mutex_2);
    writecount := writecount + 1;
    if writecount = 1 then P(r);
  V(mutex_2);

  P(w);
    writing is performed
  V(w);

  P(mutex_2);
    writecount := writecount - 1;
    if writecount = 0 then V(r);
  V(mutex_2);

I have implemented that into c++ under linux.

Here's my code:

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <time.h>
#include <iostream>
#include <string.h>
#include <sstream>

using namespace std;


int readcount = 0, writecount = 0;
pthread_mutex_t mutex_1, mutex_2, mutex_3, w, r;


// my own additional variables
int R, W; // read from main args ammount of reading and writing threads
int var_for_msg = 0; // should be 0 or 1 (number of writers actually in a library)
pthread_mutex_t  msg_lock; // locking message so two threads will not be able to write at the same time in the console
// ---------------------- //

void write_if_smth_changed()
{
pthread_mutex_lock(&msg_lock);  // lock msg_lock
cout<<"ReaderQ: " << R-readcount << " WriterQ: " << W - var_for_msg << " [in: R:" << readcount <<" W:"<< var_for_msg << "]" << endl;
pthread_mutex_unlock(&msg_lock); // unlock msg_lock

if (var_for_msg > 0)
{
cout<< "HAHAHAHAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" << endl;
}
}

void *f_write(void *writers_number)
{
while (true)
    {   
    pthread_mutex_lock(&mutex_2); 
    writecount++;   
    if (writecount == 1)
        {
        pthread_mutex_lock(&r);     
        }   
    pthread_mutex_unlock(&mutex_2);


    pthread_mutex_lock(&w); 


    var_for_msg++;  
    write_if_smth_changed();
    var_for_msg--;
    write_if_smth_changed();

    pthread_mutex_unlock(&w);

    pthread_mutex_lock(&mutex_2);

    writecount--;   
    if (writecount == 0)
        {       
        pthread_mutex_unlock(&r);
        }
    pthread_mutex_unlock(&mutex_2); 
    //sleep(1);
    }
}

void *f_read(void *readers_number)
{
while (true)
    {
    pthread_mutex_lock(&mutex_3);
        pthread_mutex_lock(&r);     
            pthread_mutex_lock(&mutex_1);
                readcount++;
                if (readcount == 1)
                    {                   
                    pthread_mutex_lock(&w);
                    }
            pthread_mutex_unlock(&mutex_1); 
        pthread_mutex_unlock(&r);

    pthread_mutex_unlock(&mutex_3);

    write_if_smth_changed();

    pthread_mutex_lock(&mutex_1);
    readcount--;    
    if (readcount == 0)
        {
            pthread_mutex_unlock(&w);
        }
    pthread_mutex_unlock(&mutex_1);
    //sleep(1);
    }
}


int main(int argc, char *argv[])
{

/* args input */
istringstream iss(argv[1]);
iss >> R;
istringstream iss2(argv[2]);
iss2 >> W;
/* ----------------------- */


pthread_mutex_init(&mutex_1,NULL);
pthread_mutex_init(&mutex_2,NULL);
pthread_mutex_init(&mutex_3,NULL);
pthread_mutex_init(&w,NULL);
pthread_mutex_init(&r,NULL);
pthread_mutex_init(&msg_lock,NULL);

pthread_t *thread_readers_array = new pthread_t[R];
pthread_t *thread_writers_array = new pthread_t[W];

for (int i = 0; i< R; i++)
{
pthread_create(&thread_readers_array[i],NULL,f_read,(void *)i); 
}

for (int i = 0; i< W; i++)
{
pthread_create(&thread_writers_array[i],NULL,f_write,(void *)i);    
}


pthread_join(thread_writers_array[0],NULL);
pthread_join(thread_readers_array[0],NULL); // 


return 0;
}

I've added in there a function which is supposed to write to the console the actual number of readers/writers in Queue and readers/writers inside library.
e.g.:

ReaderQ: 618 WriterQ: 5 [in: R: 382 W:0] would mean: 618 readers and 5 writers in queue, 382 readers and 0 writers inside library.

When I launch the executable file with args 1000 and 5 (1000 reading and 5 writing threads)

./a.out 1000 5

It should once in a while output HAHAHAHA~ (because there's a writer in the room) however it's not happening. Could this be possible that the algo presented in a wikipedia is wrong? If not, where's the problem?

Answer 1

This is not how you would write the code in C++ (this is C (and awful C at that)).

I would implement the class:

 class RWLock
 {
      friend class ReadLocker;
      friend class WriteLocker;

      void getReadLock();
      void unLockRead();

      void getWriteLock();
      void unLockWrite();


      private:
          // All the members you need here
      public:
          RWLock(): /*Initialize all members explicitly*/ {}
         ~RWLock() {/* TIDY UP */}
 };
 class ReadLocker
 {
      RWLock& lock;
      public:
          ReadLocker(RWLock& lock) : lock(lock) {lock.getReadLock();}
         ~ReadLocker()                          {lock.unlockRead();}
 };
 class WriteLocker
 {
      RWLock& lock;
      public:
          WriteLocker(RWLock& lock) : lock(lock) {lock.getWriteLock();}
         ~WriteLocker()                          {lock.unlockWrite();}
 };

Then usage would be:

 void Reader()
 {
     // Reader
     ReadLocker    lock(sourceLock);    // gets a read lock
     // do reading here
 }

 void Writer()
 {
     // Writter
     WriteLock     lock(sourceLock);   // gets a write lock.
     // do writting here
 }

Now all you have to implement are four methods the usage of the class is short and intuitive. And most importantly the user of the RWLock can not use it incorrectly and leave the locks in the incorrect state.

So now lets implement the getXXXLock()

Notice that we use RAII to call Lock/Unlock on any lockable objects in an exception safe manor. For this we use MutexLocker which will call lock() in the constructor of the object and unlock() in the destructor when it goes out of scope. In the few cases were we can not use RAII directly we still guarantee that the calls will be made in pairs because we guarantee that for each call to getXXXLock() we will also call unlockXXX().

void RWLock::getReadLock()
{
    MutexLocker lock3(mutex_3);
    MutexLocker lockR(r);
    MutexLocker lock1(mutex_1);

    ++readcount;
    if (readcount)
    {   Lock(w);   // This is OK
    }              // because we guarantee that 
                   // unlockRead() is called thus making sure we call
                   // the unlock() as a controlled pair.
}  

// reading is performed

void RWLock::unlockRead()
{
     MutexLocker lock1(mutex_1);
    --readcount;
     if (readcount == 0)
     {    unlock(w);     // See getReadLock()
     }
}


void RWLock::getWriteLock()
{
    {
        MutexLocker  lock2(mutex_2);
        ++writecount;
        if (writecount == 1)
        {   Lock(r);          // See lock(w) below
        }
    }

    lock(w);      // This is OK because we guarantee to call unlockWrite()
                  // Which is the counterpart to this call.
}

// writing is performed

void RWLock::unlockWrite()
{
    unlock(w);

    MutexLocker lock2(mutex_2);
    --writecount;
    if (writecount == 0)
    {   unlock(r);
    }
}