How do I wait on a condition without it being a race condition?-CodePudding

If you run the code as is, you'll likely have no problems. The problem is if broadcast happens before cond_wait this code no longer works. If you uncomment the sleep below you'll encounter the problem every run

How do I write this so there's no race condition? I happen to know I can solve this using futex but I'm looking for a pthread solution

//clang   -g -fsanitize=undefined,thread main.cpp && ./a.out
#include <cstdio>
#include <pthread.h>
#include <unistd.h>
pthread_mutex_t write_lock = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t checkpoint = PTHREAD_COND_INITIALIZER;

int data;

void*fn(void*) {
    pthread_mutex_lock(&write_lock);
    auto temp =   data;
    pthread_mutex_unlock(&write_lock);
    
    //This if makes it so no thread will pass until both/all threads have finished writing to data
    if (temp < 2) {
        //sleep(1); // <--------- uncomment to see problem
        while (temp < 2)
        {
            pthread_mutex_t local_mutex = PTHREAD_MUTEX_INITIALIZER;
            pthread_mutex_lock(&local_mutex);
            pthread_cond_wait(&checkpoint, &local_mutex);

            //Recheck the condition
            pthread_mutex_lock(&write_lock);
            temp = data;
            pthread_mutex_unlock(&write_lock);
        }
    } else {
        pthread_cond_broadcast(&checkpoint);
    }
    
    int sum=data; //no way to get here until both/all threads execute the sync code
    return 0;
}

int main() {
    pthread_t thread_id[2];
    for(int i=0; i<2; i  ) {
        pthread_attr_t attr;
        pthread_attr_init(&attr);
        pthread_create(&thread_id[i], &attr, &fn, 0);
    }
    for(int i=0; i<2; i  ) {
        pthread_join(thread_id[i], 0);
    }
    printf("Finish\n");
}

CodePudding user response：

Maybe I'm not clear on your real problem. This looks like you're attempting to stack up your client threads until such time as all of them have surpassed a specific checkpoint. In this case, that checkpoint is achieved once data has been bumped by all threads

Therefore, data holds your predicate state. Testing or writing it is the purpose of the mutex protection. Change detection is the purpose of the cond-var.

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

pthread_mutex_t write_lock = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t checkpoint = PTHREAD_COND_INITIALIZER;

int data;

void *fn(void *pv)
{
    // construction-passed top limit
    intptr_t n = (intptr_t)pv;

    // changing predicate, so lock mutex
    pthread_mutex_lock(&write_lock);
      data;

    // changed predicate, so tell people.
    pthread_cond_signal(&checkpoint);

    // mutex is still locked. if our predicate state isn't
    //  sufficient to move on, we wait (and release the mutex
    //  in the process)
    while (data < n)
    {
        pthread_cond_wait(&checkpoint, &write_lock);

        // TODO: woke up here. may be spurious, may be legit, but
        //  we're going to make sure on the next iteration of the
        //  loop by rechecking the predicate data, which we can do
        //  because the mutex is locked coming out of the wait.

        // ALSO: if you want off-the-mutex processing here of some
        //  kind you would do it by releasing the mutex, do your
        //  thread work, then reacquire the mutex again before the
        //  next iteration of the loop
    }

    // NOTE: still own the mutex.
    
    // no way to get here until all threads execute the sync code
    //  note: not a legit use of pthread_self, use at your peril
    printf("%p : %d\n", (void*)pthread_self(), data);

    // leaving the party, release the mtx and tell someone.
    pthread_mutex_unlock(&write_lock);
    pthread_cond_signal(&checkpoint);

    return 0;
}

#define N_THREADS   16

int main()
{
    pthread_t thread_id[N_THREADS];
    for (intptr_t i = 0; i < N_THREADS; i  )
        pthread_create(&thread_id[i], NULL, &fn, (void*)(intptr_t)N_THREADS);

    for (intptr_t i = 0; i < N_THREADS; i  )
        pthread_join(thread_id[i], 0);

    printf("Finish\n");
}

Output (example only)

0x7000019e9000 : 16
0x700001448000 : 16
0x700001342000 : 16
0x700001654000 : 16
0x7000016d7000 : 16
0x7000012bf000 : 16
0x70000175a000 : 16
0x7000013c5000 : 16
0x7000017dd000 : 16
0x70000123c000 : 16
0x700001860000 : 16
0x70000154e000 : 16
0x7000018e3000 : 16
0x7000014cb000 : 16
0x700001966000 : 16
0x7000015d1000 : 16
Finish

That is what it at-least seems you're trying to accomplish.