14

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. :)

GazTheDestroyer
  • 20,722
  • 9
  • 70
  • 103
  • 2
    What do you expect to gain from declaring a variable `volatile`? In most cases, users of `volatile` expect it to do anything but what it actually does. – Jon Feb 23 '12 at 12:31
  • The scope in which you define the volatile is not valid. _'Local variables cannot be declared volatile.'_ Which makes sense if you [read the documentation](http://msdn.microsoft.com/en-us/library/x13ttww7(v=vs.100).aspx). – Grant Thomas Feb 23 '12 at 12:31
  • Declare your volatile local variable as a field of your class and will work. But +1 for @Jon – Arnaud F. Feb 23 '12 at 12:33
  • 2
    @Mr.Disappointment - the OP knows this - this is why he/she said "I would want to declare totalEvents as volatile, but C# does not allow this" – Rob Levine Feb 23 '12 at 12:36
  • @RobLevine That doesn't indicate that the OP knows why, and if not, knowing this could well in turn answer their own question - give a man a fish. Your quote doesn't make our knowledge of the OPs knowledge and intent any clearer. – Grant Thomas Feb 23 '12 at 12:38
  • 3
    @Mr.Disappointment - I believe the OP does know why - this is an interesting question. I think what the OP is trying to understand is this: At class level you can apply `volatile` to a field to ensure it can be read and written to safely by multiple threads (all seeing the latest value), due to certain compiler optimizations not being applied. In the case of the example above, the OP is simulating a similar situation using a closure. `totalEvents` is modifiable by multiple threads in that example. The question is "can you get that same behaviour in this scenario and if so, how?" – Rob Levine Feb 23 '12 at 12:58
  • @Jon: I expect volatile to prevent the compiler caching the value, and to give me the latest value as updated by any other thread. I was under the impression that C# guaranteed this as opposed to C volatile. – GazTheDestroyer Feb 23 '12 at 13:19
  • @RobLevine: As Mr.D said, I'm aware that I can't use it hence the question. – GazTheDestroyer Feb 23 '12 at 13:22
  • 1
    @GazTheDestroyer - I realise you do - I was saying "the OP _does_ understand this and is asking a good question" – Rob Levine Feb 23 '12 at 13:34
  • 1
    @RobLevine: Oops, sorry Rob, was getting my usernames mixed up. – GazTheDestroyer Feb 23 '12 at 13:38
  • @GazTheDestroyer: The thing is, since you are going to be concurrently using the variable you must protect it anyway. If you use either a `lock` or `Interlocked` operations, cache flushing is implicitly performed so `volatile` is not needed. I was going to give out some links, but then I saw that all this information is linked to in the answers to [a previous question of yours](http://stackoverflow.com/questions/7833462/does-locking-ensure-reads-and-writes-are-flushed-from-caches-if-so-how). TL;DR: You 're going to use a `lock` anyway. You don't need `volatile`. – Jon Feb 23 '12 at 13:52
  • @GazTheDestroyer I've updated my answer following your edit - you can use the Interlocked class to get thread safety. – Rich O'Kelly Feb 23 '12 at 16:04

5 Answers5

4

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..

Frederik
  • 2,777
  • 1
  • 20
  • 26
3

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);
   }
}
Rich O'Kelly
  • 41,274
  • 9
  • 83
  • 114
2

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();
}
spender
  • 117,338
  • 33
  • 229
  • 351
2

This won't work if events are fired in parallel. n++, unfortunately, is not an atomic operation in .NET, so you can't just expect multiple threads executing n++ 10 times to actually increase n by 10, it can be increased by less. Here's a little program that proves it (and in the process makes sure closures are properly handled when used in parallel):

class Program
{
    static volatile int _outer = 0;
    static void Main(string[] args)
    {
        int inner = 0;

        Action act_outer1 = () => _outer++; // Interlocked.Increment(ref _outer);
        Action act_inner1 = () => inner++;  // Interlocked.Increment(ref inner);
        Action<int> combined = (i) => { act_outer1(); act_inner1(); };

        Console.WriteLine("None outer={0}, inner={1}", _outer, inner);
        Parallel.For(0, 20000000, combined);
        Console.WriteLine("Once outer={0}, inner={1}", _outer, inner);
        Console.ReadKey();
    }
}

The Interlocked.Increment variant works as expected.

zmbq
  • 38,013
  • 14
  • 101
  • 171
2

Local variables captured by closures get "hoisted" out to a different class generated by the compiler, which doesn't loosen the "locals can't be volatile" rule when doing so, even though the local "really" ends up as an instance field. Your best bet is to construct the closure class by hand ("function object") or use some IL manipulation tool, eg. ILDASM.

cynic
  • 5,305
  • 1
  • 24
  • 40