Amit Singh Tomar
Amit Singh Tomar

Reputation: 8610

Why this pthread code just hangs?

Following code snippet is written to print even number with one thread and odd number with other thread.

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

#define LIMIT 100

int counter;

pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER;

void* function(void *p)
{
    int expected = *(int *)p;
    while(expected < LIMIT) 
    {
        pthread_mutex_lock( &mutex1 );
        while (counter != expected);
            printf("%d\n", counter++);
        expected += 2;
        pthread_mutex_unlock( &mutex1 );
    };
    exit(0);
}

int main(int argc, char *argv[])
{
    pthread_t thread1, thread2;
    int counter = 0;
    int expected_0 = 0, expected_1 = 1;

    pthread_create(&thread1, NULL, function, (void *)&expected_0);
    pthread_create(&thread2, NULL, function, (void *)&expected_1);

    pthread_join(thread1, NULL);
    pthread_join(thread2, NULL);

    return 0;
}

But when execute it shows an output of 0 and hangs

amit@ubuntu:~$ gcc -pthread even_odd.c 
amit@ubuntu:~$ ./a.out 
0

Could any one please point me out what is going here?

Also would there be a better logic to accomplish this task?

Upvotes: 1

Views: 4500

Answers (2)

Sunil S
Sunil S

Reputation: 976

It is a classic case of mutex deadlock.

Let's see the details:

Here you created two threads sequentially, first one expecting counter to be 0 and the second one expecting counter to be 1.Then each one will increase counter by one and that make another thread's expected value.

Now it is not necessary that if you create thread thread1 before thread thread2 then thread thread1 will always execute before thread thread2. This totally depends on you OS scheduler and you have minimal control on it.

So when thread2 starts executing before thread1, it will acquire the mutex1 and will start waiting in while (counter != expected); statement. Now when thread1 starts excecuting, it will be blocked by mutex1 as it is already aquired by thread2. Now, there is no one left to update counter. This will cause the deadlock.

You should use following code as the thread function();

void* function(void *p)
{
    int expected = *(int *)p;
    while(expected < LIMIT)
    {
        pthread_mutex_lock( &mutex1 );
        if (counter == expected) {
            printf("%d\n", counter++);
            expected += 2;
        }
        pthread_mutex_unlock( &mutex1 );
    };
}

Here

  • I am locking mutex whenever I want to read or write the global variable counter. This prevents race condition.
  • I am releasing mutex without busy waiting inside locked mutex for the same variable. This prevents deadlock.

Upvotes: 1

Jonathan Wakely
Jonathan Wakely

Reputation: 171253

You have several problems here.

The main one is that when a thread should wait for counter to reach its expected value, it doesn't unlock the mutex to allow the other thread to proceed. This fixes that (add <stdbool.h> and compile as C99):

void* function(void *p)
{
    int expected = *(int *)p;
    while(expected < LIMIT)
    {
        bool my_turn = false;
        while (!my_turn)
        {
            pthread_mutex_lock( &mutex1 );
            my_turn = (counter == expected);
            pthread_mutex_unlock( &mutex1 );
        }
        pthread_mutex_lock( &mutex1 );
        printf("%d\n", counter++);
        pthread_mutex_unlock( &mutex1 );
        expected += 2;
    };

The code above locks the mutex to check the counter, then unlocks it again and loops if the counter hasn't reached the expected value, so the other thread can run. It then locks the mutex again to access counter again.

Another problem is calling exit(0) which causes both threads to exit as soon as one of them finishes. You probably want to just return from the function instead:

    return NULL;
}

Another way to write the loop would be:

        while (true)
        {
            pthread_mutex_lock( &mutex1 );
            if (counter == expected)
                break;
            pthread_mutex_unlock( &mutex1 );
        }
        printf("%d\n", counter++);
        pthread_mutex_unlock( &mutex1 );
        expected += 2;

This is slightly more complicated to understand, but avoids unlocking and re-locking the mutex when the counter has reached the expected value. Instead it breaks out of the loop leaving the mutex locked, prints the value, then unlocks the mutex.

Upvotes: 1

Related Questions