2

So I have this lab problem where I need to use 2 threads with mutex and cond var to print the title pattern. The pattern is stored in an array of char, and there's a global keeping track of which index to print. In my code I created my logic was this

thread A created first, so thread A runs first and locks the mutex so only A should be running and B should be asleep

A enters while loop and prints A0, increments index, then using pthread_cond_wait(), unlocks mutex and sleeps and waits for signal cord

B runs and prints next sequence then sends signal and Thread A wakes up and pthread_cond_wait() upon return relocks mutex so Thread A has control back and B sleeps

This continues and prints the pattern sequence. But when I run it I just get B0B1B2B3B4B5B6B7B8B9. I don't know where I went wrong and would like insight, thanks

#include <stdio.h>       
#include <stdlib.h>      
#include <unistd.h>      
#include <pthread.h>     

int string_index = 0;
char string_to_print[10] = "0123456789";
pthread_mutex_t lock;
pthread_cond_t cond;

void *funcA(void *arg)
{   
    pthread_mutex_lock(&lock); //lock mutex

    while (1) 
    {   
        printf("A%c", string_to_print[string_index]);
        string_index++;
        pthread_cond_wait(&cond, &lock); //sleep and wait for signal
    }
}

void *funcB(void *arg)
{   
    while (1) 
    {   
        pthread_mutex_lock(&lock); //lock mutex
        printf("B%c", string_to_print[string_index]);
        string_index++;

        if(string_index > 9)
        {
            string_index = 0;
            printf("\n");
        }
        pthread_mutex_unlock(&lock); //unlock mutex

        pthread_cond_signal(&cond); //send cond var signal
    }
}

int main(void)
{   
    //initialize mutex lock
    if (pthread_mutex_init(&lock, NULL) != 0) 
    { 
        printf("\n mutex init has failed\n"); 
        return 1; 
    } 

    //initialize conditional variable
    if (pthread_cond_init(&cond, NULL) != 0)
    {
        printf("cond var init failed");
        return 1;
    }

    pthread_t A,B;

    //create thread A
    if (pthread_create(&A, NULL, &funcA, NULL) != 0) 
        {
            printf("Error creating thread");
            exit(-1);
        }
    
    //create thread B
    if (pthread_create(&B, NULL, &funcB, NULL) != 0) 
        {
            printf("Error creating thread");
            exit(-1);
        }

    sleep(20);   /* sleep 20 secs to allow time for the threads to run */
                    /* before terminating them with pthread_cancel()      */

    pthread_cancel(A);
    pthread_cancel(B);
    pthread_exit(NULL);
    pthread_mutex_destroy(&lock);
    pthread_cond_destroy(&cond);
}
Improve this question
6
  • 2
    You do not have anything causing funcB to wait for funcA to do its thing. When funcB calls pthread_cond_signal, it immediately loops and calls pthread_mutex_lock. It takes time for the system to send the signal to the other thread and for that thread to wake up. Before that happens, funcB has taken the lock again, and then funcA cannot get the lock and goes back to sleep, waiting. You need both threads to signal the other and to wait for a signal from the other. Commented Feb 18 at 21:49
  • 2
    You are not using your condition variable idiomatically. CVs are meant to support waiting, if necessary, for some testable condition to be satisfied. Such as a computable representation of "it's my turn". The wait must (i) be gated by a test of whether the condition is presently satisfied, and (ii) be resumed in the event that the condition is not satisfied after the wait returns. There are lots of examples here on SO and elsewhere. Commented Feb 18 at 22:31
  • 1
    "thread A created first, so thread A runs first and locks the mutex so only A should be running and B should be asleep" -- this is not a safe assumption, but proper CV usage would make that moot. Commented Feb 18 at 22:33
  • To complement on CVs, waits can have spurious wakeups, which means your thread will stop waiting even if the other thread never signaled. You NEED to use them idiomatically in a loop or it will just not work. Commented Feb 18 at 22:41
  • Please note that a previous version of my answer had a bug. It had a race condition that could cause one thread to sleep forever, if the other thread terminates without signalling the thread to wake up. I believe I have fixed it now. Commented Feb 19 at 21:03

2 Answers 2

Reset to default
1

thread A created first, so thread A runs first

The fact that thread A was created first does not necessarily mean that thread A will be the first thread to lock the mutex. It is possible for thread B to be faster.

If you want to ensure that thread A does something before thread B, you must make thread B wait for thread A to be finished doing that thing (for example using pthread_cond_wait).

B runs and prints next sequence then sends signal and Thread A wakes up

Thread A will not be able to awaken until the condition varible is signaled AND the mutex is unlocked.

Thread B locks the mutex again a very short time after unlocking it. Mutexes are not guaranteed to be fair, and are often implemented in such a way that they favor performance over fairness. Therefore, you cannot rely on Thread A having an opportunity to acquire the mutex.

If you want to ensure that thread A gets an opportunity to wake up, you must make thread B wait for thread A (for example using pthread_cond_wait).

In order to ensure that the threads take turns in printing one character, I suggest that you add a variable named turn to your program which indicates which thread's turn it is to print the next character. This variable should be protected by the mutex. You should also add a second condition variable, so that each thread has its own condition variable to wait on.

Another issue in your code is that your cleanup code in the main thread is not being executed, because you have a pthread_exit function call beforehand. I am not sure if this is intentional.

Your cleanup code should have calls to pthread_join even if those threads have been canceled with pthread_cancel. Otherwise, you will have a resource leak. Another reason to use pthread_join is that you should not attempt to destroy the mutex or the condition variable(s) while one of the threads is still running.

Even if you wait for the threads to complete using pthread_join, it is possible that one of the threads will terminate while leaving the mutex in a locked state. See this answer to another question for further information. This will cause the function call to pthread_mutex_destroy to invoke undefined behavior. This is because according to POSIX, calling pthread_mutex_destroy on a locked mutex invokes undefined behavior.

In order to prevent this undefined behavior from being possibly invoked, it would probably be better to use a different mechanism for stopping the threads than pthread_cancel, because it is difficult to use pthread_cancel in such a way that the mutex gets unlocked before the thread terminates. Therefore, instead of using pthread_cancel, I suggest that the main thread sets a variable to tell the other threads to stop. I do not recommend protecting this variable with the mutex, because the main thread may not be able to acquire the mutex in a timely manner, since the other threads are constantly contending for the mutex. Instead, I recommend an atomic variable.

After doing the above, the code will look like this:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>
#include <stdbool.h>
#include <stdatomic.h>

int turn = 0;
atomic_bool terminate = false;
int string_index = 0;
char string_to_print[10] = "0123456789";
pthread_mutex_t lock;
pthread_cond_t condA, condB;

void *funcA(void *arg)
{
    pthread_mutex_lock(&lock); // lock mutex

    while (1) 
    {
        if (terminate)
        {
            pthread_mutex_unlock(&lock);
            pthread_cond_signal(&condB);
            pthread_exit(NULL);
        }

        if (turn == 0)
        {
            printf("A%c", string_to_print[string_index]);
            string_index++;
            turn = 1;
            pthread_cond_signal(&condB); //send cond var signal
        }

        pthread_cond_wait(&condA, &lock); //sleep and wait for signal
    }
}

void *funcB(void *arg)
{
    pthread_mutex_lock(&lock); //lock mutex

    while (1) 
    {
        if (terminate)
        {
            pthread_mutex_unlock(&lock);
            pthread_cond_signal(&condA);
            pthread_exit(NULL);
        }

        if (turn == 1)
        {
            printf("B%c", string_to_print[string_index]);
            string_index++;
            if(string_index > 9)
            {
                string_index = 0;
                printf("\n");
            }
            turn = 0;
            pthread_cond_signal(&condA); //send cond var signal
        }

        pthread_cond_wait(&condB, &lock); //sleep and wait for signal
    }
}

int main(void)
{
    //initialize mutex lock
    if (pthread_mutex_init(&lock, NULL) != 0) 
    {
        printf("\n mutex init has failed\n"); 
        return 1; 
    }

    //initialize conditional variables
    if (pthread_cond_init(&condA, NULL) != 0)
    {
        printf("cond var init failed");
        return 1;
    }
    if (pthread_cond_init(&condB, NULL) != 0)
    {
        printf("cond var init failed");
        return 1;
    }

    pthread_t A,B;

    //create thread A
    if (pthread_create(&A, NULL, &funcA, NULL) != 0) 
    {
        printf("Error creating thread");
        exit(-1);
    }
    
    //create thread B
    if (pthread_create(&B, NULL, &funcB, NULL) != 0) 
    {
        printf("Error creating thread");
        exit(-1);
    }

    sleep(20);

    // set this atomic variable instead of using pthread_cancel
    terminate = true;
    //pthread_cancel(A);
    //pthread_cancel(B);

    pthread_join(B, NULL);
    pthread_join(A, NULL);
    pthread_mutex_destroy(&lock);
    pthread_cond_destroy(&condB);
    pthread_cond_destroy(&condA);
}

This code has the following output:

A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
[...]

As you can see, the threads are now reliably taking turns in printing characters.

It may be worth noting that if you are sure that you have at least two hardware threads, you can also use a single atomic variable for thread synchronization, instead of mutexes and condition variables. That way, the threads will busy-wait instead of falling asleep:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>
#include <stdbool.h>
#include <stdatomic.h>

atomic_int turn = 0;
int string_index = 0;
char string_to_print[10] = "0123456789";

void *funcA(void *arg)
{
    while (1) 
    {
        switch ( turn )
        {
            case 0:
                printf("A%c", string_to_print[string_index]);
                string_index++;
                turn = 1;
                break;
            case 1:
                continue;
            default:
                pthread_exit( NULL );
        }
    }
}

void *funcB(void *arg)
{
    while (1) 
    {
        switch ( turn )
        {
            case 0:
                continue;
            case 1:
                printf("B%c", string_to_print[string_index]);
                string_index++;
                if(string_index > 9)
                {
                    string_index = 0;
                    printf("\n");
                }
                turn = 0;
                break;
            default:
                pthread_exit( NULL );
        }
    }
}

int main(void)
{
    pthread_t A,B;

    //create thread A
    if (pthread_create(&A, NULL, &funcA, NULL) != 0) 
    {
        printf("Error creating thread");
        exit(-1);
    }
    
    //create thread B
    if (pthread_create(&B, NULL, &funcB, NULL) != 0) 
    {
        printf("Error creating thread");
        exit(-1);
    }

    sleep(20);

    turn = 2;

    pthread_join(B, NULL);
    pthread_join(A, NULL);
}

This program has the same output as the other one.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

The answer by Andreas Wenzel clearly wanders off topic when it delves into using atomic variables rather than a mutex. The question clearly states:

So I have this lab problem where I need to use 2 threads with mutex and cond var to print the title pattern.

If the answer demonstrating the use of atomic variables is on topic then a solution in another programming language should be just as much on topic.

For example, the following Ada program using neither atomics nor mutexes and condition variables should be just as much on topic as the program Andreas Wenzel produced using atomic variable.

with Ada.Text_IO; use Ada.Text_IO;

procedure Rendezvous_sequencer is
   String_To_Print : String := "0123456789";

   task task_a is
      entry set_Idx (Idx : in Positive);
      entry stop;
   end task_a;

   task task_b is
      entry set_idx (Idx : in Positive);
   end task_b;

   task body task_a is
      Index : Positive;
   begin
      loop
         select
            accept stop;
            exit;
         else
            accept set_Idx (Idx : in Positive) do
               Index := Idx;
            end set_Idx;
         end select;
         Put ("A" & String_To_Print (Index));
         task_b.set_idx (Index + 1);
      end loop;
   end task_a;

   task body task_b is
      Index : Positive;
      Lines : Positive := 15;
   begin
      for loop_count in 1 .. Lines loop
         loop
            accept set_idx (Idx : in Positive) do
               Index := Idx;
            end set_idx;
            Put ("B" & String_To_Print (Index));
            if Index + 1 > String_To_Print'Last then
               New_Line;
               if loop_count < Lines then
                  task_a.set_Idx (String_To_Print'First);
               else
                  task_a.stop;
               end if;
               exit;
            else
               task_a.set_Idx (Index + 1);
            end if;
         end loop;
      end loop;
   end task_b;

begin
   task_a.set_Idx (String_To_Print'First);
end Rendezvous_sequencer;

This Ada example uses the Ada Rendezvous feature. The Rendezvous feature allows direct synchronous messaging between Ada tasks. Ada tasks are the active unit of concurrency in the Ada language and are often implemented as operating system threads. Rendezvous are implemented through declared task entries.

In this example the task named task_a has two entries and the task named task_b has one entry. Task entries are declared in the task specification and are implemented in the task body.

   task task_a is
      entry set_Idx (Idx : in Positive);
      entry stop;
   end task_a;

   task task_b is
      entry set_idx (Idx : in Positive);
   end task_b;

A task calling an entry in another task suspends until the called task accepts the entry. Similarly, the called task suspends at its accept statement until some other task calls the entry.

   task body task_a is
      Index : Positive;
   begin
      loop
         select
            accept stop;
            exit;
         else
            accept set_Idx (Idx : in Positive) do
               Index := Idx;
            end set_Idx;
         end select;
         Put ("A" & String_To_Print (Index));
         task_b.set_idx (Index + 1);
      end loop;
   end task_a;

The body of task_a must deal with having two entries and does so using a select clause.The select clause allows task_a to suspend until either of the two entries is called and then respond to that entry.

When the Stop entry for task_a is called the task responds by exiting its otherwise infinite loop, causing the task to complete. When the set_idx entry is called the task assigns the parameter passed to the entry call to a local parameter and then processes that parameter. Task_a outputs 'A' followed by the array element indexed by the value passed through the set_idx entry and then calls the task_b set_idx entry passing the value incremented by 1.

Task_b has only one entry and has additional behavior compared to task_a.

   task body task_b is
      Index : Positive;
      Lines : Positive := 15;
   begin
      for loop_count in 1 .. Lines loop
         loop
            accept set_idx (Idx : in Positive) do
               Index := Idx;
            end set_idx;
            Put ("B" & String_To_Print (Index));
            if Index + 1 > String_To_Print'Last then
               New_Line;
               if loop_count < Lines then
                  task_a.set_Idx (String_To_Print'First);
               else
                  task_a.stop;
               end if;
               exit;
            else
               task_a.set_Idx (Index + 1);
            end if;
         end loop;
      end loop;
   end task_b;

Task_b has a second local variable named Lines, which determines the number of times the output sequence is printed. The for loop in task_b iterates through the values 1 through Lines, after which task_b completes. Each iteration of the outer loop executes an iteration of an inner loop which accepts the set_idx entry and prints out the "B" followed by the array element indexed by the value received from the set_idx entry.

When the value received by the sed_idx entry equals the maximum index value for the String_To_Print string task_b commands the output to print a new line character. If the number of iterations of the outer loop is less than Lines task_b calls the task_a set_idx entry with the first array index value, unless the number of iterations equals Lines, in which case task_b calls the Stop entry for task_a. If the value task_b received is less than the maximum index value for String_To_Print then task_b calls the task_a set_idx entry passing the next index value in the array.

All this happens in a parameterless procedure named Rendezvous_sequencer, which is the program entry point, similar to main in C++. The begin reserved word starts the executable part of Rendezvous_sequencer, which contains only one statement. Rendezvous_sequencer calls the task_a set_idx entry with the first index value of the string String_To_Print.

Both tasks start automatically when the program reaches the begin reserved word in the Rendezvous_sequencer procedure. Both tasks remain suspended until some task calls one of their entries, thus controlling the sequencing of the output.

The output is:

A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
A0B1A2B3A4B5A6B7A8B9
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.