Thread: Problem with multithreading and pipe

  1. #1
    Registered User
    Join Date
    May 2020
    Posts
    10

    Problem with multithreading and pipe

    Hi guys, I'm trying to make a script that uses 3 threads that have the task of searching and sending to a memorizer via a pipe, the prime numbers up to a certain n (passed as an input parameter). All 3 threads must work at the same time.
    I have 2 problems basically ...
    The first is that the threads don't analyze all the numbers, some are skipped, I don't understand why.
    The second is that if I launch the main file and then the memorizer, they both stay planted and give no output.
    Could you please help me?

    Main file
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <unistd.h>
    #include <pthread.h>
    #include <sys/types.h>
    #include <sys/stat.h>
    #include <fcntl.h>
    #include <errno.h>
    
    
    pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
    int num, n ; 
    int fp ; 
    
    
    
    
    
    
    void *find_prime_number (void* name){
    
    
    int *th  = (int *) name;
    //while(num <= n ){
     int mynum;
     while (pthread_mutex_lock(&mutex), mynum = num++, pthread_mutex_unlock(&mutex), mynum <= n)
      {
    
    
    int check = 0; 
    
    
    ////
    
    
    
    
    for(int i=2 ;i<=num/2; i++ ){
       if(num<=n){
           if (num % i==0 ){
         printf("Calc%d : %d NON è un numero primo \n", *th, num); 
         check=1; 
         break; 
       }
    
    
    }
    
    
    
    
    else{
    
    
    return 0 ; 
    
    
        }
    }
    /////
    
    
    if (num<=n){
        if(check == 0){
            
            printf("Calc%d : %d  è un numero primo , lo invio al memorizzatore\n", *th, num); 
            write (fp, &num , sizeof(num)); 
    
    
        }
    
    
    }
     else {
    
    
        return 0 ; 
    
    
    
    
     }
    
    
    
    
    if(num <= n){
        num++; 
    
    
    }
    
    
    
    
    }
    
    
    
    
    }
    
    
    int main(int argc , char *argv[]){
    
    
    char *fifo =  "myfifo" ;
    
    
    if (argc != 2 ){
    printf("Errore nell'immissione dei parametri \n ");
    return -1 ; 
        } 
    
    
    int error ; 
    n= atoi(argv[1]); 
    pthread_t t1 , t2 , t3 ; 
    pthread_t *calc_arr[3] = { &t1 , &t2 , &t3 } ;  //array per l'id dei thread
    
    
    if (error = mkfifo( fifo , 0666)== -1){
    
    
    //    printf("Errore nella pipe fifo \n"); 
        perror(fifo);
        if (errno != EEXIST){
        return -1; 
        }
    
    
    
    
    
    
    }
    
    
    if ((fp = open(fifo, O_RDONLY))==-1 ){
    
    
     perror(fifo);
     //if (errno != EEXIST)
     //return -1 ; 
     return 1; 
    }
    
    
    
    
    int thread[]= {1,2,3}; 
    
    
    for(int i = 0; i<3 ; i++ ){
    if(error = pthread_create(calc_arr[i], NULL,find_prime_number,&thread[i]) != 0  ){
    
    
    perror("Errore nella creazione dei thread");
    exit(1);
    
    
         }
    
    
    }
    
    
    for (int i = 0; i < 3 ; i++){
    
    
    if (error=pthread_join(*calc_arr[i], NULL )!= 0  ){
        
    perror("Errore nel join dei thread ");
    exit(1);
       }
    
    
    }
    
    
    close(fp);
    printf("Terminazione dei calcolatori \n"); 
    return 0 ; 
    
    
    }
    Memorizer.c

    Code:
    #include <fcntl.h>#include <stdio.h>
    #include <stdlib.h>
    #include <sys/stat.h>
    #include <unistd.h>
    
    
    #define MAX_BUF 1024
    #define MAX_PID 8
    
    
    int main()
    {
        int fd ; 
        int bytesread;
        char * myfifo = "myfifo";
        char buf[MAX_BUF];
    
    
        fd = open( myfifo, O_RDONLY );
       while(1)
    {
        if((bytesread = read( fd, buf, MAX_BUF - 1)) > 0)
        {
            buf[bytesread] = '\0';
            printf("Received: %s\n", buf);
        }
        else
            break;
    }
        close(fd);
    
    
        return 0;
    }
    Last edited by Keplero90; 07-03-2021 at 04:13 AM.

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,656
    If you're going to write code, at least learn how to indent.
    You don't have a hope in hell of working out what's going on if your code is all over the place.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <unistd.h>
    #include <pthread.h>
    #include <sys/types.h>
    #include <sys/stat.h>
    #include <fcntl.h>
    #include <errno.h>
    
    
    pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
    int num, n;
    int fp;
    
    void *find_prime_number(void *name)
    {
      int *th = (int *) name;
    //while(num <= n ){
      int mynum;
      while (pthread_mutex_lock(&mutex), mynum = num++, pthread_mutex_unlock(&mutex), mynum <= n) {
        int check = 0;
        for (int i = 2; i <= num / 2; i++) {
          if (num <= n) {
            if (num % i == 0) {
              printf("Calc%d : %d NON è un numero primo \n", *th, num);
              check = 1;
              break;
            }
          }
          else {
            return 0;
          }
        }
    
        if (num <= n) {
          if (check == 0) {
            printf("Calc%d : %d  è un numero primo , lo invio al memorizzatore\n", *th, num);
            write(fp, &num, sizeof(num));
          }
        } else {
          return 0;
        }
        if (num <= n) {
          num++;
        }
      }
    }
    
    
    int main(int argc, char *argv[])
    {
      char *fifo = "myfifo";
    
      if (argc != 2) {
        printf("Errore nell'immissione dei parametri \n ");
        return -1;
      }
    
      int error;
      n = atoi(argv[1]);
      pthread_t t1, t2, t3;
      pthread_t *calc_arr[3] = { &t1, &t2, &t3 }; //array per l'id dei thread
    
      if (error = mkfifo(fifo, 0666) == -1) {
        //    printf("Errore nella pipe fifo \n");
        perror(fifo);
        if (errno != EEXIST) {
          return -1;
        }
      }
    
      if ((fp = open(fifo, O_RDONLY)) == -1) {
        perror(fifo);
        //if (errno != EEXIST)
        //return -1 ;
        return 1;
      }
    
      int thread[] = { 1, 2, 3 };
      for (int i = 0; i < 3; i++) {
        if (error = pthread_create(calc_arr[i], NULL, find_prime_number, &thread[i]) != 0) {
          perror("Errore nella creazione dei thread");
          exit(1);
        }
      }
    
      for (int i = 0; i < 3; i++) {
        if (error = pthread_join(*calc_arr[i], NULL) != 0) {
          perror("Errore nel join dei thread ");
          exit(1);
        }
      }
    
      close(fp);
      printf("Terminazione dei calcolatori \n");
      return 0;
    }
    
    
    
    #include <fcntl.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <sys/stat.h>
    #include <unistd.h>
    
    #define MAX_BUF 1024
    #define MAX_PID 8
    
    int main()
    {
      int fd;
      int bytesread;
      char *myfifo = "myfifo";
      char buf[MAX_BUF];
    
      fd = open(myfifo, O_RDONLY);
      while (1) {
        if ((bytesread = read(fd, buf, MAX_BUF - 1)) > 0) {
          buf[bytesread] = '\0';
          printf("Received: %s\n", buf);
        } else
          break;
      }
      close(fd);
    
      return 0;
    }
    > while (pthread_mutex_lock(&mutex), mynum = num++, pthread_mutex_unlock(&mutex), mynum <= n)
    What are you doing!?
    Abusing the comma operator for one thing.
    Having lock and unlock in there just makes locking a complete waste of time. Your loop body isn't guarded at all.

    All those 'else return 0;' calls in find_prime_number() pretty much made sure nothing happens.

    Before you try and make a threaded version, at least make sure the single threaded version works.
    You start single threaded, make it work, then make threads out of it.
    You don't start hacking threads and then try and make the logic work.

    > if (error = pthread_create(calc_arr[i], NULL, find_prime_number, &thread[i]) != 0)
    Beware of operator precedence issues. You're not comparing the result of pthread_create with zero.

    > pthread_t *calc_arr[3]
    Why not just pthread_t calc_arr[3] ?

    All those global variables need to go away, except for the mutex.
    Most people associate 'fp' as being a FILE* (from fopen), and 'fd' as being an int descriptor from the likes of open.


    Code:
    typedef struct {
        int fd;  // not fp
        int num;  // pick a better name
        int n;  // pick a better name
    } thread_param_t;
    
    
    void *find_prime_number(void *name)
    {
      thread_param_t *params = (thread_param_t*) name;
      // use params->member for your needs here
    
      // important!
      return NULL;
    }
    
    ///
    int main ( ) {
      thread_param_t params[3];
      pthread_t calc_arr[3];
      for ( int i = 0 ; i < 3 ; i++ ) {
        // setup params[i].member
        pthread_create(&calc_arr[i], NULL, find_prime_number, &params[i]);
      }
    }
    This is also wrong.
    Code:
        if ((bytesread = read(fd, buf, MAX_BUF - 1)) > 0) {
          buf[bytesread] = '\0';
          printf("Received: %s\n", buf);
    Except you're not writing printable data, only raw integers.
    > write(fp, &num, sizeof(num));
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Dec 2017
    Posts
    1,626
    You need to compile with full warnings and fix all of them. Here are a couple of mistakes:
    Code:
        if (error = mkfifo(fifo , 0666) == -1) {
    ...
            if (error = pthread_create(calc_arr[i], NULL, find_prime_number, &thread[i]) != 0) {
    ...
            if (error = pthread_join(*calc_arr[i], NULL) != 0) {
    (Hint: you need an extra pair of parens in there.)

    find_prime_number is a total mess. It makes no sense. Presumably you mean to protect num with the mutex, but you access (and modify) it without any protection.

    In memorizer, why are you printing the received integer as a string?

    BTW, the limit for looking for prime factors isn't num/2 but sqrt(num). And you seem to be checking num (presuamably meaning mynum) against n over and over even though it's not possible for it to be > n due to the loop condition.
    Last edited by john.c; 07-03-2021 at 07:36 AM.
    A little inaccuracy saves tons of explanation. - H.H. Munro

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Simple multithreading problem
    By praised in forum C Programming
    Replies: 11
    Last Post: 08-14-2011, 01:03 PM
  2. Pipe problem, popen(), pipe().
    By apacz in forum C Programming
    Replies: 7
    Last Post: 06-08-2006, 12:55 AM
  3. Multithreading problem
    By Bacardi34 in forum C Programming
    Replies: 5
    Last Post: 09-02-2004, 02:26 PM
  4. Multithreading & Problem :: MFC
    By kuphryn in forum Windows Programming
    Replies: 1
    Last Post: 06-06-2002, 12:33 PM
  5. Interesting Multithreading Problem :: MFC
    By kuphryn in forum Windows Programming
    Replies: 4
    Last Post: 04-22-2002, 02:28 PM

Tags for this Thread