Global counters and concurrent threads. How to check counter progress?

Question

I am trying to write a simple program to understand threads. I want each thread to increment a global variable 'counter' to 4 million. Each thread only counts to 2 million. I placed a print statement at the end of each function to see how many iterations and where the global counter is at upon completion of the function. But the global counter in thread1Func is always very high, like 3.8 - 3.9 million, and then in thread2Func the counter is always 4 mil (as expected).

Am I doing this correctly? Is there a reason thread1Func is always printing such a high value for 'counter'? I would imagine it should be somewhere between 2 mil - 4 mil more evenly. Any advice would be greatly appreciated!

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


#define MAX 2000000UL
pthread_mutex_t lock;
//pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

struct shared_data
{
    int value;     /* shared variable to store result*/
    
};

struct shared_data  *counter;


void* thread1Func(void * tid){
    uint32_t i = 0;
    while(i < MAX){
        
        if(pthread_mutex_trylock(&lock) == 0){
            counter->value++;
            pthread_mutex_unlock(&lock);
            i++;
        }
    }
    printf("I am thread 1. I counted %d times. Global counter = %d\n", i, counter->value);
    return NULL;
}

void* thread2Func(void * tid){
    
    uint32_t i = 0;
    while(i < MAX){
        
        if(pthread_mutex_trylock(&lock) == 0){
            counter->value++;
            pthread_mutex_unlock(&lock);
            i++;
        }

    }
    printf("I am thread 2. I counted %d times. Global counter = %d\n", i, counter->value);
    return NULL;
}


int main() {
    
    counter = (struct shared_data *) malloc(sizeof(struct shared_data));
    printf("Initial Counter Value: %d\n", counter->value);
    
    pthread_t thread1;
    pthread_t thread2;
    pthread_mutex_init(&lock, NULL);
    
    pthread_create(&thread1, NULL, thread1Func, NULL);
    pthread_create(&thread2, NULL, thread2Func, NULL);
    
    
    pthread_join(thread1,NULL);
    pthread_join(thread2,NULL);
    
    
    printf("Final Counter: %d\n", counter->value);
    return 0;
}

You are not initializing `counter->value`. It would probably be appropriate to initialize it to `0`. What does the line `printf("Initial Counter Value: %d\n", counter->value);` print? — Andreas Wenzel, Feb 17 '23 at 23:26
Side note: Instead of `counter = (struct shared_data *) malloc(sizeof(struct shared_data));`, you can simply write `counter = malloc(sizeof *counter);`. You may want to read this: [Do I cast the result of malloc?](https://stackoverflow.com/q/605845/12149471) — Andreas Wenzel, Feb 17 '23 at 23:41
Why shouldn't it be? Both threads try to run at the same time — user253751, Feb 18 '23 at 00:02
@Matthew: Note that you cannot rely on the memory returned by `malloc` to be `0`. It just happens to be `0` in your case. If you want it to be guaranteed that it has the value `0`, you can (1) assign the value `0` to `counter->value` or (2) use `calloc` instead of `malloc`. — Andreas Wenzel, Feb 18 '23 at 11:26
Style note: Your `thread1Func` and `thread2Func` are identical except for the string literals in their `printf()` statements. It would be better practice to just have a single function, and pass in its thread number through an argument struct. It's no big deal here, but in large projects, this kind of copy-and-paste programming can lead to errors when somebody needs to change the behavior, and they forget to change it in _all_ of the different copies of the original function. https://en.m.wikipedia.org/wiki/Don%27t_repeat_yourself — Solomon Slow, Feb 18 '23 at 22:47

score 0 · Accepted Answer · answered Feb 18 '23 at 22:58

the global counter in thread1Func is always very high, like 3.8 - 3.9 million, and then in thread2Func the counter is always 4 mil

That is no surprise. If both threads are working at roughly the same speed, then when the first thread finishes, the second thread should be very nearly finished too. The first thread at that point has added its two million to the global counter, and then second thread already will have added almost two million of its own. You should totally expect the global counter to be almost four million when the first thread finishes.

The only way the first thread could print two million is if the first thread is finished before the second thread has begun to work.

That makes sense. The threads should be distributed somewhat evenly. I had a misunderstanding about how the threads worked with concurrency, I thought it would be more random. Thank you for clarifying. — Matthew, Mar 12 '23 at 01:00

score -1 · Answer 2 · answered Feb 18 '23 at 01:10

Fun example, good work!

Try changing the thread priorities and see what happens to see different counts (see code below).

Maybe consider adding a semaphore to ping pong the count between the 2 threads so they execute equally and report 3999999 and 4000000 as the final counts.

I am sure you have other ideas too, thanks for posting.

gcc main.c -o main
./main
Initial Counter Value: 0
I am thread 2. I counted 2000000 times. Global counter = 3880728
I am thread 1. I counted 2000000 times. Global counter = 4000000
Final Counter: 4000000

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


#define MAX 2000000UL
pthread_mutex_t lock;
//pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

struct shared_data
{
    int value;     /* shared variable to store result*/
    
};

struct shared_data  *counter;


void* thread1Func(void * tid){
    uint32_t i = 0;
    while(i < MAX){
        
        if(pthread_mutex_trylock(&lock) == 0){
            counter->value++;
            pthread_mutex_unlock(&lock);
            i++;
        }
    }
    printf("I am thread 1. I counted %d times. Global counter = %d\n", i, counter->value);
    return NULL;
}

void* thread2Func(void * tid){
    
    uint32_t i = 0;
    while(i < MAX){
        
        if(pthread_mutex_trylock(&lock) == 0){
            counter->value++;
            pthread_mutex_unlock(&lock);
            i++;
        }

    }
    printf("I am thread 2. I counted %d times. Global counter = %d\n", i, counter->value);
    return NULL;
}


int main() {
    
    counter = (struct shared_data *) malloc(sizeof(struct shared_data));
    printf("Initial Counter Value: %d\n", counter->value);
    
    pthread_t thread1;
    pthread_t thread2;
    pthread_mutex_init(&lock, NULL);

    pthread_attr_t attr;
    struct sched_param sch_params;
    pthread_attr_init(&attr);
    pthread_attr_getschedparam(&attr, &sch_params);
    sch_params.sched_priority = 99;
    //pthread_attr_setschedpolicy(&attr, SCHED_RR);
    pthread_attr_setschedparam(&attr, &sch_params);
    pthread_create(&thread1, &attr, thread1Func, NULL);

    sch_params.sched_priority = 1;
    pthread_create(&thread2, &attr, thread2Func, NULL);
    
    
    pthread_join(thread1,NULL);
    pthread_join(thread2,NULL);
    
    
    printf("Final Counter: %d\n", counter->value);
    return 0;
}

Changing the priorities gave me the kind of results I was expecting. Thank you! — Matthew, Feb 18 '23 at 06:29
@Matthew, Changing the priorities is a Bad Idea. If it gave you the results you expected, it's because changing the priorities allowed one thread to do all of its work before the other thread even got a chance to begin. Changing the priorities _effectively_ turned your program into a single--threaded program, which probably is not what you wanted. Thread priorities is a super-advanced topic, and it's very easy to do more harm that good if you mess about with them and you don't have a deep understanding of how priorities affect the relationship between threads. — Solomon Slow, Feb 18 '23 at 23:03

Global counters and concurrent threads. How to check counter progress?

2 Answers2