Mutex file reading synchronization

Question

I am trying to synchronize the reading of 5 files so each character is read in from a file and then from the next file another character and so on. At the end an array will print out the content. I am able to read from the files but the synchronization is way off. I tried to fix it with a control variable to only run the block of code when it is that files turn but I get a wonky output. Here is my section where I do work on the critical sections

while(!feof(drive1)) {
        if(control == 0) {
            pthread_mutex_lock(&thread1);
            //printf("Mutex lock\n");
            c = getc(drive1);
            printf("%c", (char)c);
            control = 1;
            pthread_mutex_unlock(&thread1);
            //printf("Mutex unlock\n");
        } else if(control == 1) {
            pthread_mutex_lock(&thread2);
            //printf("Mutex lock\n");
            a = getc(drive2);
            printf("%c", (char)a);
            control = 2;
            pthread_mutex_unlock(&thread2);
            //printf("Mutex unlock\n");
        } else if(control == 2) {
            pthread_mutex_lock(&thread3);
            //printf("Mutex lock\n");
            b = getc(drive3);
            printf("%c", (char)b);
            control = 3;
            pthread_mutex_unlock(&thread3);
            //printf("Mutex unlock\n");
        } else if(control == 3) {
            pthread_mutex_lock(&thread4);
            //printf("Mutex lock\n");
            d = getc(drive4);
            printf("%c", (char)d);
            control = 4;
            pthread_mutex_unlock(&thread4);
            //printf("Mutex unlock\n");
        } else if(control == 4) {
            pthread_mutex_lock(&thread5);
            //printf("Mutex lock\n");
            e = getc(drive5);
            printf("%c", (char)e);
            control = 0;
            pthread_mutex_unlock(&thread5);
            //printf("Mutex unlock\n");
        }

I initially tried it wwith only one thread1 being used to lock the mutexes and unlock them but then decided to create 5 to see if that would help, but it didn't. I also have to use 5 threads to do this for each of the files.

pthread_t th1;
    pthread_create(&th1, NULL, processing, NULL);
    pthread_t th2;
    pthread_create(&th2, NULL, processing, NULL);
    pthread_t th3;
    pthread_create(&th3, NULL, processing, NULL);
    pthread_t th4;
    pthread_create(&th4, NULL, processing, NULL);
    pthread_t th5;
    pthread_create(&th5, NULL, processing, NULL);
    pthread_join(th1, NULL);
    pthread_join(th2, NULL);
    pthread_join(th3, NULL);
    pthread_join(th4, NULL);
    pthread_join(th4, NULL);

This is the output I get

And the output is supposed to be "1234567890abcdefghij"

UPDATE: Based on one of the comments I have modified the code to use the variable "test" as what is being tested in the critical section. With this code I get the output 1212.

void* disk1(void* args) {
    //Initializing array of files
    FILE *drive[5];

    drive[0] = fopen("drive1.data", "r");
    drive[1] = fopen("drive2.data", "r");
    drive[2] = fopen("drive3.data", "r");
    drive[3] = fopen("drive4.data", "r");
    drive[4] = fopen("drive5.data", "r");


    int c;

    if(test < initialFileSize * 2) {
        pthread_mutex_lock(&thread1);
        if(test % 2 == 0) {
            c = getc(drive[0]);
            printf("%c", (char)c);
            test++;
        }
        if(test % 2 == 1) {
            c = getc(drive[1]);
            printf("%c", (char)c);
            test++;
        }
        pthread_mutex_unlock(&thread1);
    }
}

What do you think the mutexes should be doing? They only make sure that the statements in between lock() and unlock() happen in one thread. They do nothing to ensure that threads are given equal time slices, or run in any particular order. — Lee Daniel Crocker, Oct 15 '18 at 23:49
Then how would I implement some sort of schedule. We are not allowed to use semaphores. — Sai Peri, Oct 15 '18 at 23:51
This is related to, though different from, [Dynamically adding to char array with unknown size](https://stackoverflow.com/questions/52825270/dynamically-adding-to-char-array-with-unknown-size). — Jonathan Leffler, Oct 16 '18 at 00:00
Since each thread only looks at its own mutex, there isn't much mutual exclusion. And since all the threads look at the shared resource `control` without ensuring mutual exclusion, it's anyone's guess as to what any given process will see. Off the top of my head, I think you probably need to look at condition variables with a single mutex controlling access to `control`, and the threads locking the mutex, waiting on the condition, testing whether `control` is set to their value (waiting once more if not), and proceeding when the value indicates it is their turn. — Jonathan Leffler, Oct 16 '18 at 00:05
[Why `while (!feof(file))` is always wrong](https://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) — Barmar, Oct 16 '18 at 00:13

score 0 · Answer 1 · answered Oct 16 '18 at 00:30

you can synch them with a single flag but they will spend most of their time waiting so this is probably not optimal:

char shouldRead(){
  static unsigned int activeThread = 0; //used as index into the file to read and the thread compare.
  char x = 0;

  //wait for mutex
  pthread_mutex_lock(&readVariableMutex);
  //acquired mutex

  //read variable to see what thread should run
  if(availableThreads[activeThread] == myThreadId){
    x = getc(drive[activeThread]); //or w/e your doing when it should run
    activeThread++;
    activeThread %= MAX_THREADS; //should be 5 right
  }

  pthread_mutex_unlock(&readVariableMutex);
  return x;
}

//where ever you spawn your threads add them to a global availableThreads[MAX_THREADS] array;
//Also, when you intialize your FILE *'s, add them to a drive array.
//The above idea is to wrap access to variable "activeThread" around the mutex such that the controlling thread will always be the next thread, or it will do nothing and release the mutex.

All threads would have something like:

while(!eof(drive[thead_id])){ //Each thread needs to know when its at end of file somehow
  printf("%c",shouldRead());
}

note that readVariableMutex would be initialized prior to when you spawn your 5 threads (in main presumably), and would NOT be a thread but a pthread_mutex_t type instead. — Bwebb, Oct 16 '18 at 00:33

Preeti · Answer 2 · 2018-10-18T04:47:06.863

There are few points need to consider:

if(control == 0): The if condition is wrongly placed. When threads start executing the function, the counter is equal to 0. So, most probably more than one thread enters if condition.
In the next line, you are locking the mutex. But there may be threads which already entered the if condition. Mutext is only making the threads to execute next lines one by one. So one by one each thread is entering and reading from drive1 and printing first char. This is the reason, you are getting output “11111”
Same is true for other else if statements and you are getting output "111112222233333 ..."
while(!feof(drive1)): The while condition is checking for the end-of-file of file drive1. Depending on number of characters in drive1, it will loop. Instead, a check on the individual files or a check on the total number of characters to be printed will give a more accurate result.

Few points might help:

Mutex or any other lock is used to synchronize the access of the shared resource or variable or a code block. There may be multiple threads trying to access the code block or variable/resource. However, The number of mutexes needed is based on the number of variables or code blocks you wish to synchronize.
In this case, the expected output is a specific sequence. This can be achieved by synchronizing access to the entire block, where counter value is checked and a specific file is read and written to stdout. So, we have one code block as a critical section and we need one mutex.
If protecting a shared variable/resource which is used in multiple functions. We need to use one mutex to protect one variable/resource in all functions.

Please lock the mutex before checking the condition if(control == 0) and unlock after completing last else if. It will synchronize access to the code block. Also need to change while(!feof(drive1)) condition.

One way to solve it is given below.

I have used the total number of characters to be printed in while condition.
Used "total" as a volatile variable.
Checking the value of "total" again inside while loop, it's possible, the value of total is changed when a thread gets into the critical section.

With this solution, its possible that same thread may get the lock again and print values.

        while(total < TOTAL_CHAR_TO_PRINT)
        {
                pthread_mutex_lock(&m1);
                if(total >= TOTAL_CHAR_TO_PRINT)
                {
                        pthread_mutex_unlock(&m1);
                        return 0;
                }

                counter = counter % NUM_OF_FILES;
                d = fgetc(fptr[counter]);
                printf("%c", d);

                counter += 1;
                total += 1;

                pthread_mutex_unlock(&m1);
        }

Thanks for your detailed response. I haven't been able to try it yet though but I will keep you updated. I forgot to mention but the reason I am using feof(drive1) is because each drive file is of equal length. So as a character is read in from drive 1 I wanted it to do the same from the other drives. That was my initial logic. — Sai Peri, Oct 19 '18 at 03:57
Yes, it seems to work in this case. However, it's error-prone like if you add a space by mistake in drive1 but rest of the files already reached end-of-file. Also, you need another check inside while loop for threads which are waiting on the mutex. — Preeti, Oct 19 '18 at 09:59

Mutex file reading synchronization

2 Answers2