Event handlers not thread safe? [duplicate]

27

This question already has an answer here:

So i've read around that instead of calling a event directly with

if (SomeEvent != null)
   SomeEvent(this, null);

i should be doing 

SomeEventHandler temp = SomeEvent;
if (temp != null)
    temp(this, null);

Why is this so? How does the second version become thread safe? What is the best practice?

Daniel

출처 c# .net multithreading events thread-safety
  • Reading over the tentative, qualified answers here I get the sense that event handling in C# is tightly-coupled, error-prone, and not understood very well. - micahhoover

4 답변

14

Events are really syntactic sugar over a list of delegates. When you invoke the event, this is really iterating over that list and invoking each delegate with the parameters you have passed.

The problem with threads is that they could be adding or removing items from this collection by subscribing/unsubscribing. If they do this while you are iterating the collection this will cause problems (I think an exception is thrown)

The intent is to copy the list before iterating it, so you are protected against changes to the list.

Note: It is however now possible for your listener to be invoked even after you unsubscribed, so you should make sure you handle this in your listener code.

  • Actually, it will not cause a problem while you iterate over the collection. The delegates in play here are immutable. The only problem is the split second before checking if anyone has subscribed and actually calling the event handlers. Once you've started executing those, no background changes will affect the current invocation. - Lasse Vågsæther Karlsen
30

IMO, the other answers miss one key detail - that delegates (and therefore events) are immutable. The significance of this is that subscribing or unsubscribing an event handler doesn't simply append/remove to a list - rather, it replaces the list with a new one with an extra (or one less) item on it.

Since references are atomic, this means that at the point you do:

var handler = SomeEvent;

you now have a rigid instance that cannot change, even if in the next picosecond another thread unsubscribes (causing the actual event field to become null).

So you test for null and invoke it, and all is well. Note of course that there is still the confusing scenario of the event being raised on an object that thinks it unsubscribed a picosecond ago!

  • That addresses one concern, but is the replacement itself done safely in such a way to guarantee that all requested operations happen? I.e., if two threads try to subscribe distinct delegates at the same time, is it guaranteed that both will be subscribed in the end or is it possible for one of those subscriptions to silently fail? - binki
  • @binki yes; it uses an interlocked exchange loop (modern compiler), or a synchronized (lock) region (older compilers) - Marc Gravell♦
5

Best practice is the second form. The reason is that another thread might null or alter SomeEvent between the 'if' test and the invocation.

  • Why can't that happen in the second statement? - Daniel
  • The read is of SomeEvent is atomic, i.e. it happens all the way or nothing. Thus temp cannot be modified outside that thread due to being a local. - user7116
  • Ooops s/Thus/Also/ - user7116
2

Here is a good write up about .NET events and race conditions with threads. It covers some common scenarios and has some good references in it.

Hope this helps.

  • Thanks for the link - Daniel

Linked

Related

Latest