2

Consider the following example code snippet:

void thread_function(void *);

volatile int running = 0;
pthread_t myThread;

int main () {
    running =1;
    pthread_create(myThread, NULL, (void *)thread_function, NULL);

    /* do some stuff */

    /* exit condition met */
    running = 0;
    pthread_join(&myThread);

    /* clean up */

    return 0;
}

void thread_function(void *) 
{
    while(running) {
       /* do stuff */
    }
}

I have read lots of posts regarding the volatile keyword and how it should not be used in pretty much all but memory mapped IO but, in this particular circumstance, I cannot understand how it would not perform as expected? Yes, I may get one extra iteration of the while loop (if I'm unlucky enough to get a context switch while I'm setting the 'running' variable to zero), but that doesn't matter.

Is there a good reason why I should mutex protect this variable? Is there something I've overlooked?

Martin
  • 301
  • 2
  • 8
  • 1
    If you are using a multi core cpu volatile i believe is not enough to make changes in different cores immediately visible to all cores. Only the memory barriers that a mutex should use will do that. – RedX Jul 29 '14 at 14:25
  • 3
    C has an `atomic_bool`. – chris Jul 29 '14 at 14:25
  • Related to [Is it necessary to lock an array that is *only written to* from one thread and *only read from* another?](http://stackoverflow.com/questions/24682518/is-it-necessary-to-lock-an-array-that-is-only-written-to-from-one-thread-and) – Shafik Yaghmour Jul 29 '14 at 14:27
  • 2
    Please read this: https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong – Jan Spurny Jul 29 '14 at 14:32
  • wasn't `atomic_bool` rejected? – sp2danny Jul 29 '14 at 14:46
  • @sp2danny: It's in C11, see N1570 7.17.6. – Keith Thompson Jul 29 '14 at 15:10
  • I'd missed the new atomic qualifier. The posts are an interesting read, and nothing I didn't already know. The fact is I'm maintaining a few applications which use *no* locking or synchronisation at all between threads; just global volatiles - counters, flags, timestamps. It seems crazy and actually would be, if there were any problems - which there aren't, it's processing realtime video too! Could it be a platform / toolchain specific thing? Where volatiles are somehow treated atomically? I could experiment (just finding the time...) – Martin Jul 31 '14 at 20:46

2 Answers2

1

As written, your program is safe, because pthread_create and pthread_join are memory barriers. However, as soon as you fill in /* do some stuff */ you will find that writes to running are reordered relative to other data shared between the threads; in this case, this can result in myThread terminating earlier than expected.

Use a condition variable, or atomic_bool. Safe to use volatile bool to force another thread to wait? (C++)

Community
  • 1
  • 1
ecatmur
  • 152,476
  • 27
  • 293
  • 366
  • I don't think it's safe as written. `pthread_join` synchronizes *when it joins*, not as soon as it starts waiting to join, or at least that's the obvious interpretation of what it should do. I see no reason in OP's program that the thread would *ever* terminate. – R.. GitHub STOP HELPING ICE Jul 29 '14 at 14:56
  • Naturally I didn't think these semantics were safe, and previous experience tells me to always protect access to shared variables... But the fact is I've just started maintaining / adding new features to suite of applications which use *no* proper locking or synchronisation at all between threads; just global volatiles - for anything; counters, flags, timestamps. It seems crazy and actually would be, if there were any noticeable problems - of which there are none, it is processing realtime video too! – Martin Jul 31 '14 at 20:53
0

if running is volatile... and changed on another core, you will get the updated version of running ... it may not be the next instruction, but a memory barrier will eventually happen and you will get the valid value.

So it isn't safe if you need to guarantee the order of execution, but it will work in a modern multithreaded OS for non-critical operations, like locking a UI element, or sending some network api call.

I have tried using OS X to even make the volatile even matter, and I can never get the change to not be seen:

#import <pthread.h>
bool flag;

void * secondThread(void *);

int main(int argc, const char * argv[])
{
    flag = true;
    @autoreleasepool
    {
        pthread_t thread;
        int status = pthread_create(&thread, NULL, secondThread, NULL);

        if (status)
        {
            printf("problem with pthread_create %i",errno);
        }
        sleep(4);
        flag = false;
    }
    return 0;
}


void * secondThread(void * arg)
{
    while (flag)
    {
        puts("sleeping\n");
        sleep(1);

    }
    return NULL;
}

running with -Ofast I get:

sleeping

sleeping

sleeping

sleeping

my conclusion (and maybe a bad one) is that I include volatile for semantic correctness more than I do to actually change the way the program executes.

If you are running an embedded system or RTOS I suspect it may be more important.

Grady Player
  • 14,399
  • 2
  • 48
  • 76