Da Vinci
Da Vinci

Reputation: 21

Confusion with posix threads in cpp

I'm really confused with this particular code. AFAIK, this program should not have a race condition but it does. What is really confusing is removing the loops and just duplicating the code works fine.

NOTE: I saw a question about threads in a loop but it does not really capture what i'm trying to impose.

Here it is

#include <cstdio>
#include <cstdlib>
#include <pthread.h>

void *functionC(void*);
pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER;
int counter = 0;


int main() {
    pthread_t thread1, thread2;
    pthread_t threads[] = { thread1, thread2 };

    for (auto th : threads) {
        if (pthread_create(&th, NULL, &functionC, NULL) != 0)
        {
            printf("Thread Creation Failed");
        }
    }

    for (auto th : threads) {
        pthread_join(th, NULL);
    }

    exit(0);
}

void *functionC(void *) {
    pthread_mutex_lock(&mutex1);

    counter++;
    printf("Counter Value: %d\n", counter);
    pthread_mutex_unlock(&mutex1);

    return NULL;
}

Built as follows

FILE=mutex

all:
        g++ $(FILE).cpp -lpthread -o bin && ./bin

I was expecting the counter variable to increment once per thread but sometimes nothing prints other times the counter variable remains 1 for both executions which i have read is due to low level scheduling operations.

Upvotes: 1

Views: 268

Answers (1)

John Zwinck
John Zwinck

Reputation: 249093

Your bug is here (two places, the first of which is critical):

for (auto th : threads) {

That should be:

for (auto& th : threads) {

It needs to be a reference so that when you take the address of th and pass it to pthread_create(), you are actually passing the address of threads[0] and not merely the address of th.

Also note that thread1 and thread2 are useless in your program, and should be removed. Enabling compiler warnings would tell you this.

Upvotes: 4

Related Questions