Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Making variables captured by a closure volatile

How do variables captured by a closure interact with different threads? In the following example code I would want to declare totalEvents as volatile, but C# does not allow this.

(Yes I know this is bad code, it's just an example)

private void WaitFor10Events()
{
     volatile int totalEvents = 0; // error CS0106:

     _someEventGenerator.SomeEvent += (s, e) => totalEvents++;

     while(totalEvents < 10)
        Thread.Sleep(100);
}

EDIT: People seem to be missing the point of my question a bit. I know I can't use volatile on local vars. I also know that the example code code is bad and could be implemented in other ways, hence my "bad code" disclaimer. It was just to illustrate the problem.

Anyway, it would appear that there is no way to force volatile semantics onto captured local variables, so I will implement a different way. Thanks for the answers though, I have learned a couple of useful things anyway. :)

like image 254
GazTheDestroyer Avatar asked Feb 23 '12 12:02

GazTheDestroyer


3 Answers

Volatile.Write to the rescue:

private void WaitFor10Events()
{
     int totalEvents = 0; 

     _someEventGenerator.SomeEvent += (s, e) => Volatile.Write(ref totalEvents, totalEvents+1);

     while(totalEvents < 10)
        Thread.Sleep(100);
}

That said, I would still use Interlocked.Incrementfor this particular case..

like image 69
Frederik Avatar answered Nov 04 '22 07:11

Frederik


You can't declare locals volatile. Besides, there are better ways to acheive your goal... Use System.Threading.CountdownEvent instead. It's going to be more efficient than your poll/sleep method.

using(CountdownEvent cde = new CountdownEvent(10))
{
  _someEventGenerator.SomeEvent += (s, e) => cde.Signal();
  cde.Wait();
}
like image 26
spender Avatar answered Nov 04 '22 07:11

spender


It is not valid to have a local variable marked as volatile. A closure can capture volatile fields, the following is perfectly legal:

volatile int totalEvents = 0;
private void WaitFor10Events()
{
   _someEventGenerator.SomeEvent += (s, e) => totalEvents++;
   ...
}

See here for information about the volatile keyword;

As an aside, you might consider using a reset event (auto, manual), the monitor class (pulse and wait methods) or a countdown event to have a thread sleep until an event is raised, it is far more efficient than sleeping in a loop.

Update

Following on from the edit on the question, a simple way to get thread-safe semantics is to use the Interlocked class. To re-write your example in this fashion (although as stated in other answers there are better ways of writing this example):

private void WaitFor10Events()
{
   long totalEvents = 0;
   _someEventGenerator.SomeEvent += (s, e) => Interlocked.Increment(ref totalEvents);

   while(Interlocked.Read(ref totalEvents) < 10)
   {
     Thread.Sleep(100);
   }
}
like image 42
Rich O'Kelly Avatar answered Nov 04 '22 08:11

Rich O'Kelly