C# Events and thread-safety

After reading “C# Programming Language”, which is more or less a dictionary of C# language features, I noticed that the spec claims thread-safety in event default accessors.

So code defined like this:

public event SomeEvent;

compiled into something roughly as this:

   1:  private EventHandler __SomeEvent;

   2:  public event SomeEvent {

   3:      add { lock(this) { __SomeEvent += value; }}

   4:      remove { lock(this) { __SomeEvent -= value; }}

   5:  }

Actually, add and remove keywords are translated as add_SomeEvent and remove_SomeEvent methods with [MethodImpl(MethodImplOptions.Synchronized)] attributes and thus equivalent to lock(this) shown above.

However, There is a problem here to achieve thread-safety. Using [MethodImpl(..)] is a bad practice b/c your code is trying to enter a monitor for this object instance. Thus, if another thread of execution obtains the monitor first, your subsequent call to “+=” or “-=” will block and will be hard to troubleshoot.

Instead consider this pattern where you implement custom accessors locking on a private object and give caller to invoke event safely through a method:

private object l_SomeEvent;
private EventHandler __SomeEvent;
public event SomeEvent {
 add { lock(l_SomeEvent) { __SomeEvent += value; }}
 remove { lock(l_SomeEvent) { __SomeEvent -= value; }}
}
public void OnSomeEvent(EventArgs e) {
 EventHandler temp;
 lock(l_SomeEvent) { temp = __SomeEvent; }
 if (temp != null) { __SomeEvent(this,e); }
}