views:

229

answers:

5

On most common platforms (the most important being x86; I understand that some platforms have extremely difficult memory models that provide almost no guarantees useful for multithreading, but I don't care about rare counter-examples), is the following code safe?

Thread 1:

someVariable = doStuff();
atomicSet(stuffDoneFlag, 1);

Thread 2:

while(!atomicRead(stuffDoneFlag)) {}  // Wait for stuffDoneFlag to be set.
doMoreStuff(someVariable);

Assuming standard, reasonable implementations of atomic ops:

  1. Is Thread 1's assignment to someVariable guaranteed to complete before atomicSet() is called?
  2. Is Thread 2 guaranteed to see the assignment to someVariable before calling doMoreStuff() provided it reads stuffDoneFlag atomically?

Edits:

  1. The implementation of atomic ops I'm using contains the x86 LOCK instruction in each operation, if that helps.
  2. Assume stuffDoneFlag is properly cleared somehow. How isn't important.
  3. This is a very simplified example. I created it this way so that you wouldn't have to understand the whole context of the problem to answer it. I know it's not efficient.
A: 

1)Yes

2)Yes

Both work.

Gandalf
A: 

This code looks thread safe, but I question the efficiency of your spinlock (the while loop) unless you are only spinning for a very short amount of time. There is no guarantee on any given system that Thread 2 won't completely hog all processing time.

I would recommend using some actual synchronization primitives (looks like boost::condition_variable is what you want here) instead of relying on the spin lock.

Nick Haddad
The spin-lock approach could be made nicer by sleeping for a small interval in the while() body in thread 2.
Will
A: 

The atomic instructions ensure that the thread 2 waits for thread 1 to complete setting the variable before thread 2 proceeds. There are, however, two key issues:

1) the someVariable must be declared 'volatile' to ensure that the compiler does not optimise it's allocation e.g. storing it in a register or deferring a write.

2) the second thread is blocking while waiting for the signal (termed spinlocking). Your platform probably provides much better locking and signalling primatives and mechanisms, but a relatively straightforward improvement would be to simply sleep() in the thread 2's while() body.

Will
A: 

dsimcha written: "Assume stuffDoneFlag is properly cleared somehow. How isn't important." This is not true!

Let's see scenario:

  1. Thread2 checks the stuffDoneFlag if it's 1 start reading someVariable.
  2. Before the Thread2 finish reading the task scheduler interrupt its task and suspend the task for some time.
  3. Thread1 again access to the someVariable and change the memory content.
  4. Task scheduler switch on again Thread2 and it continue the job but memory content of someVariable is changed!
GJ
+1  A: 

If your actual x86 code has the store to someVariable before the store in atomicSet in Thread 1 and load of someVariable after the load in atomicRead in Thread 2, then you should be fine. Intel's Software Developer's Manual Volume 3A specifies the memory model for x86 in Section 8.2, and the intra-thread store-store and load-load constraints should be enough here.

However, there may not be anything preventing your compiler from reordering the instructions generated from whatever higher-level language you are using across the atomic operations.

Tuure Laurinolli
Thanks. In D (the high level language I was using), there is no code motion across inline assembler statements, and atomic ops are implemented using inline assembler.
dsimcha