Question

I thought up a good use of the static keyword inside a function to be something like this:

void threadSafeWrite(int *array, int writeIndex, int writeData){
    static void *threadLock = Lock_create(); //in my code locks are void* to be cross-platform compatable
    Lock_aquire(threadLock);
    array[writeIndex] = writeData;
    Lock_release(threadLock);
}

In short it seems like a good way to make a critical section. My question is though, how do I initialize the threadLock in a threadsafe manner? The problem with the example I fear is that the lock will get allocated multiple times and each thread will use a different lock. Any ideas on how to fix this? Seems like a chicken and egg problem. I want a solution (or solutions) which work with both pthreads and windows threads.

EDIT: the reason I want this functionality is because it provides a non-intrusive way to test whether there is a difference when running a piece of code single-threaded or multithreaded (meant for debug purposes).

Was it helpful?

Solution

One approach is to use a global lock to serialize the initialization paths. You'll need a portable wrapper over a SMP memory barrier however; the acquire barrier implied by the lock is not sufficient, as it, in principle, allows the compiler and/or CPU to cache the results of memory reads from before the lock acquisition. Here's an example:

Lock global_init_lock; // Should have low contention, as it's only used during startup

void somefunc() {
    static void *data;
    static long init_flag = 0;
    if (!init_flag) { // fast non-atomic compare for the fast path
        global_init_lock.Lock();
        read_memory_barrier(); // make sure we re-read init_flag
        if (!init_flag)
            data = init_data();
        write_memory_barrier(); // make sure data gets committed and is visible to other procs
        init_flag = 1;
        global_init_lock.Unlock();
    }
    read_memory_barrier(); // we've seen init_flag = 1, now make sure data is visible
    // ....
}

That said, I would recommend putting the locks with the data, not with the functions that act on the data. After all, how do you intend to synchronize readers like this? What if you want to use separate locks for separate arrays later? And what if you want to write other functions that take that lock later down the road?

OTHER TIPS

Won't work because the initializer of a static variable must be a constant in C. A function call is not a constant. This is different from C++, where you can make a static perform work before main is entered. For example, this won't compile:

int deepthought()
{
    return 42;
}

void ask()
{
    static int answer = deepthought();
}

The simplest option is to make your locks global and initialize them before going into MT mode. A better option is to pass them along with the data they guard.

P.S.: I advise against using a void * to get an opaque pointer. Instead, implement a platform-specific, one-element struct Lock for type-safety.

The whole idea of a Lock_create function is broken; the act of creating it would require synchronization, which you can't guarantee because you don't have a lock yet! Don't use pointers for locks. Instead create a struct and pass the address of that struct to your locking and unlocking functions. Or better yet, use the ones provided by your OS, since there's no way to implement locking yourself in plain C.

Also everyone who's said you need to make the lock "global" is wrong. Function level scope is fine as long as it's static storage duration, which you have. You just need to eliminate the allocation function Lock_create and use an appropriate type for the lock.

There are various options:

  • Manually call the initialization code at a safe time (e.g. before exposing the code to additional threads).
  • Use pthread_once() or equivalent, which will ensure the initialization code is called once and all subsequent callers see its effects.
  • Use a static initialization of the lock that does not involve calling any function. For example, using pthreads

    static pthread_mutex_t mylock = PTHREAD_MUTEX_INITIALIZER;
    
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top