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