Does an EventWaitHandle have any implicit MemoryBarrier?

Question

New to this website, so let me know if I'm not posting in an accepted manner.

I've frequently coded something along the lines of the sample below(with stuff like Dispose ommited for clarity. ). My question is, are the volatiles needed as shown? Or does the ManualResetEvent.Set have an implicit memory barrier as I've read Thread.Start does? Or would an explicit MemoryBarrier call be better than the volatiles? Or is it completely wrong? Also, the fact that the "implicit memory barrier behavior" in some operations is not documented as far as I've seen is quite frutrating, is there a list of these operations somewhere?

Thanks, Tom

:

class OneUseBackgroundOp
{

   // background args
   private string _x;
   private object _y;
   private long _z;

   // background results
   private volatile DateTime _a
   private volatile double _b;
   private volatile object _c;

   // thread control
   private Thread _task;
   private ManualResetEvent _completedSignal;
   private volatile bool _completed;

   public bool DoSomething(string x, object y, long z, int initialWaitMs)
   {
      bool doneWithinWait;

      _x = x;
      _y = y;
      _z = z;

      _completedSignal = new ManualResetEvent(false);

      _task = new Thread(new ThreadStart(Task));
      _task.IsBackground = true;
      _task.Start()

      doneWithinWait = _completedSignal.WaitOne(initialWaitMs);

      return doneWithinWait;

   }

   public bool Completed
   {
      get
      {
         return _completed;
      }
   }

   /* public getters for the result fields go here, with an exception
      thrown if _completed is not true; */

   private void Task()
   {
      // args x, y, and z are written once, before the Thread.Start
      //    implicit memory barrier so they may be accessed freely.

      // possibly long-running work goes here

      // with the work completed, assign the result fields _a, _b, _c here

      _completed = true;
      _completedSignal.Set();

   }

}

Answer 1

Based on what you've shown, I would say that, no, the volatiles are not required in that code.

The ManualResetEvent itself doesn't have an implicit memory barrier. However, the fact that the main thread is waiting for the signal means that it can't modify any variables. At least, it can't modify any variables while it's waiting. So I guess you could say that waiting on a synchronization object is an implicit memory barrier.

Note, however, that other threads, if they exist and have access to those variables, could modify them.

From your question, it seems that you're missing the point of what volatile does. All volatile does is tell the compiler that the variable might be modified by other threads asynchronously, so it shouldn't optimize code that accesses the variable. volatile does not in any way synchronize access to the variable.

Answer 2

Note that this is off the cuff, without studying your code closely. I don't think Set performs a memory barrier, but I don't see how that's relevant in your code? Seems like more important would be if Wait performs one, which it does. So unless I missed something in the 10 seconds I devoted to looking at your code, I don't believe you need the volatiles.

Edit: Comments are too restrictive. I'm now referring to Matt's edit.

Matt did a good job with his evaluation, but he's missing a detail. First, let's provide some definitions of things thrown around, but not clarified here.

A volatile read reads a value and then invalidates the CPU cache. A volatile write flushes the cache, and then writes the value. A memory barrier flushes the cache and then invalidates it.

The .NET memory model ensures that all writes are volatile. Reads, by default, are not, unless an explicit VolatileRead is made, or the volatile keyword is specified on the field. Further, interlocked methods force cache coherency, and all of the synchronization concepts (Monitor, ReaderWriterLock, Mutex, Semaphore, AutoResetEvent, ManualResetEvent, etc.) call interlocked methods internally, and thus ensure cache coherency.

Again, all of this is from Jeffrey Richter's book, "CLR via C#".

I said, initially, that I didn't think Set performed a memory barrier. However, upon further reflection about what Mr. Richter said, Set would be performing an interlocked operation, and would thus also ensure cache coherency.

I stand by my original assertion that volatile is not needed here.

Edit 2: It looks as if you're building a "future". I'd suggest you look into PFX, rather than rolling your own.

Answer 3

First, I'm not sure if I should "Answer my own question" or use a comment for this, but here goes:

My understanding is that volatile prevents code/memory optimizations from moving the accesses to my result variables (and the completed boolean) such that the the thread that reads the result will see upt-to-date data.

You wouldn't want the _completed boolean made visible to all threads after the Set() due to compiler or emmpry optimaztions/reordering. Likewise, you wouldn't want the writes to the results _a, _b, _c being seen after the Set().

EDIT: Further explaination/clarification on the question, in regards to items mentioned by Matt Davis:

Finally, i should point out that whenever a thread calls an interlocked method, the CPU forces cache coherency. So if you are manipulating variables via interlocked methods, you do not have to worry about all of this memory model stuff. Furthermore, all thread synchronization locks (Monitor, ReaderWriterLock, Mutex, Semaphone, AutoResetEvent, ManualResetEvent, etc.) call interlocked methods internally.

So it would seem that, as wekempf pointed out, that the result variables do not require the volatile keyword in the example as shown since the ManualResetEvent ensures cache coherency.

So you are both in agreement that such an operation takes care of caching between processors or in registers etc.

But does it prevent reording to guarantee such that BOTH the results are assigned before the completed flag, and that the completed flag is assigned true before the ManualResetEvent is Set?

First, my initial assumption was that the background thread would potentially run multiple times. I obviously overlooked the name of the class (OneUseBackgroundOp)! Given that it is only run once, it is not clear to me why the DoSomething() function calls WaitOne() in the manner that it does. What is the point of waiting initialWaitMs milliseconds if the background thread may or may not be done at the time DoSomething() returns? Why not just kickoff the background thread and use a lock to synchronize access to the results variables OR simply execute the contents of the Task() function as part of the thread that calls DoSomething()? Is there a reason not to do this?

The concept of the sample is to execute a possibly long-runnig task. If the task can be completed within an exceptable amount of time, then the calling thread will get access to the result and continue with normal processing. But sometime a task can take quite a long time and the claiing thread cannot be blocked for that period and can take reasonable steps to deal with that. That can include checking back later on the operation using the Completed property.

A concrete example: A DNS resolve is often very quick (subsecond) and worth waiting for even from a GUI, but sometimes it can take many many seconds. So by using a utility class like the sample, one could gets a result easily from the point-of-view of the caller 95% of the time and not lock up the GUI the other 5%. One could use a Background worker, but that can be overkill for an operation that the vast majority of the time doesn't need all that plumbing.

Second, it seems to me that not using some kind of locking mechanism on the results variables is still a bad approach. True, it is not needed in the code as shown.

The result (and completed flag) data is meant to be write-once, read-many. If I added a lock to assign the results and flag, I'd also have to lock in my result getters, and I never liked seeing getters lock just to return a data point. From my reading, such fine-grained locking is not appropriate. If an operation has 5 or 6 results, the caller has to take and release the lock 5 or 6 times needlessly.

But at some point down the road, another thread may come along and try to access the data. It would be better in my mind to prepare for this possibility now rather than try to track down mysterious behavior anomalies later.

Because I have a volatile completed flag that is guarenteed to be set before the volatile results are, and the only access to the results is through the getters, and as mentioned in the smaple, an exception is thrown if the getter is called and the operation is not yet complete, I'd expect that the Completed and result getters CAN be invoked by a thread other than the one that called DoSomething(). That's my hope anyway. I believe this to be true with the volatiles anyway.