Events and Thread Safety

[Fred West]

I have an object with events that are subscribed to, from different threads.
My code currently looks like this:

private event MyEventHandler myEvent = null;
public event MyEventHandler MyEvent
{
add {myEvent += value;}
remove {myEvent -= value;}
}

FIRST QUESTION: Do I need to lock in the add and remove? Such as:

private myEventLock = new object();
private event MyEventHandler myEvent = null;
public event MyEventHandler MyEvent
{
add {lock(myEventLock) {myEvent += value;}}
remove {lock(myEventLock) {myEvent -= value;}}
}

I could have two threads subscribing at the same time. Thus I wonder if it's
possible one subscription could get lost if I do not lock.


SECOND QUESTION: Do I need to lock when calling the event? Such as:

lock (myEventLock) {if (myEvent!=null) myEvent();}


Or does the C# compiler handle all these details for you. Thanks,

- Fred

 

[Brian Gideon]

Fred,

The following article specifically answers both questions. It's a
short read, but very informative.

<http://www.yoda.arachsys.com/csharp/threads/lockchoice.shtml>

Brian

 

[Fred West]

The article state that the compiler locks on this for you automatically if
you declare an event without specifying the add/remove code. How do we know
this?

- Fred

 

[Brian Gideon]

Fred,

I asked the same question not too long ago. It's in section 17.7.1 of
the C# specification (ECMA version).

"When compiling a field-like event, the compiler automatically creates
storage to hold the delegate, and creates accessors for the event that
add or remove event handlers to the delegate field. In order to be
thread-safe, the addition or removal operations are done while holding
the lock (§15.12) on the containing object for an instance event, or
the type object (§14.5.11) for a static event."

Brian

 

[Fred West]

One more question:

To fire the event and prevent a possible null reference exception, is it
sufficient then to just do the following:

tempEvent = myEvent;
if (tempEvent!=null)
tempEvent();

The article suggests that you lock when assigning the tempEvent:

lock(myEventLock) tempEvent = myEvent;

if (tempEvent!=null)
tempEvent();

The reason I ask is that I am debating not using an explicit Add/Remove
accessor for my event. Thus (based on my new found knowledge) I expected the
compiler to generate one for me with the implicit lock. However, it could
either lock on "this" or a hidden instance field (according to ECMA-334). I
do not know which, so when I fire the event, a lock on this to assign the
tempEvent may be meaningless if the implementation of the compiler uses the
hidden instance field. Thanks for your help,

- Fred




Fred,

I asked the same question not too long ago. It's in section 17.7.1 of
the C# specification (ECMA version).

"When compiling a field-like event, the compiler automatically creates
storage to hold the delegate, and creates accessors for the event that
add or remove event handlers to the delegate field. In order to be
thread-safe, the addition or removal operations are done while holding
the lock (§15.12) on the containing object for an instance event, or
the type object (§14.5.11) for a static event."

Brian

 

[Jon Skeet [C# MVP]]

One more question:

To fire the event and prevent a possible null reference exception, is it
sufficient then to just do the following:

tempEvent = myEvent;
if (tempEvent!=null)
tempEvent();

That would certainly avoid a NullReferenceException. However, you might
not get the latest value of the event handling delegate.

The article suggests that you lock when assigning the tempEvent:

lock(myEventLock) tempEvent = myEvent;

if (tempEvent!=null)
tempEvent();

The reason I ask is that I am debating not using an explicit Add/Remove
accessor for my event. Thus (based on my new found knowledge) I expected the
compiler to generate one for me with the implicit lock. However, it could
either lock on "this" or a hidden instance field (according to ECMA-334). I
do not know which, so when I fire the event, a lock on this to assign the
tempEvent may be meaningless if the implementation of the compiler uses the
hidden instance field. Thanks for your help,

In C# 1.1, it's guaranteed to be locked on "this" for instance events
or the type reference for static events. Only in C# 2.0 is the compiler
permitted to lock on something else.

Personally (unsurprisingly I'd go with the extra code. Of course,
all of this only matters if you want to be thread-safe with regard to
event handlers being added/removed from other threads. If you don't
care about that, you don't need any locking at all.