I want to write a program, what counts to 100. I want to accomplish this with 10 threads, using pthread lock. When the program steps into a thread it generates a number between 0-2, this value will be added to it’s index of array and also to the global variable sum. When the sum value reaches 100, every thread should print it’s own value of array (the amount of the entire array should be equal with the sum variable).
My problem is the following: Always the first thread locks the mutex variable, but I want to distribute the tasks between all threads (arr[1] = 100, every other = 0, but I want for example arr[1] = 14, arr[2] = 8, etc. up to 100). Where am I wrong?
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>
#include <time.h>
#include <errno.h>
pthread_mutex_t mutex;
int arr[10];
int sum = 0;
void *add (void* input) {
int *id = (int*) input, x, s;
int ind = *id;
while (sum < 100) {
while (1) {
if (pthread_mutex_trylock(&mutex) != EBUSY)
break;
}
if (sum < 100) {
x = rand() % 3;
arr[ind] = arr[ind] + x;
sum += x;
}
pthread_mutex_unlock(&mutex);
sleep(0.1);
}
printf("The %d. thread got %d points!n", ind, arr[ind]);
return NULL;
}
int main (void) {
int i;
pthread_t threads[10];
pthread_mutex_init(&mutex, NULL);
srand(time(NULL));
for (i = 0; i < 10; i++) {
if (pthread_create(&threads[i], NULL, add, &i)){
perror("pthread_create");
exit(1);
}
}
for (i = 0; i < 10; i++) {
if (pthread_join (threads[i], NULL)){
perror("pthread_join");
}
}
pthread_mutex_destroy(&mutex);
return 0;
}
The output:
The 1. thread got 100 points! The 2. thread got 0 points! The 3. thread got 0 points! The 4. thread got 0 points! The 5. thread got 0 points! The 6. thread got 0 points! The 7. thread got 0 points! The 8. thread got 0 points! The 9. thread got 0 points! The 0. thread got 0 points!
Advertisement
Answer
You have several problems in your code, with varying degrees of relevance to the specific issue you asked about. Before we go further, one output I get from running your code as written may be illustrative:
The 2. thread got 100 points! The 8. thread got 0 points! The 9. thread got 0 points! The 5. thread got 0 points! The 5. thread got 0 points! The 6. thread got 0 points! The 7. thread got 0 points! The 2. thread got 100 points! The 4. thread got 0 points! The 1. thread got 0 points!
Note that some threads got the same counter index. This can happen because you have a data race between the main thread and each child thread: the main thread passes a pointer to its local variable i, and then proceeds to modify the value of that variable. Meanwhile the child thread reads the value of the variable via the pointer. These actions are not synchronized, so the behavior is undefined.
This particular problem has several solutions. The simplest is probably to cast i to void * and pass the result (by value). This is in fact pretty common where you just want to pass an integer:
if (pthread_create(&threads[i], NULL, add, (void *)i)) {
// ...
Of course, the thread function needs to convert it back:
void *add (void* input) {
int /*no need for 'id' */ x, s;
int ind = (int) input;
// ...
Next, observe that you have another data race among all your child threads where you read the value of sum in the condition of the while loop. That one does not seem to be biting you at the moment, but it could do at any time. Since the threads, as a group, both read and modify sum, you must ensure that all such accesses are synchronized — for example, by performing them only while holding the mutex locked.
Skipping a bit (we’ll come back), you have a problem with your sleep() call. That function’s parameter is of type int, so your actual argument, the double 0.1, is converted to int, yielding 0. The compiler might optimize that out altogether, and if it doesn’t, then it might effectively be a no-op. Even more importantly, however. sleep() is simply the wrong tool for this job, or for pretty much any job related to inter-thread synchronization and timing.
Recognizing now that there is no sleeping, you should see that your outer while loop is quite tight, in that a thread that has just unlocked the mutex will immediately try to lock it again. It is quite common for such a re-locking attempt to succeed on the first try, because default mutex behavior makes no promises about thread scheduling fairness.
Additionally, you gain no particular benefit from the peculiar busy loop around pthread_mutex_trylock(). Just use pthread_mutex_lock() if otherwise all you’re going to do when the mutex is busy is re-try. This will save you CPU time and does not have appreciably different semantics.
Overall, though, you have a strategy problem. Mutexes do not generally provide any guarantees around fair scheduling. Usually for a tight multithread interaction such as this one, you need to perform some kind of scheduling management manually, and that typically benefits greatly from adding a condition variable to the mix.
It looks like you don’t want to force the threads to take turns, but perhaps it would be sufficient to ensure that the thread that just ran does not immediately get selected again. In that case, you could add a shared variable that records the index of the thread that just ran, and use that to prevent it from being selected again as the next thread. Schematically, the thread function might then look something like this
- loop indefinitely:
- lock the mutex
- loop indefinitely:
- compare the last thread index to my index
- if it is different, break from the (inner) loop
- else wait on the condition variable
- set my index as the last thread index
- (still holding the mutex locked) broadcast to or signal the CV
- if
sumis less than 100- update
sum - unlock the mutex
- update
- otherwise (
sumis >= 100)- unlock the mutex
- break from the (outer) loop