Why is output of my program always different when using threads?

Question

My program's desired functionality:
Using the command line user inputs N and M. N is the number of new threads that will be created and M is the number of how much every thread increments the global variable A.

This is my code:

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

static int A = 0;

void *Thread(void* x){
        int i;
        int n = *((int*)x);
        for (i = 0; i<n; i++){
                A++;
        }
}

int main(int argc, char* argv[]){
        int i;
        int N = atoi(argv[1]);
        int M = atoi(argv[2]);

        pthread_t *thread = (pthread_t *) malloc(sizeof(pthread_t)*N);

        if(!thread){
                printf("No memory\n");
                exit(2);
        }

        for (i = 0; i< N; i++){
                if (pthread_create(&thread[i], NULL, Thread, &M)){
                        printf("Not able to create a thread\n");
                        exit(1);
                }
        }

        for(i = 0; i< N; i++)
                pthread_join(thread[i], NULL);

        printf("A = %d\n", A);

        return 0;
}

The problem is that every time I run it there's a different output. Screenshot of my terminal when i run the program multiple times in a row

Concurrent read/write access to a variable (`A` here) should be protect . For example by using a mutex. — alk, Nov 12 '16 at 16:11
OT: This cast `(pthread_t *)` is useless in C. Or should you be using a C++ compiler? — alk, Nov 12 '16 at 16:12

maki · Accepted Answer · 2016-11-13T08:01:08.583

The problem is that you are creating multiple threads that in parallel are trying to modify your static A global variable at the same time, without any kind of protection.

That means that depending on the scheduling of the threads, the changes on your global variable will not be atomic, producing this effect.

You can solve this with a mutex, declare it with:

pthread_mutex_t mutex;

And initialise / release it with pthread_mutex_init and pthread_mutex_destroy.

Inside of the thread, before doing the change protect the resource to change with pthread_mutex_lock, and release it with pthread_mutex_unlock. So the code will be changed like this:

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

static int A = 0;
pthread_mutex_t mutex;


void *Thread(void* x){
        int i;
        int n = *((int*)x);

        pthread_mutex_lock(&mutex);
        A += n;
        pthread_mutex_unlock(&mutex);
}

int main(int argc, char* argv[]){
        int i;
        int N = atoi(argv[1]);
        int M = atoi(argv[2]);

        pthread_mutex_init(&mutex, NULL);
        pthread_t *thread = (pthread_t *) malloc(sizeof(pthread_t)*N);

        if(!thread){
                printf("No memory\n");
                exit(2);
        }

        for (i = 0; i< N; i++){
                if (pthread_create(&thread[i], NULL, Thread, &M)){
                        printf("Not able to create a thread\n");
                        exit(1);
                }
        }

        for(i = 0; i< N; i++)
                pthread_join(thread[i], NULL);

        printf("A = %d\n", A);

        pthread_mutex_destroy(&mutex);

        return 0;
}

*At least* the calls to `*lock` and `*unlock` should be checked for failure. — alk, Nov 12 '16 at 16:19
One suggestion: In Thread() why do we need for loop? We can do A+=n. — MayurK, Nov 12 '16 at 16:36
Not needed at all, I guess he was just just playing with the code as an example. — maki, Nov 12 '16 at 16:37
If you're going to initialize the `pthread_mutex`, use the officially sanctioned `PTHREAD_MUTEX_INITIALIZER` in place of the unsanctioned and not reliably portable `0`. — Jonathan Leffler, Nov 12 '16 at 16:49
For completeness: If initilaising a mutex via `pthread_mutex_init` ths mutex passed does not need to be initialised at all before. — alk, Nov 13 '16 at 07:43
Thanks for the feedback, totally correct, just changed it. The initial assignation to the mutex was a typo. Also changed the for loop by MayurK suggestion. — maki, Nov 13 '16 at 08:02

Why is output of my program always different when using threads?

1 Answers1