Help using semphores and threads

Question

I am using the pthread library to simulate a threaded buffer. I am also using semaphores as a solution to accessing critical section variables one at a time.

The main problem is that the producer is filling the entire buffer and the consumer is then emptying the entire buffer. Is this code correct? I was assuming that the production and consumption would occur before the buffer was full or empty.

Here is my code and any comments would help a lot, and yes this is for a class.

Thank you in advance

void *Producer(void *threadid) 
{
   long tid;
   tid = (long)threadid;

   while (c < Cycles)  //While stuff to buffer
   {                        
        pthread_mutex_lock(&lock);           
        while(size == BUFFER_SIZE)  
        {
           pthread_cond_wait(&cond, &lock);    
        }
     buffer [full] = rand();
     data << size+1 << ". Produce: " << buffer[full] << endl;
     printBuffer();
     full = (full + 1) % BUFFER_SIZE;
     size++;
     pthread_cond_signal(&cond1);
     pthread_mutex_unlock(&lock);
     c++;
   }
   pthread_exit(NULL);
}

You can also download all the code or see the log file...

download main.cpp view the log file at funkohland.com/pthreads/log.txt

Answer 1

This is a well known problem with Mutexes. A Mutex is an expensive operation that requires lots of cycles. When you unlock the mutex the other thread has a TINY opportunity in which to exit its lock and gain a lock. Basially you need to spend less time in the mutex to give the other thread an opportunity to run. Basically you need to choose the portion of code that ACTUALLY required the mutex and then lock the mutex quickly do whatever you need to do with that variable (and nothing more) and then unlock it.

Answer 2

For starters, you should ensure that access to shared variables 'c' and 'size' are carried inside a critical section. Thats between a lock and unlock calls. Currently you are freely accessing 'c' and sometimes 'size' without proper locking.

Answer 3

While Goz's answer is the reason, the possible solution is to yield to scheduler outside the mutex, either using usleep(1) or sched_yield()/Sleep(0) (depending on OS).

Also, since your producer uses rand(), it makes little difference but if it was using time-consuming I/O or advanced algorithms for making the new objects to be inserted, the right approach is to perform the actual production unmutexed, into a local buffer, then only mutex inserting the newly created object into the queue.