CODEWITHSUNDEEP
cconcurrencysemaphore
Bruno Kim
Bruno Kim

Reputation: 2350

Solve race condition during semaphore initialization

I have an array that must be shared between threads, protected by a semaphore. I've put the initialization code inside a function that can be called multiple times, a "constructor", as follows:

#include <stdbool.h> //for bool
#include <semaphore.h>

sem_t global_mutex;
char global_array[N]; // Protected with global_mutex

struct my_struct *new_my_struct(){
    static bool is_init = false; // This will be initialized only once, right?
    if (!is_init){                         // 1
        sem_init(&global_mutex, 0, 1);     // 2
        sem_wait(&global_mutex);           // 3
        if (!is_init){                     // 4
           is_init = true;                 // 5
           ... initialize global_array ... // 6
        }
        sem_post(&global_mutex);           // 7
    }

    ... proceed on the create and return a my_struct pointer ...
}

In an ideal world, a thread would run from 1 to 7, initialize the array and exit the critical region. Even if another thread had stopped in 2, the test in 4 would be false and the array wouldn't be overwritten. I haven't thinked much of what would happen if a thread stuck in 1 and reinitialized the semaphore, but I believe it isn't of much concern as long as is_init be set to true by the first thread to run!

Now, there is a race condition if a thread stops in 4, and another one runs from the beggining to completion, initializing and populating the global_array. When the thread stopped at 4 runs, it will reinitialize the array and delete the state stored by the first thread.

I would like to know if there is any way to not suffer that race condition (maybe a clever use of static?) or if I should separate the initialization code from the constructor and use it in the main thread, when there's no concurrency.

This code is in use and I haven't suffered from a race condition yet. However, as I know its possible, I'd wish to correct it.

Upvotes: 1

Views: 953

Answers (2)

Steve Jessop
Steve Jessop

Reputation: 279215

There are a few ways to do thread-safe lazy initialization, but this isn't one of them.

pthread_once is one way, and using a global mutex that's actually a mutex (initialized statically) to synchronize the initialization is another. Implementations might guarantee thread-safe initialization of static local variables, but don't have to (at least, they didn't prior to C11 and I haven't checked that).

However you synchronize the actual initialization, though, double-checked locking is not guaranteed to work in C or in Posix. It's a data race to check a flag in one thread, that was set in another thread, without some kind of synchronization in both threads. The implementation of pthread_once should do its best to be fast in the common case that the initialization has already been done. If your implementation guarantees thread-safe intialization of function-scoped static variables, then that will also do its best. Unless you really know what you're doing (e.g. you're implementing pthread_once yourself for some new system), use one of those in preference to rolling your own attempt to avoid costly locking in the common case.

Upvotes: 1

Jens Gustedt
Jens Gustedt

Reputation: 78903

If the real use of the semaphore is really as a mutex, use just that pthread_mutex_t. These can be initialized statically, so your problem would disappear.

The syntax would be

pthread_mutex_t global_mutex = PTHREAD_MUTEX_INITIALIZER;

If you really need a dynamic initialization of a global object, have a look into pthread_once. This is the type (pthread_once_t) and function that is foreseen by POSIX for such a task.

Upvotes: 2

Related Questions