Question

I'm writing C multi thread program with the following structure:

struct mystruct {
    int a;
    int b;
    int c;
    int d;
} Data;


void *thr_1();
void *thr_2();


int main(int argc, char *argv[]) {
    pthread_t t_1,
          t_2;

    if (pthread_create(&t_1, NULL, thr_1, NULL)
     || pthread_create(&t_2, NULL, thr_2, NULL)) {
        perror("pthread_create() on main()");
        return -1;
    }

    while (a < 2000) {
    /* do a lot of stuff with Data (never writes on it) */
    }

    pthread_cancel(thr_1);
    pthread_cancel(thr_2);
    pthread_join(thr_1, NULL);
    pthread_join(thr_2, NULL);

    return 0;
}

/* Threads */
void *thr_1() {
    while (1) {
        /* Read from stream and write to Data */
        usleep(50000);
    }
    return NULL;
}

void *thr_2() {
    while (1) {
        /* do some stuff with Data (never writes on it) */
        usleep(50000);
    }
    return NULL;
}

This code is working, but after some research (race conditions and thread safety) it became clear that this code can fail at any moment because of those race conditions. My first thought then was to use mutex to lock the members of the structure on write and release it after, but there are too many read access to the Data structure in the main loop and in the thr_2. My solution so far is to make 2 access functions, one to read Data and one to write on Data, and inside those functions, use mutex to lock on write. This ugly solution seems more like a hack to me.... So, finally, my question is: is there a better way to do this?? preferably without any function needed to access Data.

Thanks a lot!

Was it helpful?

Solution

C11 supports atomic operations. Unfortunately, most compilers do not support C11 atomics, but you can them for example with GCC 4.9:

#include <stdatomic.h>

struct mystruct
{
    _Atomic int a;
    ...
};

int load(struct mystruct* s)
{
    return atomic_load(&s->a);
}

void store(struct mystruct* s, int value)
{
    atomic_store(&s->a, value);
}

Note that it is not sufficient to protect only write operations. The C11 standard is pretty clear about that. If two threads can access the same memory location at the same time, and at least one of the operations is a write access, then the behaviour of the program is undefined. That means you have to protect all accesses.

OTHER TIPS

You really need a bit more information to make a truly informed response but a few quick items.

Atomics are also available in boost as well, boost::atomic<>, which would allow you todo the following.

struct mystruct 
{
    boost::atomic<int> a;
    boost::atomic<int> b;
    boost::atomic<int> c;
    boost::atomic<int> d;
}Data;

boost should take care of all of the gotchas for you depending on the platform, as some do not have fully qualified CAS and/or memory barriers.

Along those lines you could also use one of the lockfree structures in boost. Just remember in both of these cases, atmoics and lockfree, the best case scenario is that they are implemented with CAS operations, worst case can be much uglier. Even CAS is not free as the operation could empty the CPUs instruction cache causing delays in execution.

However depending on your use case (as you noted only one thread writes to the data) if your processing can be tolerant to delayed updates of the data then no synchronization would be required.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top