Thread: Pthread Circular buffer not looping, need help!

  1. #16
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,661
    > The main problem is that I'm getting: error: request for member `in` in something not a structure or union.

    Do you?
    Code:
    $ gcc -g -Wall -Wextra foo.c -pthread
    foo.c:129:6: warning: return type of ‘main’ is not ‘int’ [-Wmain]
     void main(int argc, char *argv[]) // to avoid segmentation fault
          ^
    foo.c: In function ‘main’:
    foo.c:181:5: warning: null argument where non-null required (argument 1) [-Wnonnull]
         pthread_mutex_destroy(NULL);
         ^
    foo.c:129:15: warning: unused parameter ‘argc’ [-Wunused-parameter]
     void main(int argc, char *argv[]) // to avoid segmentation fault
                   ^
    Yeah, calling mutex_destroy with NULL is going to be bad.

    But it has nothing to do with in and struct.
    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.

  2. #17
    Registered User
    Join Date
    May 2020
    Posts
    29
    I temporarily disabled mutex destroy, but the program still crashes after lift-3. I want it to return to lift-1 and keep going until the end of the text file.
    Last edited by Redsam121; 05-14-2020 at 05:55 AM.

  3. #18
    Registered User
    Join Date
    May 2020
    Posts
    29
    I solved the in and struct problems, they were printing properly but the crashing was causing the output file to blank. So they're fine.

    I don't know if I arranged the pthread_join()s correctly or if I did the wait and signal conditions right either.

  4. #19
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,661
    Only that each of your cond variables needs a corresponding mutex.

    Oh and declare main properly please.

    Apparently, fprintf is thread-safe (ish)
    c - Is fprintf() thread safe? - Stack Overflow

    You might still end up with interleaved lines, in which case you might want to sprintf to a large buffer, then fprintf the whole thing in one go.
    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.

  5. #20
    Registered User
    Join Date
    May 2020
    Posts
    29
    So for the sprintf, it'd be something like sprintf(A[i].line, "New Lift Request from Floor %d to Floor %d \n", A[in_i].from, A[in_i].to); and then fprintf("%s", A[i].line);? Is my assumption correct? I also recently changed my assert to perror for better error detection which stopped the crashes.
    Last edited by Redsam121; 05-14-2020 at 08:16 PM.

  6. #21
    Registered User
    Join Date
    May 2020
    Posts
    29
    My latest update of the code. I've changed the error checking parameters and added an error check for argc. I've change the fprintfs to singluars, but haven't implemented sprintf. I've also changed the while statement in request_t. I don't get what you by mutex with cond variables as I already have a mutex lock for them.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <pthread.h> 
    #include <unistd.h>
    #include <assert.h>
    #include "list.h"
    pthread_cond_t full, empty; //declare thread conditions
    pthread_mutex_t lock; //declare mutex to lock functions
    int counter; //number of requests made
    static int size; //buffer size (must only change once at beginning)
    static int sec; //Time in seconds, needed for lift to execute operation
    int length; //Number of values in buffer
    unsigned int in_i, out_i; //tail and head of buffer
    buffer_t *A; //the circular queue, is initialized in main due to size declared by user input
    write_t write1; //Used for printing the information from buffer.
    void *lift(void *vargp) //Consumer
    {
        pthread_mutex_lock(&lock);
        while (length <= 0)
        {
            printf("Waiting for request\n");
            pthread_cond_wait(&empty, &lock); //buffer is empty
        }
        sleep(sec); //Simulate time of elevator movement
        printf("lifting information\n");
        //gather information to print
        if (write1.p == 0) //only for when system begins (Assign Previous to the first record `from` value)
        {
            write1.p = A[out_i].from;
        }
        write1.rf = A[out_i].from;
        write1.rt = A[out_i].to;
        write1.m = (write1.p - A[out_i].from) + (A[out_i].to - A[out_i].from);
        write1.c = A[out_i].to;
        int id = ((share_t*)vargp)->id;
        //Now write the information
        write1.total_r[id] = write1.total_r[id] + 1;
        write1.total_m[id] = write1.total_m[id] + 1;
        fprintf(((share_t*)vargp)->out, "Lift-%d Operation\n"
        "Previous position: Floor %d\n"
        "Request: Floor %d to Floor %d\n"
        "Detail operations:\n"
        "    Go from Floor %d to Floor %d\n"
        "    Go from Floor %d to Floor %d\n"
        "    #movement for this request: %d\n"
        "    #request: %d\n"
        "    Total #movement: %d\n"
        "Current Position: Floor %d\n"
        "\n"
        "----------------------------\n", id, write1.p, write1.rf, write1.rt, write1.p, write1.rf, 
            write1.rf, write1.rt, write1.m, write1.total_r[id],
            write1.total_m[id], write1.c);
        write1.p = write1.c; //for next statement
        length--;
        out_i = (out_i + 1) % size;
        if (length == size-1)
        {
            printf("Signalling Request\n");
            pthread_cond_signal(&full);
        }
        pthread_mutex_unlock(&lock);
        return NULL;
    }
    void *request_t(void *vargp) //producer
    {
        FILE *in = ((share_t*)vargp)->in;//Needed for feof() due not accepting structs or union
        if (in == NULL) //Check if files exists before we can use them 
        {
            perror("Invalid files\n");
            exit(EXIT_FAILURE);
        }
        pthread_mutex_lock(&lock); //Now only request can operate (mutual exclusion)
        //read the input line by line and into the buffer
        do{
            while (length >= size) //buffer is full
            {
                printf("Waiting for lift\n");
                pthread_cond_wait(&full, &lock);
            }
            printf("Requesting information\n");
            fscanf(((share_t*)vargp)->in, "%d %d\n", &A[in_i].from, &A[in_i].to); //Stores values in buffer
            //If requests go out of bounds, they will be automatically registered as either
            //maximum level or minimum level
            if (A[in_i].to > 20)
            {
                A[in_i].to = 20;
            }
            else if (A[in_i].to < 1)
            {
                A[in_i].to = 1;
            }
            else if (A[in_i].from > 20)
            {
                A[in_i].from = 20;
            }
            else if (A[in_i].from < 1)
            {
                A[in_i].from = 1;
            }
            //Print buffer information to sim_output
            fprintf(((share_t*)vargp)->out, "New Lift Request from Floor %d to Floor %d \n" 
                    "Request No %d \n" 
                    "----------------------------\n", A[in_i].from, A[in_i].to, counter++);
            length++;
            in_i = (in_i + 1) % size;
            if (length == 1) //is causing system to crash.
            {
                printf("Signalling Lift\n");
                pthread_cond_signal(&empty);
            }
        } while (!feof(in));
        pthread_mutex_unlock(&lock);
        pthread_exit(NULL);
    }
    int main(int argc, char *argv[]) // to avoid segmentation fault
    {
        if (argc != 3)
        {
            perror("Insufficient arguements\n");
            exit(EXIT_FAILURE);
        }
        //establish values for use (convert argv to int values)
        counter = 1;
        share_t share[4]; //Allow multiple arguments in pthread
        in_i = 0, out_i = 0, length = 0; //All slots are empty at first
        long arg = strtol(argv[1], NULL, 10);
        size = arg;
        if (size <= 1)
        {
            perror("buffer size too small\n");
            exit(EXIT_FAILURE);
        }
        //initialize Buffer as empty array.
        A = calloc(size, sizeof *A);
        arg = strtol(argv[2], NULL, 10);
        sec = arg; //time required by each lift to serve a request
        if (sec <= 0)
        {
            perror("insufficient time\n");
            exit(EXIT_FAILURE);
        }
        FILE *out_t = fopen("sim_output.txt", "w");
        FILE *in_t = fopen("sim_input.txt", "r");
        if (in_t == NULL && out_t == NULL) //Check if files exists before we can use them
        {
            perror("Invalid files\n");
            exit(EXIT_FAILURE);
        }
        for (int i = 0; i < 4; i++)//Load files into share for transporting.
        {
            share[i].in = in_t;
            share[i].out = out_t;
            share[i].id = i;
        }
        //Create threads
        printf("Creating threads\n");
        pthread_t Lift_R, lift_1, lift_2, lift_3;
        pthread_cond_init(&full, NULL);
        pthread_cond_init(&empty, NULL);
        pthread_mutex_init(&lock, NULL);
        int LR = pthread_create(&Lift_R, NULL, request_t, &share[0]);
        if (LR != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l1 = pthread_create(&lift_1, NULL, lift, &share[1]);
        if (l1 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l2 = pthread_create(&lift_2, NULL, lift, &share[2]);
        if (l2 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l3 = pthread_create(&lift_3, NULL, lift, &share[3]);
        if (l3 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        pthread_join(Lift_R, NULL);
        pthread_join(lift_1, NULL);
        pthread_join(lift_2, NULL);
        pthread_join(lift_3, NULL);
        //pthread_mutex_destroy(NULL);
        int sum_r = write1.total_r[1] + write1.total_r[2] + write1.total_r[3],
            sum_t = write1.total_m[1] + write1.total_m[2] + write1.total_m[3];
        fprintf(out_t, "\n"
        "Total number of requests %d \n"
        "Total number of movememts %d \n", sum_r, sum_t);
        fclose(in_t);
        fclose(out_t);
        printf("finished\n");
        return 0;
    }
    Last edited by Redsam121; 05-15-2020 at 12:05 AM.

  7. #22
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,661
    > I don't get what you by mutex with cond variables as I already have a mutex lock for them.
    I mean you should have ONE mutex for each cond
    Code:
    pthread_cond_t full, empty; //declare thread conditions
    pthread_mutex_t lock_full, lock_empty; //declare mutex to lock functions
    Each cond has it's own mutex.

    The other problem would seem to be your lifts only process one move and then exit.
    Code:
    $ gcc -g -Wall -Wextra foo.c -pthread
    $ gdb -q ./a.out
    Reading symbols from ./a.out...done.
    (gdb) run 3 1
    Starting program: /home/sc/Documents/a.out 3 1
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
    Creating threads
    [New Thread 0x7ffff77ef700 (LWP 6902)]
    Requesting information
    Signalling Lift
    Requesting information
    Requesting information
    Waiting for lift
    [New Thread 0x7ffff6fee700 (LWP 6903)]
    [New Thread 0x7ffff67ed700 (LWP 6904)]
    [New Thread 0x7ffff5fec700 (LWP 6905)]
    lifting information
    Signalling Request
    Requesting information
    Waiting for lift
    [Thread 0x7ffff6fee700 (LWP 6903) exited]
    lifting information
    Signalling Request
    Requesting information
    Waiting for lift
    [Thread 0x7ffff5fec700 (LWP 6905) exited]
    lifting information
    Signalling Request
    Requesting information
    Waiting for lift
    [Thread 0x7ffff67ed700 (LWP 6904) exited]
    ^C
    Thread 1 "a.out" received signal SIGINT, Interrupt.
    0x00007ffff7bc298d in pthread_join (threadid=140737345681152, thread_return=0x0) at pthread_join.c:90
    90	pthread_join.c: No such file or directory.
    (gdb) info thread
      Id   Target Id         Frame 
    * 1    Thread 0x7ffff7fc1700 (LWP 6898) "a.out" 0x00007ffff7bc298d in pthread_join (threadid=140737345681152, thread_return=0x0) at pthread_join.c:90
      2    Thread 0x7ffff77ef700 (LWP 6902) "a.out" pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
    (gdb) thread apply all bt
    
    Thread 2 (Thread 0x7ffff77ef700 (LWP 6902)):
    #0  pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
    #1  0x0000000000400f3c in request_t (vargp=0x7fffffffdde0) at foo.c:99
    #2  0x00007ffff7bc16ba in start_thread (arg=0x7ffff77ef700) at pthread_create.c:333
    #3  0x00007ffff78f741d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
    
    Thread 1 (Thread 0x7ffff7fc1700 (LWP 6898)):
    #0  0x00007ffff7bc298d in pthread_join (threadid=140737345681152, thread_return=0x0) at pthread_join.c:90
    #1  0x00000000004014c8 in main (argc=3, argv=0x7fffffffdf38) at foo.c:206
    (gdb) print counter
    $1 = 7
    (gdb) print length
    $2 = 3
    (gdb) print write1
    $3 = {p = 7, rf = 6, rt = 7, total_m = {0, 1, 1}, c = 7, m = 16, total_r = {0, 1, 1}}
    Your three lift threads perform a single "lifting information" and then exit.

    Meanwhile, your request thread is stuck "waiting for lift" for something to empty the queue.
    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.

  8. #23
    Registered User
    Join Date
    May 2020
    Posts
    29
    But previously you said
    >Another problem is both threads are writing to your output file in an unsafe way.

    >request_t has a different mutex to lift.

    So I removed one of the mutexes.

    Also the three lifts performing one lifting information has been the problem the whole time.

  9. #24
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,661
    Yeah that was my bad - I mis-read the mutex as being for the output, not the conds

    share_t *share = vargp;
    at the start of each function means you can get rid of all ((share_t*)vargp)-> things in favour of share->
    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.

  10. #25
    Registered User
    Join Date
    May 2020
    Posts
    29
    All is forgiven, mistakes happen. So how do I stop the three lift threads from exiting until request thread is done?

  11. #26
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,661
    Well each lift basically needs a while ( true ) loop around the body of the code.

    Then you need some mechanism for request_t to tell each lift that everything is processed when it exits the loop
    > while (!feof(in));

    Speaking of which, you should be checking that fscanf returns EOF, not checking whether feof() returns true.
    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.

  12. #27
    Registered User
    Join Date
    May 2020
    Posts
    29
    I tried using while (fscanf(((share_t*)vargp)->in, "%d %d\n", &A[in].from, &A[in].to) != EOF) before but that caused the lift write the wrong values to output. A value would be print before the request was made.

    Each lift should go one at a time (FIFO).

    I've implemented the while(true) and now it goes through the entire thing but only uses lift-1. Just need to design that mechanism for request_t.
    Last edited by Redsam121; 05-15-2020 at 01:31 AM.

  13. #28
    Registered User
    Join Date
    May 2020
    Posts
    29
    I came up with a solution for (fscanf(((share_t*)vargp)->in, "%d %d\n", &A[in].from, &A[in].to) != EOF). I put it into an if statement and when it reaches the file, it'll break the while loop.

  14. #29
    Registered User
    Join Date
    May 2020
    Posts
    29
    Here is the latest update so far. It now goes through the file to the end, but once it's finished it becomes stuck in "Waiting for request" and only uses lift-1. Each lift should be used one at a time in a cycle (lift-1 to lift-2 to lift-3 to lift-1).

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <pthread.h> 
    #include <unistd.h>
    #include <assert.h>
    #include "list.h"
    pthread_cond_t full, empty; //declare thread conditions
    pthread_mutex_t lock_full, lock_empty; //declare mutex to lock functions
    int counter; //number of requests made
    static int size; //buffer size (must only change once at beginning)
    static int sec; //Time in seconds, needed for lift to execute operation
    int length; //Number of values in buffer
    unsigned int in_i, out_i; //tail and head of buffer
    buffer_t *A; //the circular queue, is initialized in main due to size declared by user input
    write_t write1; //Used for printing the information from buffer.
    void *lift(void *vargp) //Consumer
    {
        share_t* share = vargp;
        pthread_mutex_lock(&lock_empty);
        while (true)
        {
            printf("%d\n", length);
            while (length <= 0)
            {
                printf("Waiting for request\n");
                pthread_cond_wait(&empty, &lock_empty); //buffer is empty
            }
            sleep(sec); //Simulate time of elevator movement
            printf("lifting information\n");
            //gather information to print
            if (write1.p == 0) //only for when system begins (Assign Previous to the first record `from` value)
            {
                write1.p = A[out_i].from;
            }
            write1.rf = A[out_i].from;
            write1.rt = A[out_i].to;
            write1.m = (write1.p - A[out_i].from) + (A[out_i].to - A[out_i].from);
            write1.c = A[out_i].to;
            int id = share->id;
            //Now write the information
            write1.total_r[id] = write1.total_r[id] + 1;
            write1.total_m[id] = write1.total_m[id] + write1.m;
            fprintf(share->out, "Lift-%d Operation\n"
                "Previous position: Floor %d\n"
                "Request: Floor %d to Floor %d\n"
                "Detail operations:\n"
                "    Go from Floor %d to Floor %d\n"
                "    Go from Floor %d to Floor %d\n"
                "    #movement for this request: %d\n"
                "    #request: %d\n"
                "    Total #movement: %d\n"
                "Current Position: Floor %d\n"
                "\n"
                "----------------------------\n", id, write1.p, write1.rf, write1.rt, write1.p, write1.rf,
                write1.rf, write1.rt, write1.m, write1.total_r[id],
                write1.total_m[id], write1.c);
            write1.p = write1.c; //for next statement
            length--;
            out_i = (out_i + 1) % size;
            if (length == size - 1)
            {
                printf("Signalling Request\n");
                pthread_cond_signal(&full);
            }
        }
        pthread_mutex_unlock(&lock_empty);
    }
    void *request_t(void *vargp) //producer
    {
        share_t * share = vargp;
        pthread_mutex_lock(&lock_full); //Now only request can operate (mutual exclusion)
        //read the input line by line and into the buffer
        while (1){
            while (length >= size) //buffer is full
            {
                printf("Waiting for lift\n");
                pthread_cond_wait(&full, &lock_full);
            }
            printf("Requesting information\n");
            if (fscanf(share->in, "%d %d\n", &A[in_i].from, &A[in_i].to) == EOF)
            {
                break;
            }
             //Stores values in buffer
            //If requests go out of bounds, they will be automatically registered as either
            //maximum level or minimum level
            if (A[in_i].to > 20)
            {
                A[in_i].to = 20;
            }
            else if (A[in_i].to < 1)
            {
                A[in_i].to = 1;
            }
            else if (A[in_i].from > 20)
            {
                A[in_i].from = 20;
            }
            else if (A[in_i].from < 1)
            {
                A[in_i].from = 1;
            }
            //Print buffer information to sim_output
            fprintf(share->out, "New Lift Request from Floor %d to Floor %d \n"
                    "Request No %d \n" 
                    "----------------------------\n", A[in_i].from, A[in_i].to, counter++);
            length++;
            in_i = (in_i + 1) % size;
            if (length == 1) //is causing system to crash.
            {
                printf("Signalling Lift\n");
                pthread_cond_signal(&empty);
            }
        }
        pthread_mutex_unlock(&lock_full);
        pthread_exit(NULL);
    }
    int main(int argc, char * argv[]) // to avoid segmentation fault
    {
        if (argc != 3)
        {
            perror("Insufficient arguements\n");
            exit(EXIT_FAILURE);
        }
        //establish values for use (convert argv to int values)
        counter = 1;
        share_t share[4]; //Allow multiple arguments in pthread
        in_i = 0, out_i = 0, length = 0; //All slots are empty at first
        long arg = strtol(argv[1], NULL, 10);
        size = arg;
        if (size <= 1)
        {
            perror("buffer size too small\n");
            exit(EXIT_FAILURE);
        }
        //initialize Buffer as empty array.
        A = calloc(size, sizeof *A);
        arg = strtol(argv[2], NULL, 10);
        sec = arg; //time required by each lift to serve a request
        if (sec <= 0)
        {
            perror("insufficient time\n");
            exit(EXIT_FAILURE);
        }
        FILE *out_t = fopen("sim_output.txt", "w");
        FILE *in_t = fopen("sim_input.txt", "r");
        if (in_t == NULL && out_t == NULL) //Check if files exists before we can use them
        {
            perror("Invalid files\n");
            exit(EXIT_FAILURE);
        }
        for (int i = 0; i < 4; i++)//Load files into share for transporting.
        {
            share[i].in = in_t;
            share[i].out = out_t;
            share[i].id = i;
        }
        //Create threads
        printf("Creating threads\n");
        pthread_t Lift_R, lift_1, lift_2, lift_3;
        pthread_cond_init(&full, NULL);
        pthread_cond_init(&empty, NULL);
        pthread_mutex_init(&lock_full, NULL);
        pthread_mutex_init(&lock_empty, NULL);
    
        int LR = pthread_create(&Lift_R, NULL, request_t, &share[0]);
        if (LR != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l1 = pthread_create(&lift_1, NULL, lift, &share[1]);
        if (l1 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l2 = pthread_create(&lift_2, NULL, lift, &share[2]);
        if (l2 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        int l3 = pthread_create(&lift_3, NULL, lift, &share[3]);
        if (l3 != 0)
        {
            perror("error");
            exit(EXIT_FAILURE);
        }
        pthread_join(lift_1, NULL);
        pthread_join(lift_2, NULL);
        pthread_join(lift_3, NULL);
        pthread_join(Lift_R, NULL);
        int sum_r = write1.total_r[1] + write1.total_r[2] + write1.total_r[3],
            sum_t = write1.total_m[1] + write1.total_m[2] + write1.total_m[3];
        fprintf(out_t, "\n"
        "Total number of requests %d \n"
        "Total number of movememts %d \n", sum_r, sum_t);
        fclose(in_t);
        fclose(out_t);
        printf("finished\n");
        return 0;
    }
    Last edited by Redsam121; 05-15-2020 at 10:34 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Circular buffer - console crashing
    By High Voltage in forum C++ Programming
    Replies: 10
    Last Post: 02-26-2017, 08:59 AM
  2. Circular Buffer using Virtual Memory
    By SMurf in forum Windows Programming
    Replies: 5
    Last Post: 03-26-2013, 07:05 PM
  3. Replies: 3
    Last Post: 06-27-2011, 11:25 AM
  4. Circular buffer - logic difficulty
    By erasm in forum C Programming
    Replies: 2
    Last Post: 10-05-2009, 01:08 PM
  5. Circular Buffer
    By parisframe in forum C Programming
    Replies: 11
    Last Post: 09-16-2009, 11:05 PM

Tags for this Thread