another trouble with semaphores

[jsrig88]

This is only part of the program that I'm needing to write. In its current form, I'm trying to use semaphores to allow writing threads to write to the lowercase letter buffer and reading threads to read from it, without the two happening simultaneously. Also only one writing thread can write to the buffer at one time, but up to three reading threads can read from it simulatenously. Finally, with the sole exception of the first time that the buffer is written to, I'm trying to keep the lowercase buffer from being written to before it is read. So it should be roughly write-read-write-read-write-read, or something like that.

My problem is this: Sometimes it's getting written to twice or more in a row. I've included a little bit of printf()-oriented code in the writeLower() method to help me debug this, and that's how I'm seeing this happen.

I thought by initializing sem_t lowercaseSemaphoreReady to 1, placing a sem_wait() for it at the beginning of the writeLower() method, and placing a sem_getvalue()-sem_post() combination at the end of the readLower method, everything would be fine. But somehow it's not working. Can anyone spot what's going wrong here please?

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <semaphore.h>
#include <time.h>

#ifndef false
#define false (0)
#endif
#ifndef true
#define true (!false)   
#endif

/* globals */

/* the semaphores */
/* to prevent reader/writer conflicts */
sem_t lowercaseSemaphoreWR;
sem_t capitalSemaphoreWR;
sem_t digitSemaphoreWR;

/* to prevent writer/writer conflicts */   
sem_t lowercaseSemaphoreW;
sem_t capitalSemaphoreW;
sem_t digitSemaphoreW;

/* to prevent reader/reader conflicts */
sem_t lowercaseSemaphoreR;
sem_t capitalSemaphoreR;
sem_t digitSemaphoreR;

/* to make sure a buffer is ready to be written to */
sem_t lowercaseSemaphoreReady;
sem_t capitalSemaphoreReady;
sem_t digitSemaphoreReady;

/* the three character buffers */
char lowercase = '+'; /* for a lowercase letter */
char capital = '+';   /* for a capital letter */
char digit = '+';     /* for a digit 0-9 */
        
/************************************************************************
* method:  int myRandom()                                               *
* purpose: to serve as a random number generator                        *
* returns: a random number between 0 aand 259                           *
*                                                                       *
* approach: This will start off by initializing a static int called     *
*       sentinel to 0, as well as declaring an int called theReturn.    *
*       Then it will test to see if sentinel is still equal to 0.  If   *
*       so the method will call srand(time(NULL)) to get the original   *
*       seed and set sentinel to 1 (to make sure that srand is only     *
*       called once during execution of the program).  Finally it will  *
*       call rand(), tweak the result with mod 260, set theReturn equal *
*       to the result, and return theReturn.                            *
************************************************************************/
int myRandom()
{
        static sentinel = 0;
        int theReturn;

        if (sentinel == 0) /* if this is the first time this method has been called */
        {
                srand(time(NULL)); /* fetch random seed */
                sentinel = 1;
        }

        theReturn = rand() % 260;
        return theReturn;
}

/************************************************************************
* method:  void readBuffers()                                           *
* purpose: to continuously randomly select a character buffer, read its *
*       contents, and print it onscreen                                 *
* returns N/A                                                           *
*                                                                       *
* approach:                                                             *
************************************************************************/
void readBuffers()
{
        int i = 0;
        for (i; i < 10; i++) 
        {
                int choice = 1;
        
                if (choice == 1)
                {
                        readLower();
                }
        }
}

/************************************************************************
* method:  void readLower()                                             *
* purpose: to read from the lowercase buffer and print its character    *
*       onscreen                                                        *
* returns: N/A                                                          *
*                                                                       *
* approach:                                                             *
************************************************************************/
void readLower()
{
        sem_wait(&lowercaseSemaphoreWR);
        sem_wait(&lowercaseSemaphoreR);
                 
        if (lowercase != '+')
        {
                printf("%c\n", lowercase);
        }

        sem_post(&lowercaseSemaphoreR);
        sem_post(&lowercaseSemaphoreWR);

        /* letting writing processes know that this buffer has been read from */
        int value;
        sem_getvalue(&lowercaseSemaphoreReady, &value);

        if (value == 0) /* if this is the first time the buffer has been read */
                                                                                                                                                      {               /* since the last time it was written to */
                sem_post(&lowercaseSemaphoreReady);
        }
}
                 
/************************************************************************
* method:  void writeLower()                                            *
* purpose: to write into the lowercase buffer                           *
* returns: N/A                                                          *
*                                                                       *
* approach:                                                             *
************************************************************************/
void writeLower(int x)
{
        int i;
        for (i = 0; i < 10; i++)
        {
                char temp = myRandom() % 26 + 97;

                sem_wait(&lowercaseSemaphoreReady);
                sem_wait(&lowercaseSemaphoreWR);
                sem_wait(&lowercaseSemaphoreW);
                 
                printf("%d - %d - %c\n",x,i,temp);
                lowercase = temp;

                sem_post(&lowercaseSemaphoreW);
                sem_post(&lowercaseSemaphoreWR);
                /* only reading threads can increment lowercaseSemaphoreReady */
        }
}
/************************************************************************
* method:  int main()                                                   *
* purpose: to serve as the driving/starting method for the program      *
* returns: 0 to the operating system to signal normal completion        *
*                                                                       *
* approach:                                                             *
************************************************************************/
int main()
{
        /* initializing all of the semaphores basically to 1, except for the
        reader/reader (R) ones, since up to three reading threads can execute
        on the same buffer at one time */
        sem_init(&lowercaseSemaphoreWR, 0, 1);
        sem_init(&capitalSemaphoreWR, 0, 1);   
        sem_init(&digitSemaphoreWR, 0, 1);
        sem_init(&lowercaseSemaphoreW, 0, 1);
        sem_init(&capitalSemaphoreW, 0, 1);
        sem_init(&digitSemaphoreW, 0, 1);
        sem_init(&lowercaseSemaphoreR, 0, 3);
        sem_init(&capitalSemaphoreR, 0, 3);
        sem_init(&digitSemaphoreR, 0, 3);
        sem_init(&lowercaseSemaphoreReady, 0, 1);
        sem_init(&capitalSemaphoreReady, 0, 1);
        sem_init(&digitSemaphoreReady, 0, 1);

        /* managing the threads */
        /* thread ID arrays */
        pthread_t idLowercaseWriter[10]; /* lowercase buffer writer threads */
        pthread_t idCapitalWriter[10]; /* capital buffer writer threads */   
        pthread_t idDigitWriter[10]; /* digit buffer writer threads */
        pthread_t idReader[10]; /* reader threads */
        
        /* readying for thread creation */
        pthread_attr_t attr;
        pthread_attr_init(&attr);
        pthread_attr_setscope(&attr, PTHREAD_SCOPE_SYSTEM);
        
        /* thread creation */
        int i = 0;
        for (i; i < 10; i++)
        {
                pthread_create(&idLowercaseWriter[i], &attr, &writeLower, i);
                pthread_create(&idReader[i], &attr, &readBuffers, 0);
        }
        
        return 0;
}

[C_ntua]

This cannot be right. Except if you canged it for testing?

void readBuffers()
{
        int i = 0;
        for (i; i < 10; i++) 
        {
                int choice = 1;
        
                if (choice == 1)
                {
                        readLower();
                }
        }
}

Also, you are using C99 standard to have "int choice=1" inside for-loop. You should use then

for (int i = 0; i < 10; i++)

[jsrig88]

That was more or less put there for testing. Eventually I would come back and allow for the other two buffers to be read from and written to, adding appropriate methods, but I was trying to get that one buffer and its methods working right first.