4

Studying multi-threading from albahari atricle. Do I need a lock _locker in the sample below? I suppose no, since _message is protected by EventWaitHandle. Am I right?

class TwoWaySignaling
{
  static EventWaitHandle _ready = new AutoResetEvent (false);
  static EventWaitHandle _go = new AutoResetEvent (false);
  static readonly object _locker = new object();
  static string _message;

  static void Main()
  {
    new Thread (Work).Start();

    _ready.WaitOne();                  // First wait until worker is ready
    lock (_locker) _message = "ooo";
    _go.Set();                         // Tell worker to go

    _ready.WaitOne();
    lock (_locker) _message = "ahhh";  // Give the worker another message
    _go.Set();
    _ready.WaitOne();
    lock (_locker) _message = null;    // Signal the worker to exit
    _go.Set();
  }

  static void Work()
  {
    while (true)
    {
      _ready.Set();                          // Indicate that we're ready
      _go.WaitOne();                         // Wait to be kicked off...
      lock (_locker)
      {
        if (_message == null) return;        // Gracefully exit
        Console.WriteLine (_message);
      }
    }
  }
}
helb
  • 7,609
  • 8
  • 36
  • 58
vico
  • 17,051
  • 45
  • 159
  • 315
  • 2
    I can't see a way in which removing the `lock` could change the order of execution in this example. – Rotem Nov 27 '17 at 09:52
  • 3
    No lock needed, the ping-ponging between the two AutoResetEvents provides the synchronization, ensuring that the threads cannot access the variable at the same time. The WaitOne() call provides a memory barrier, ensuring that the variable update is visible to another thread. These kind of accidental barriers are not very pretty but not uncommon. You'd strongly favor using the Barrier class, its SignalAndWait() method makes this code much easier to understand. And is a lot more efficient. Code that you can understand avoids threading bugs, efficient code makes everybody happy. – Hans Passant Nov 27 '17 at 11:30

2 Answers2

4

You are correct.

These kind of problems cannot be simply tested with trial and error. They are best analyzed with logical thinking:

  • Which thread runs which code?
  • What could happen in the other thread right now?
  • What happens if the other thread gets more CPU time? etc.

The main thread will only execute the code in Main() and the worker thread will only execute the code in Work(). So that's simple enough.

If you look at how the critical resource is accessed you will note that access to _message in Main() is always between

_ready.WaitOne();

and

_go.Set();

whereas access to _message in Work() is always between

_go.WaitOne();

and

_ready.Set();

Therefore, one of the threads will always wait for the other before accessing _message and the lock is not needed.

helb
  • 7,609
  • 8
  • 36
  • 58
0

You stopped your threads from accessing the _message variable at the same time. Your logic here is self sufficient and predictable, which is not always the case. sometimes you’ll read from other places and the flow is not fully controllable, in which case you'll need to lock critical variables.

Majid ALSarra
  • 394
  • 2
  • 7