How to get the correct order of execution of pthreads

Question

I was doing Histogram using pthreads and after long struggle on it.. finally it said:

Segmentation Fault (Core Dumped) 

unfortunately I had this line

p=(struct1 *)malloc(sizeof(struct1)); 

after getting the values to the struct variables from command line.. So that was cleared off.. Thanks for @DNT for letting me know that..

Now when I try to execute the following program.. It sometimes displays the output and sometimes it is going out to the which_bin function and prints the following

output type 1(which is not the correct output): 
Data = 0.000000 doesn't belong to a bin! 
Quitting

output type 2(almost the correct output of histo with time taken by threads): 
10.000-28.000: 
28.000-46.000: 
46.000-64.000: 
64.000-82.000: 
82.000-100.000: XXXXXXXXXX 
The code to be timed took 0.000415 seconds

My ques is why the same prog when ran shows different outputs.. I am confused of what it is exactly looking for..

    #include <stdio.h>
    #include <stdlib.h>
    #include <pthread.h>
    #include "timer.h"
    void Usage(char prog_name[]);

    void Gen_data(void *p);
    void Gen_bins(void *p);
    int Which_bin(void *p);
    void Print_histo(void *p);


    void func(void *p);

    struct test
    {
     int bin_count, i, bin;
     float min_meas, max_meas;
     float* bin_maxes;
     int* bin_counts;
     int data_count;
     float* data;
    };

    typedef struct test struct1;

    int main(int argc, char* argv[])
    {
    double start, finish, elapsed;
    GET_TIME(start);

    struct1 *p;
    pthread_t th1, th2, th3;

    p=(struct1 *)malloc(sizeof(struct1));
    if (argc != 5)
        Usage(argv[0]);
    p->bin_count = strtol(argv[1], NULL, 10);
    p->min_meas = strtof(argv[2], NULL);
    p->max_meas = strtof(argv[3], NULL);
    p->data_count = strtol(argv[4], NULL, 10);

    p->bin_maxes = malloc(p->bin_count*sizeof(float));
    p->bin_counts = malloc(p->bin_count*sizeof(int));
    p->data = malloc(p->data_count*sizeof(float));


    pthread_create(&th1,NULL,(void*) Gen_data,(void*) p);
    pthread_create(&th2,NULL,(void*) Gen_bins,(void*) p);
    pthread_create(&th3,NULL,(void*) func,(void*) p);
    printf("Hi\n");


    pthread_join(th1,NULL);
    pthread_join(th2,NULL);
    pthread_join(th3,NULL);


    Print_histo(p);
    free(p->data);
    free(p->bin_maxes);
    free(p->bin_counts);

    GET_TIME(finish);
    elapsed = finish - start;
    printf("The code to be timed took %f seconds\n", elapsed);
    return 0;
}  /* main */

void func(void *p)
{
    int i;
    struct1 *args;
    args=(struct1*)p;

    for (i = 0; i < args->data_count; i++)
    {
        args->bin = Which_bin(args);
        args->bin_counts[args->bin]++;
    }

    #  ifdef DEBUG
        printf("bin_counts = ");
        for (i = 0; i < args->bin_count; i++)
            printf("%d ", args->bin_counts[i]);
        printf("\n");
    #  endif
}

/*---------------------------------------------------------------------
 * Function:  Usage
 * Purpose:   Print a message showing how to run program and quit
 * In arg:    prog_name:  the name of the program from the command line
 */
void Usage(char prog_name[] /* in */)
{
    fprintf(stderr, "usage: %s ", prog_name);
    fprintf(stderr, "<bin_count> <min_meas> <max_meas> <data_count>\n");
    exit(0);
}  /* Usage */


void Gen_data(void *p)
{
    struct1 *args;
    args=(struct1*)p;
    int i;
    srandom(0);
    for (i = 0; i < args->data_count; i++)
        args->data[i] = args->min_meas + (args->max_meas - args->min_meas)*random()/((double) RAND_MAX);

    #ifdef DEBUG
        printf("data = ");
        for (i = 0; i < args->data_count; i++)
            printf("%4.3f ", args->data[i]);
        printf("\n");
    #endif
} /* Gen_data */


void Gen_bins(void* p)
{
    struct1 *args;
    args=(struct1*)p;
    float bin_width;
    int   i;
    bin_width = (args->max_meas - args->min_meas)/args->bin_count;

    for (i = 0; i < args->bin_count; i++)
    {
        args->bin_maxes[i] = args->min_meas + (i+1)*bin_width;
        args->bin_counts[i] = 0;
    }

    #  ifdef DEBUG
        printf("bin_maxes = ");
        for (i = 0; i < args->bin_count; i++)
            printf("%4.3f ", args->bin_maxes[i]);
        printf("\n");
    #  endif
}

int Which_bin(void* p)
{
    struct1 *args;
    args=(struct1*)p;
    int bottom = 0, top =  args->bin_count-1;
    int mid;
    float bin_max, bin_min;

    while (bottom <= top)
    {
        mid = (bottom + top)/2;
        bin_max = args->bin_maxes[mid];
        bin_min = (mid == 0) ? args->min_meas: args->bin_maxes[mid-1];
        if (*(args->data) >= bin_max)
            bottom = mid+1;
        else if (*(args->data) < bin_min)
            top = mid-1;
        else
            return mid;
    }
    fprintf(stderr, "Data = %f doesn't belong to a bin!\n", args->data);
    fprintf(stderr, "Quitting\n");
    exit(-1);
}

void Print_histo(void *p)
{
    struct1 *args;
    args=(struct1*)p;
    int i, j;
    float bin_max, bin_min;

    for (i = 0; i < args->bin_count; i++)
    {
        bin_max = args->bin_maxes[i];
        bin_min = (i == 0) ? args->min_meas: args->bin_maxes[i-1];
        printf("%.3f-%.3f:\t", bin_min, bin_max);
        for (j = 0; j < args->bin_counts[i]; j++)
            printf("X");
        printf("\n");
    }
}

/* Print_histo */    #include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include "timer.h"
void Usage(char prog_name[]);

void Gen_data(void *p);
void Gen_bins(void *p);
int Which_bin(void *p);
void Print_histo(void *p);

void func(void *p);

pthread_mutex_t lock;
struct test
{
    int bin_count, i, bin;
    float min_meas, max_meas;
    float* bin_maxes;
    int* bin_counts;
    int data_count;
    float* data;
};

typedef struct test struct1;

int main(int argc, char* argv[])
{
    if (pthread_mutex_init(&lock, NULL) != 0)
    {
        printf("\n mutex init failed\n");
        return 1;
    }
    double start, finish, elapsed;
    GET_TIME(start);

    struct1 *p;
    pthread_t th1, th2, th3;

    p=(struct1 *)malloc(sizeof(struct1));
    if (argc != 5)
        Usage(argv[0]);
    p->bin_count = strtol(argv[1], NULL, 10);
    p->min_meas = strtof(argv[2], NULL);
    p->max_meas = strtof(argv[3], NULL);
    p->data_count = strtol(argv[4], NULL, 10);

    p->bin_maxes = malloc(p->bin_count*sizeof(float));
    p->bin_counts = malloc(p->bin_count*sizeof(int));
    p->data = malloc(p->data_count*sizeof(float));


    pthread_create(&th1,NULL,(void*) Gen_data,(void*) p);
    pthread_create(&th2,NULL,(void*) Gen_bins,(void*) p);
    pthread_create(&th3,NULL,(void*) func,(void*) p);
    printf("Hi\n");


    pthread_join(th1,NULL);
    pthread_join(th2,NULL);
    pthread_join(th3,NULL);


    Print_histo(p);
    free(p->data);
    free(p->bin_maxes);
    free(p->bin_counts);

    GET_TIME(finish);
    elapsed = finish - start;
    printf("The code to be timed took %f seconds\n", elapsed);
    return 0;
}  /* main */

void func(void *p)
{
    pthread_mutex_lock(&lock);
    printf("th3 from Gen_func\n");
    int i;
    struct1 *args;
    args=(struct1*)p;

    for (i = 0; i < args->data_count; i++)
    {
        args->bin = Which_bin(args);
        args->bin_counts[args->bin]++;
    }

    #  ifdef DEBUG
        printf("bin_counts = ");
        for (i = 0; i < args->bin_count; i++)
            printf("%d ", args->bin_counts[i]);
        printf("\n");
    #  endif
    pthread_mutex_unlock(&lock);
}

/*---------------------------------------------------------------------
 * Function:  Usage
 * Purpose:   Print a message showing how to run program and quit
 * In arg:    prog_name:  the name of the program from the command line
 */
void Usage(char prog_name[] /* in */)
{
    fprintf(stderr, "usage: %s ", prog_name);
    fprintf(stderr, "<bin_count> <min_meas> <max_meas> <data_count>\n");
    exit(0);
}  /* Usage */


void Gen_data(void *p)
{
    pthread_mutex_lock(&lock);
    printf("th1 from Gen_data\n");
    struct1 *args;
    args=(struct1*)p;
    int i;
    srandom(0);
    for (i = 0; i < args->data_count; i++)
        args->data[i] = args->min_meas + (args->max_meas - args->min_meas)*random()/((double) RAND_MAX);

    #ifdef DEBUG
        printf("data = ");
        for (i = 0; i < args->data_count; i++)
            printf("%4.3f ", args->data[i]);
        printf("\n");
    #endif
    pthread_mutex_unlock(&lock);
} /* Gen_data */


void Gen_bins(void* p)
{
    pthread_mutex_lock(&lock);
    printf("th2 from Gen_bins\n");
    struct1 *args;
    args=(struct1*)p;
    float bin_width;
    int   i;
    bin_width = (args->max_meas - args->min_meas)/args->bin_count;

    for (i = 0; i < args->bin_count; i++)
    {
        args->bin_maxes[i] = args->min_meas + (i+1)*bin_width;
        args->bin_counts[i] = 0;
    }

    #  ifdef DEBUG
        printf("bin_maxes = ");
        for (i = 0; i < args->bin_count; i++)
            printf("%4.3f ", args->bin_maxes[i]);
        printf("\n");
    #  endif
    pthread_mutex_unlock(&lock);
}

int Which_bin(void* p)
{
    struct1 *args;
    args=(struct1*)p;
    int bottom = 0, top =  args->bin_count-1;
    int mid;
    float bin_max, bin_min;

    while (bottom <= top)
    {
        mid = (bottom + top)/2;
        bin_max = args->bin_maxes[mid];
        bin_min = (mid == 0) ? args->min_meas: args->bin_maxes[mid-1];
        if (*(args->data) >= bin_max)
            bottom = mid+1;
        else if (*(args->data) < bin_min)
            top = mid-1;
        else
            return mid;
    }
    fprintf(stderr, "Data = %f doesn't belong to a bin!\n", args->data);
    fprintf(stderr, "Quitting\n");
    exit(-1);
}

void Print_histo(void *p)
{
    struct1 *args;
    args=(struct1*)p;
    int i, j;
    float bin_max, bin_min;

    for (i = 0; i < args->bin_count; i++)
    {
        bin_max = args->bin_maxes[i];
        bin_min = (i == 0) ? args->min_meas: args->bin_maxes[i-1];
        printf("%.3f-%.3f:\t", bin_min, bin_max);
        for (j = 0; j < args->bin_counts[i]; j++)
            printf("X");
        printf("\n");
    }
}

/* Print_histo */

I have added the lines to see if all the threads are accessing its functions.. I observed this..

output 1:
Hi
th1 from Gen_data
th3 from Gen_func
Data = 0.000000 doesn't belong to a bin!
Quitting

In the output 1, I can see that th2 is not executed and program ended displaying error..

output 2:
th1 from Gen_data
Hi
th2 from Gen_bins
th3 from Gen_func
10.000-28.000:
28.000-46.000:
46.000-64.000:
64.000-82.000:
82.000-100.000: XXXXXXXXXX
The code to be timed took 0.000348 seconds

In output 2, all the threads are executed and so is the output..

I am confused that why the thread th2 is not being executed and how can I make sure that all threads runs in correct order..

I would like to know if the program is logically wrong? if it is wrong logically in that case why is it showing the histogram output at times.. Thanks!

Answer 1

Order of thread execution is not guaranteed. On a modern, multi-core processor, the threads may even execute concurrently. There is no guarantee that the Gen_bins thread completes before the func thread. Since your threads access and manipulate the same data structures, the results are unpredictable as you have noticed.

While I don't think threads are necessary for this application, make the following change to ensure the threads execute in the order listed. Change:

pthread_create(&th1,NULL,(void*) Gen_data,(void*) p);
pthread_create(&th2,NULL,(void*) Gen_bins,(void*) p);
pthread_create(&th3,NULL,(void*) func,(void*) p);
pthread_join(th1,NULL);
pthread_join(th2,NULL);
pthread_join(th3,NULL);

to:

pthread_create(&th1,NULL,(void*) Gen_data,(void*) p);
pthread_join(th1,NULL);
pthread_create(&th2,NULL,(void*) Gen_bins,(void*) p);
pthread_join(th2,NULL);
pthread_create(&th3,NULL,(void*) func,(void*) p);
pthread_join(th3,NULL);

This ensures that each thread executes and completes before the next starts. Again, since the threads aren't executing concurrently, threading isn't necessary for this program and just adds complexity.