Pthreads program produces wrong outputs and correct outputs sometimes

Question

I am learning parallel programming using Pthreads. I wrote the following simple program as a try. It takes two vectors from stdin (the user specifies the length as command line argument) then it adds them, subtracts them, multiply them element-wise and divide them element-wise by creating a thread for each operation of the four. the problem is sometimes the code works fine and when i use the same input again it just prints zeros.

Why does it behave like this?

I am using Ubuntu 14.04 on virtual machine.

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

#define loop(i, n) for (i = 0; i < n; i++)

int thread_count, n;
int *a;
int *b;
int** results; // holds the four output vectors
void* math(void* opl);


int main(int argc, char* argv[]){
    int thread = 0, i;
    pthread_t* thread_handles;

    n = strtol(argv[1], NULL, 10);

    results = malloc(4 * sizeof(int*));
    a = malloc(n * sizeof(int));
    b = malloc(n * sizeof(int));

    loop(i, n)
        scanf("%d", a + i);

    loop(i, n)
        scanf("%d", b + i);

    thread_handles = malloc(4*sizeof(pthread_t));

    loop(thread, 4){
        results[thread] = malloc(n * sizeof(int));
        pthread_create(&thread_handles[thread], NULL, math, (void *)thread);
    }

    printf("Hello from the main thread\n");

    loop(thread, 4);
        pthread_join(thread_handles[thread], NULL);

    loop(thread, 4){
        printf("Results of operation %d:\n", thread); 
        loop(i, n)
            printf("%d ", results[thread][i]);
        printf("\n");
       free(results[thread]);
    }

    free(thread_handles);
    free(a);
    free(b);
    free(results);

   return 0;    
}

void* math(void* op1){
    int op = (int) op1, i;

    printf("%d ", op);
    if (op == 0)
        loop(i, n)
             results[op][i] = a[i] + b[i];
    else if (op == 1)
         loop(i, n)
             results[op][i] = a[i] - b[i];
    else if (op == 2)
         loop(i, n)
             results[op][i] = a[i] * b[i];
    else
        loop(i, n)
            results[op][i] = a[i] / b[i];



    return NULL;
}

If the user fails to provide an argument to the program, this program should SEGFAULT. You should check argc before indexing into argv. — tweej
I also suggest you look at this answer about casting to (void *) . I mention this not because it is related to your problem, but something you may find if you compile in a different environment. — Michael Petch
such wonderfully meaningful variable names, like 'a' and 'b' sigh . This macro: loop(i, n) is useless, clutters the code, and makes it harder to understand. Strongly suggest replacing those macro invocations with the actual 'for()` statement. — user3629249
When calling the malloc() or calloc() or realloc() functions always check (!=NULL) the returned value, before using that value, to assure the operation was successful — user3629249
a much better way to write the math() function would be to use a switch() statement, which could then have a default: case for when the operator is not one of the 4 valid values — user3629249

tweej tweej · Accepted Answer · 2015-11-02T01:46:41

The problem is induced by the godawful loop define you've learned. Please unlearn it!

#define loop(i, n) for (i = 0; i < n; i++)

This is the problem:

loop(thread, 4);
//             ^ BAD! Putting a semicolon here makes the loop empty.

The correct version using the awful define you've chosen is:

loop(thread, 4)

A better way would be just to say what you mean!

for(i=0; i<n; ++i)
{
   // Do Stuff
}

The problem results in pthread_join being called just once, and then the main thread proceeds on. This likely occurs before all threads have completed, and provided their output in the results variable. No good!

Pthreads program produces wrong outputs and correct outputs sometimes

1 Answers