2

i'm making a program where there a 2 threads that analyze an array. One (consumer) checks if an index is 1, and if it is, it turns it into a 1. The other thread (producer) does the same but when it finds a 0, it turns it into a 1.

I used pthreads but I think that each thread is creating its own copy of the structure I pass them as an argument. They should be able to see the changes the other thread made to the array, but this is not happening.

Can someone tell me a way to make that both threads analyze the same array?

#include <stdio.h>
#include <pthread.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>

#define SPACE 5
#define SEM_GR 1
#define SEM_RE 0

//Declaring a structure to pass the arguments to the threads.
struct arg_struct {
  int * semaphore;
  int * arr;
};

//Function that prints the array passed as a pointer.
void print_arr (int * a){
    for(int i = 0; i < SPACE; i++){
    if(i == SPACE - 1){
      printf("%d\n", *(a+i));
    }
    else printf("%d ", *(a+i));
    }
}


void * producing (void * arguments){
    sleep(2);
    printf("Producer started.\n");

    //Initializing the arguments structure to use its members.
    struct arg_struct *args = arguments;
    printf("Producer modifying array in: %x\n", args -> arr);

    //Transverse the array.
    for (int i = 0; i < SPACE; i++){
      sleep(1);

      //If the index in the array is empty, and if the semaphore is in green.
      printf("Producer checking array...\n");
      if(* (args -> arr + i) == 0 && * (args -> semaphore) == SEM_GR){
        //Sets the semaphore in red.
        * (args -> semaphore) = SEM_RE;
        printf("Producing!\n");

        //Sets the index in the array as 1.
        * (args -> arr + i) = 1;
        print_arr(args -> arr);

        //Sets the semaphore in green.
        * (args -> semaphore) = SEM_GR;

        //When the index is the last one in the array, it returns to the first position.
        if(i == SPACE - 1){
          i = 0;
        }
      }
    }
    pthread_exit(NULL);
    return NULL;
}

void * consuming (void * arguments){
    sleep(2);
    printf("Consumer started.\n");

    struct arg_struct * args = arguments;
    printf("Consumer modifying array in: %x\n", args -> arr);
    printf("Consumer checking array...\n");
    for (int i = 0; i < SPACE; i++){
      sleep(1);
      if(* (args -> arr + i) == 1 && * (args -> semaphore) == SEM_GR){
        * (args -> semaphore) = SEM_RE;
        printf("Consuming!\n");
        * (args -> arr + i) = 0;
        print_arr(args -> arr);
        * (args -> semaphore) = SEM_GR;
        if(i == SPACE - 1){
          i = 0;
        }
      }
    }
    pthread_exit(NULL);
    return NULL;
}

int main() {

    int * sem;
    sem = (int *) malloc(sizeof(int));
    * sem = SEM_GR;

    int * a;
    a = (int *) malloc(SPACE * sizeof(int));

    for (int i = 0; i < SPACE; i++) {
        a[i] = 1;
    }

    pthread_t produce, consume;
    struct arg_struct args;

    args.semaphore = sem;
    args.arr = a;

    printf("Array address: %x\n", a);
    print_arr(a);

    pthread_create(&consume, NULL, &consuming, (void *)&args);
    pthread_create(&produce, NULL, &producing, (void *)&args);
    pthread_join(produce, NULL);
    pthread_join(consume, NULL);

    return 0;
}
Erick Siordia
  • 171
  • 1
  • 10
  • [Here](https://stackoverflow.com/questions/19844716/producer-consumer-program-using-semaphores-and-pthreads) answered your question. – ntshetty Feb 20 '18 at 02:51
  • 1
    pthreads don't create their own arrays (unless your code tells them to do so), so that isn't the problem. Allowing another thread to read (or write) data while a first thread may be writing to it, OTOH, invokes undefined behavior, unless you synchronize the accesses using mutexes. I don't see any synchronization being done in your code -- labelling an int array as 'semaphore' isn't sufficient. Use pthread_mutex_lock()/pthread_mutex_unlock()/etc instead, that will contain the necessary magic so that the proper synchronization operations occur. – Jeremy Friesner Feb 20 '18 at 03:11
  • If you add a usleep(500000); between the two calls to pthread_create, you can get one thread to start its work (and sleep calls) a half second delayed from the other one. This basically leaves enough time for each thread to operate on the array data without much chance for conflict. Also when you check if i == SPACE - 1 I think you intended to set i=-1, not i=0 because the for loop will increment it back up to 0. – Snohdo Feb 20 '18 at 03:49
  • Modern C's prefered tool to handle race conditions are atomic types. These are optional since C11, but many compilers/C libraries support them nowadays. Just declare a variable with the qualifier `_Atomic` and you are protected against races. – Jens Gustedt Feb 20 '18 at 07:54

1 Answers1

2

Can someone tell me a way to make that both threads analyze the same array?

Both of your threads seems to analyse the same array. But there are few race conditions in the code.

To avoid race conditions, you need to synchronise the threads.

Please note, that from optimiser point of view, a = 5; a = 6; is just a = 6;. So in the code semaphore = RED; semaphore = GREEN; will be optimised to just semaphore = GREEN; unless you tell the compiler explicitly that semaphore is not a normal variable, but a dedicated synchronisation primitive.

Same with the ordering of operation. The order of operations semaphore = RED; arr = 1; semaphore = GREEN; might be changed by compiler and the CPU to semaphore = GREEN; arr = 1;, unless you explicitly tell that the order does matter or use those synchronisation primitives which also take care about the ordering.

Overall, there are variety of methods and libraries for synchronisation. Just pick one, try to fix the example and come again with new questions ;)

Andriy Berestovskyy
  • 8,059
  • 3
  • 17
  • 33