CODEWITHSUNDEEP
cmultithreadingthread-safetypthreadscompiler-optimization
Thiago Padilha
Thiago Padilha

Reputation: 4650

Is it safe to poll for a variable written by other threads in a C program?

Consider the following snippet of C code:

int flag = 0;
/* Assume that the functions lock_helper, unlock_helper implement enter/leave in
 * a global mutex and thread_start_helper simply runs the function in separate
 * operating-system threads */

void worker1()
{
  /* Long-running job here */
  lock_helper();
  if (!flag)
    flag = 1;
  unlock_helper();
}

void worker2()
{
  /* Another long-running job here */
  lock_helper();
  if (!flag)
    flag = 2;
  unlock_helper();
}


int main(int argc, char **argv)
{
  thread_start_helper(&worker1);
  thread_start_helper(&worker2);
  do
  {
    /* doing something */
  } while (!flag);
  /* do something with 'flag' */
}

Questions:

Upvotes: 8

Views: 1501

Answers (3)

cmaster - reinstate monica
cmaster - reinstate monica

Reputation: 40685

Since you can assume that the loading of an aligned int is an atomic operation, the only danger with your code is the optimizer: your compiler is allowed to optimize away all but the first reads of flag within main(), i. e. to convert your code into

int main(int argc, char **argv)
{
  thread_start_helper(&worker1);
  thread_start_helper(&worker2);
  /* doing something */
  if(!flag) {
      while(1) /* doing something */
  }
  //This point is unreachable and the following can be optimized away entirely.
  /* do something with 'flag' */
}

There are two ways you can make sure that this does not happen: 1. make flag volatile, which is a bad idea because it includes quite a bit of unwanted overhead, and 2. introduce the necessary memory barriers. Due to the atomicity of reading an int and the fact that you only want to interprete the value of flag after it has changed, you should be able to get away with just a compiler barrier before the loop condition like this:

int main(int argc, char **argv)
{
  thread_start_helper(&worker1);
  thread_start_helper(&worker2);
  do
  {
    /* doing something */
    barrier();
  } while(!flag)
  /* do something with 'flag' */
}

The barrier() used here is very lightweight, it is the cheapest of all barriers available.

This is not enough if you want to analyze any other data that is written before flag is raised, because you might still load stale data from memory (because the CPU decided to prefetch the value). For a comprehensive discussion of memory fences, their necessity, and their use, see https://www.kernel.org/doc/Documentation/memory-barriers.txt

Finally, you should be aware, that the other writer thread may modify flag at any time after the do{}while() loop exits. So, you should immediately copy its value to a shadow variable like this:

int myFlagCopy;
do
{
  /* doing something */
  barrier();
} while(!(myFlagCopy = flag))
/* do something with 'myFlagCopy' */

Upvotes: 0

James Hopkin
James Hopkin

Reputation: 13973

The code is likely to work as is, but is somewhat fragile. For one thing, it depends on the reads and writes to flag being atomic on the processor being used (and that flag's alignment is sufficient).

I would recommend either using a read lock to read the value of flag or use functionality of whatever threading library you are using to make flag properly atomic.

Upvotes: 2

Daniele
Daniele

Reputation: 831

It is possible that the while is executed before the threads... you have to wait the execution of thread before, using pthread_join()

Upvotes: 0

Related Questions