C, pthreads initialized in loop does not execute assigned function properly despite mutex

Question

I am having trouble debugging my C program where the goal is to create 5 threads and have each of them working on size-2 chunks of an array of length 10. The goal is to get the sum of that array. My actual program is a little less trivial than this as it takes dynamic array sizes and thread counts but I tried simplifying it to this simple problem and it still does not work.

ie.,

array = {1 2 3 4 5 6 7 8 9 10}

then thread1 works on array[0] and array [1]

and thread2 works on array[2] and array[3]

etc...

thread5 works on array[8] and array[9]

However when i run my code, I get weird results, even when using a mutex lock.

For example, this is one of my results when running this program.

Thread #1 adding 3 to 0 New sum: 3
Thread #1 adding 4 to 3 New sum: 7
Thread #2 adding 5 to 7 New sum: 12
Thread #2 adding 6 to 12        New sum: 18
Thread #3 adding 7 to 18        New sum: 25
Thread #3 adding 8 to 25        New sum: 33
Thread #4 adding 9 to 33        New sum: 42
Thread #4 adding 9 to 42        New sum: 51
Thread #4 adding 10 to 51       New sum: 61
Thread #4 adding 10 to 61       New sum: 71
Sum: 71

First of all, why are there no tabs before the "New sum" for the first 3 lines? (see my printf log in calculate_sum function). And more importantly, why is thread0 never executing it's job and why is thread 4 executing twice?

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

typedef struct {
    int start, end, thread_number;
    int *data;
} Func_args;

static pthread_mutex_t mutex;
static int sum = 0;

void *calculate_sum(void *args) {

    int *arr = ((Func_args *)args)->data;
    int i = ((Func_args *)args)->start;
    int end = ((Func_args *)args)->end;
    int t_id = ((Func_args *)args)->thread_number;

    while (i < end) {
        pthread_mutex_lock(&mutex);
        printf("Thread #%d adding %d to %d\t", t_id, arr[i], sum);
        sum += arr[i++];
        printf("New sum: %d\n", sum);
        pthread_mutex_unlock(&mutex);
    }

    return NULL;
}

#define NUM_THREAD 5
#define ARRAY_LEN 10

int main(int argc, char **argv) {

    int array[ARRAY_LEN] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    pthread_t tid[NUM_THREAD];
    int i, pos = 0;

    pthread_mutex_init(&mutex, NULL);

    for (i = 0; i < NUM_THREAD; i++) {
        Func_args args;
        args.data = array;
        args.thread_number = i;
        args.start = pos;
        pos += 2;
        args.end = pos;
        pthread_create(&tid[i], NULL, calculate_sum, &args);
    }

    for (i = 0; i < NUM_THREAD; i++)
        pthread_join(tid[i], NULL);

    pthread_mutex_destroy(&mutex);
    printf("Sum: %d\n", sum);

    return 0;
}

Answer 1

You are passing each thread a pointer to an object that might get destroyed before the thread starts.

args is local, so it gets destroyed when the program exits the scope it's declared in - that is, at the end of the for loop body.

The thread might takes a few moments to start up, so if the thread starts after that, it will access a destroyed object - in practice, the memory will have been reused to store the next thread's values.

You can fix it by dynamically allocating the thread data with malloc (and remembering to free it in the thread or if pthread_create fails).