Multithreaded (C) program threads not terminating

Question

I am trying to finish up a program that uses multiple threads (3) to distribute a hypothetical scholarship of $4000. Each time a thread processes, it "locks" the "critical section" and blocks the other threads from taking out their chunk from the sum. With each access, the thread is meant to take 25% of the remaining sum of "scholarship" money. The output is the amount that each thread takes as it gains access to the scholarship.

My program seems to be processing proper output so far, but when it gets to the end there seems to be a catch. Each process/thread gets to a point where it doesn't terminate or exit and the program just becomes stagnant and doesn't complete. I feel like the threads are processing but not meeting a terminating condition (scholarship is all gone). The last function totalCalc() is never reached. Does anyone see anything that I don't, which could help alleviate this problem or push the program to completion?

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

#define PERCENTAGE 0.25

pthread_mutex_t mutex; // protecting critical section
int scholarship = 4000,
                  total = 0;
void *A();
void *B();
void *C();
void *totalCalc();

int main(){ 

    pthread_t tid1,
              tid2,
              tid3;

    //pthread_setconcurrency(3); 

    pthread_create(&tid1, NULL, (void *(*)(void *))A, NULL );
    pthread_create(&tid2, NULL, (void *(*)(void *))B, NULL );
    pthread_create(&tid3, NULL, (void *(*)(void *))C, NULL );
    pthread_join(tid1,NULL);
    pthread_join(tid2,NULL);
    pthread_join(tid3,NULL);

    totalCalc();


    return 0;

}

void *A(){
    float result;
    while(scholarship > 0){
        sleep(2);
        pthread_mutex_lock(&mutex);
        result = scholarship * PERCENTAGE;
        result = ceil(result);
        total = total + result;
        scholarship = scholarship - result;
        if( result >= 1){
            printf("A = ");
            printf("%.2f",result);
            printf("\n");
        }
        if( scholarship < 1){
            pthread_exit(0);
            printf("Thread A exited\n");
            return;
        }
        pthread_mutex_unlock(&mutex);
    }

    pthread_exit(0);

}

void *B(){
    float result;
    while(scholarship > 0){
        sleep(1);
        pthread_mutex_lock(&mutex);
        result = scholarship * PERCENTAGE;
        result = ceil(result);
        total = total + result;
        scholarship = scholarship - result;
        if( result >= 1){
            printf("B = ");
            printf("%.2f",result);
            printf("\n");
        }
        if( scholarship < 1){
            pthread_exit(0);
            printf("Thread B exited\n");
            return;
        }           
        pthread_mutex_unlock(&mutex);
    }

    pthread_exit(0);
}

void *C(){
    float result;
    while(scholarship > 0){
        sleep(1);
        pthread_mutex_lock(&mutex);
        result = scholarship * PERCENTAGE;
        result = ceil(result);
        total = total + result;
        scholarship = scholarship - result;
        if( result >= 1){
            printf("C = ");
            printf("%.2f",result);
            printf("\n");
        }
        if( scholarship < 1){
            pthread_exit(0);
            printf("Thread C exited\n");
            return;
        }           
        pthread_mutex_unlock(&mutex);       
    }

    pthread_exit(0);
}

void *totalCalc(){
    printf("Total given out: ");
    printf("%d", total);
    printf("\n");
}

Output:

B = 1000.00
C = 750.00
A = 563.00
B = 422.00
C = 317.00
B = 237.00
C = 178.00
A = 134.00
B = 100.00
C = 75.00
B = 56.00
C = 42.00
A = 32.00
B = 24.00
C = 18.00
B = 13.00
C = 10.00
A = 8.00
B = 6.00
C = 4.00
B = 3.00
C = 2.00
A = 2.00
B = 1.00
C = 1.00
B = 1.00
C = 1.00
^C

Answer 1

You should not write the same function out 3 times – you can pass an argument to the thread function to give it different things to do.

• You should initialize your mutex.
• You should use one printf() statement instead of three in a row.
• You should unlock the mutex before you exit the thread function.
• You should print the status before you exit the function.
• You should return a value from the thread function when you write return.
• There's not much virtue in the totalCalc() function once you've collapsed it into a single printf() call.
• The term PERCENTAGE is a misnomer; it is a fraction, not a percentage.

I chose to use return instead of calling pthread_exit(); the difference is not crucial.

First set of cleanups

Here's a revision to your code.

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

#define FRACTION 0.25

static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static int scholarship = 4000;
static int total = 0;
static void *calculate(void *data);

struct Data
{
    const char *name;
    int doze;
};

int main(void)
{
    pthread_t tid1;
    pthread_t tid2;
    pthread_t tid3;
    struct Data a = { "A", 2 };
    struct Data b = { "B", 1 };
    struct Data c = { "C", 1 };

    pthread_create(&tid1, NULL, calculate, &a);
    pthread_create(&tid2, NULL, calculate, &b);
    pthread_create(&tid3, NULL, calculate, &c);
    pthread_join(tid1, NULL);
    pthread_join(tid2, NULL);
    pthread_join(tid3, NULL);

    printf("Total given out: %d\n", total);

    return 0;
}

static void *calculate(void *arg)
{
    struct Data *data = arg;
    float result;
    while (scholarship > 0)
    {
        sleep(data->doze);
        pthread_mutex_lock(&mutex);
        result = scholarship * FRACTION;
        result = ceil(result);
        total = total + result;
        scholarship = scholarship - result;
        if (result >= 1)
        {
            printf("%s = %.2f\n", data->name, result);
        }
        if (scholarship < 1)
        {
            printf("Thread %s exited\n", data->name);
            pthread_mutex_unlock(&mutex);
            return 0;
        }
        pthread_mutex_unlock(&mutex);
    }

    return 0;
}

And sample output (on a Mac running macOS Sierra 10.12.6, using GCC 7.1.0):

B = 1000.00
C = 750.00
A = 563.00
B = 422.00
C = 317.00
B = 237.00
C = 178.00
A = 134.00
B = 100.00
C = 75.00
B = 56.00
C = 42.00
A = 32.00
C = 24.00
B = 18.00
C = 13.00
B = 10.00
A = 8.00
C = 6.00
B = 4.00
B = 3.00
C = 2.00
A = 2.00
B = 1.00
C = 1.00
B = 1.00
C = 1.00
Thread C exited
Thread A exited
Thread B exited
Total given out: 4000

---

First phase improvement

Remember: working code can usually be improved. Here's another revision of the calculate() function which has cleaner handling of the termination conditions.

static void *calculate(void *arg)
{
    struct Data *data = arg;
    while (scholarship > 0)
    {
        sleep(data->doze);
        pthread_mutex_lock(&mutex);
        float result = ceil(scholarship * FRACTION);
        total += result;
        scholarship -= result;
        if (result >= 1)
            printf("%s = %.2f\n", data->name, result);
        pthread_mutex_unlock(&mutex);
    }
    printf("Thread %s exited\n", data->name);

    return 0;
}

It still uses mixed-mode arithmetic (floating point and integer). Further improvements would involve things like modifying the main() function to use arrays instead of separate variables for thread ID and control structures. You could then have 2-26 threads rather easily. You might also use sub-second sleeps, too. You might have different threads being differently generous with what's left of the grant money — instead of a fixed fraction, you could use different fractions in different threads.

---

All-singing, all-dancing version

There's one problem in both the previous versions (as pointed out by user3629249 in a comment — though I already had a preliminary version of the code with the necessary fix in it; it just wasn't on SO yet). The code in the calculate() function accesses the shared variable scholarship without holding a mutex on it. That shouldn't really be done. Here's a version that handles that. It also error checks the calls to the pthread_*() functions, reporting the error and exiting if there's a problem. That's dramatic but adequate for test code. The stderr.h header and supporting source code stderr.c can be found in https://github.com/jleffler/soq/tree/master/src/libsoq. The error handling masks the operation of the code to some extent, but it is very similar to what was shown before. The main change is that the mutex is locked before entering the loop, unlocked after exiting the loop, and unlocked before sleeping and relocked after waking.

This code also uses random fractions instead of one fixed fraction, and random sub-second sleep times, and it has five threads instead of just three. It uses arrays of control structures, initializing them as it needs to. Printing the seed (the current time) is a nicety; it would allow you to reproduce the random sequence used if the program was upgraded to process command line arguments. (There'd still be the indeterminacy from thread scheduling issues.)

Note that the single calls to printf() improve the appearance of the output compared with the triple calls in the original. The original code could (and did) interleave parts of lines from different threads. With each printf() producing an entire line, that is no longer an issue. You can look at flockfile() and its friends to see what goes on — there's a sweeping statement in the specification which covers the rest of the I/O library functions.

/* SO 4544-8840 Multithreaded C program - threads not terminating */

#include "stderr.h"     // https://github.com/jleffler/soq/tree/master/src/libsoq
#include <errno.h>
#include <math.h>
#include <pthread.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>

static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static int scholarship = 4000;
static int total = 0;
static void *calculate(void *data);

enum { MAX_THREADS = 5 };
enum { MIN_PERCENT = 10, MAX_PERCENT = 25 };

struct Data
{
    char name[2];
    struct timespec doze;
    double fraction;
};

static inline double random_fraction(void)
{
    return (double)rand() / RAND_MAX;
}

static inline _Noreturn void err_ptherror(int rc, const char *fmt, ...)
{
    errno = rc;
    va_list args;
    va_start(args, fmt);
    err_print(ERR_SYSERR, ERR_STAT, fmt, args);
    va_end(args);
    exit(EXIT_FAILURE);
}

int main(int argc, char **argv)
{
    err_setarg0(argv[argc-argc]);
    pthread_t tids[MAX_THREADS];
    struct Data ctrl[MAX_THREADS];
    unsigned seed = time(0);
    printf("Seed: %u\n", seed);
    srand(seed);
    int rc;

    for (int i = 0; i < MAX_THREADS; i++)
    {
        ctrl[i].name[0] = 'A' + i;
        ctrl[i].name[1] = '\0';
        ctrl[i].doze.tv_sec = 0;
        ctrl[i].doze.tv_nsec = 100000000 + 250000000 * random_fraction();
        ctrl[i].fraction = (MIN_PERCENT + (MAX_PERCENT - MIN_PERCENT) * random_fraction()) / 100;
        if ((rc = pthread_create(&tids[i], NULL, calculate, &ctrl[i])) != 0)
            err_ptherror(rc, "Failed to create thread %d\n", i);
    }

    for (int i = 0; i < MAX_THREADS; i++)
    {
        if ((rc = pthread_join(tids[i], NULL)) != 0)
            err_ptherror(rc, "Failed to join thread %d\n", i);
    }

    printf("Total given out: %d\n", total);

    return 0;
}

static void *calculate(void *arg)
{
    struct Data *data = arg;
    printf("Thread %s: doze = 0.%03lds, fraction = %.3f\n",
           data->name, data->doze.tv_nsec / 1000000, data->fraction);
    int rc;
    if ((rc = pthread_mutex_lock(&mutex)) != 0)
        err_ptherror(rc, "Failed to lock mutex (1) in %s()\n", __func__);
    while (scholarship > 0)
    {
        if ((rc = pthread_mutex_unlock(&mutex)) != 0)
            err_ptherror(rc, "Failed to unlock mutex (1) in %s()\n", __func__);
        nanosleep(&data->doze, NULL);
        if ((rc = pthread_mutex_lock(&mutex)) != 0)
            err_ptherror(rc, "Failed to lock mutex (2) in %s()\n", __func__);
        double result = ceil(scholarship * data->fraction);
        total += result;
        scholarship -= result;
        if (result >= 1)
            printf("%s = %.2f\n", data->name, result);
    }
    if ((rc = pthread_mutex_unlock(&mutex)) != 0)
        err_ptherror(rc, "Failed to unlock mutex (2) in %s()\n", __func__);
    printf("Thread %s exited\n", data->name);

    return 0;
}

You could still make a case for the code to be revised so it checks the scholarship amount after sleeping, breaking an infinite loop in the body of the loop. Such changes are left as a minor exercise for the reader.

Example run

Seed: 1501727930
Thread A: doze = 0.119s, fraction = 0.146
Thread B: doze = 0.199s, fraction = 0.131
Thread C: doze = 0.252s, fraction = 0.196
Thread D: doze = 0.131s, fraction = 0.102
Thread E: doze = 0.198s, fraction = 0.221
A = 584.00
D = 349.00
E = 678.00
B = 314.00
A = 303.00
C = 348.00
D = 146.00
A = 187.00
D = 112.00
E = 217.00
B = 100.00
A = 97.00
C = 111.00
D = 47.00
E = 90.00
A = 47.00
B = 36.00
D = 24.00
A = 31.00
C = 36.00
D = 15.00
E = 29.00
B = 13.00
A = 13.00
D = 8.00
A = 10.00
E = 13.00
B = 6.00
C = 8.00
D = 3.00
A = 4.00
D = 3.00
E = 4.00
B = 2.00
A = 2.00
C = 2.00
D = 1.00
A = 2.00
E = 2.00
B = 1.00
A = 1.00
D = 1.00
Thread D exited
Thread C exited
Thread A exited
Thread E exited
Thread B exited
Total given out: 4000

Answer 2

You should unlock mutexes before returns.

if( scholarship < 1){
    pthread_exit(0);
    printf("Thread A exited\n");
    return;
}
pthread_mutex_unlock(&mutex);

Those mutexes are never unlocked. Put a

pthread_mutex_unlock(&mutex);

before

return;