Thread-safe events in C # or John Skeet vs. Jeffrey Richter

I was preparing for an interview on C # and, among other things, I found a question similar to the following:

"How to organize a thread-safe event call in C #, taking into account the fact that a large number of threads constantly subscribe to the event and unsubscribe from it?"

The question is quite specific and clearly posed, so I didn’t even doubt that the answer to it can be given also clearly and unequivocally. But I was very wrong. It turned out that this is an extremely popular, hackneyed, but still open topic. And I also noticed a not very pleasant feature - in Russian-speaking resources very little attention is paid to this issue (and Habr is no exception), so I decided to collect all the information I found and digested on this issue.
We will also reach John Skeet and Jeffrey Richter, they, in fact, played a key role in my general understanding of the problem of how events work in a multi-threaded environment.
')
A particularly attentive reader will be able to find two xkcd-style comics in the article.
(Be careful, inside there are two pictures of about 300-400 kb each)


I duplicate the question you need to answer:

"How to organize a thread-safe event call in C #, taking into account the fact that a large number of threads constantly subscribe to the event and unsubscribe from it?"

I had an assumption that some of the questions relied on the book CLR via C # , especially since my favorite C # 5.0 in a Nutshell did not address this question at all, so let's start with Jeffrey Richter (CLR via C #).

Jeffrey Richter's Way

A short excerpt from written:

For a long time, the recommended method for triggering events was approximately the following construction:

Option 1:

public event Action MyLittleEvent; ... protected virtual void OnMyLittleEvent() { if (MyLittleEvent != null) MyLittleEvent(); } 

The problem with this approach is that in the OnMyLittleEvent method OnMyLittleEvent one thread can see that our MyLittleEvent event MyLittleEvent not null , and another thread immediately after this check, but before calling the event, can remove its delegate from the list of subscribers, and thus make our event is MyLittleEvent null , which will NullReferenceException at the place where the event was NullReferenceException .

Here is a small xkcd-style comic that clearly illustrates this situation (two threads work in parallel, time goes from top to bottom):


In general, everything is logical, we have the usual race condition (hereinafter, race condition). And this is how Richter solves this problem (and this option is most common):

Let's add a local variable to our event call method, into which we will copy our event at the time of “calling” to the method. Since delegates are immutable objects (hereinafter referred to as immutable), we will have a “frozen” copy of an event that no one else can unsubscribe from. When you unsubscribe from an event, a new delegate object is created that replaces the object in the MyLittleEvent field, while we still have a local reference to the old delegate object.

Option 2:

 protected virtual void OnMyLittleEvent() { Action tempAction = MyLittleEvent; // ""      //   tempAction     ,      null     if (tempAction != null) tempAction (); } 

Further, Richter describes that the JIT compiler may well simply omit the creation of a local variable for the sake of optimization, and make the first of the second option, that is, skip the “freezing” of the event. As a result, it is recommended to do copying through Volatile.Read(ref MyLittleEvent) , that is:

Option 3:

 protected virtual void OnMyLittleEvent() { //      Action tempAction = Volatile.Read(ref MyLittleEvent); if (tempAction != null) tempAction (); } 

You can talk about Volatile separately for a long time, but in general it “just allows you to get rid of unwanted JIT compiler optimization”. On this occasion, there will be more clarifications and details, but for now we will dwell on the general idea of ​​the current decision of Jeffrey Richter:

To ensure a thread-safe event call, you need to “freeze” the current list of subscribers by copying the event into a local variable, and then, if the resulting list is not empty, call all handlers from the frozen list. Thus we get rid of possible NullReferenceException.

I was immediately embarrassed by the fact that we are triggering events on already unsubscribing objects / threads . It is unlikely that someone unsubscribed just like that - it is likely that someone did it during the general “cleaning” of traces - along with closing the write / read streams (for example, the logger who was supposed to write data to the file), closing the connections etc., that is, the internal state of the subscriber object at the time its handler is called may not be suitable for further work.
For example, imagine that our subscriber implements the IDisposable method, and follows a convention that determines that when trying to call any method on an object freed (hereinafter - disposed), it should throw an ObjectDisposedException . Let's also agree that we unsubscribe from all events in the Dispose method.
And now imagine such a scenario - we call the Dispose method on this object exactly after the moment when another thread “froze” its list of subscribers. The thread successfully calls the handler for the unsubscribing object, and during the attempt to process the event, the object sooner or later realizes that it has already been released, and throws an ObjectDisposedException . Most likely, this exception is not caught in the handler itself, because it is logical to assume: “If our subscriber unsubscribed and was released, then his handler will never be called.” There will either crash the application, or the leak of uncontrolled resuses, or the event call will be interrupted when ObjectDisposedException first appears (if we catch an exception when calling), but the event will not get to the normal "live" handlers.

Let's go back to the comics. The story is the same - two streams, time goes from the top down. This is what actually happens:


This situation, in my opinion, is much more serious than a possible NullReferenceException when invoking an event.
Interestingly, there are tips for implementing a thread-safe event call on the side of the Observed Object, and tips for implementing thread-safe Handlers are not.

What StackOverflow Says

On SO, you can find a detailed “article” (yes, this question draws on a whole small article) on this issue.

In general, my point of view is divided there, but this comrade adds:

It seems to me that all this hype with local variables is nothing more than Cargo Cult Programming . A large number of people solve the problem of thread-safe events in this way, while for full thread safety, much more needs to be done. I can say with confidence that those people who do not add such checks to their code can do without them. This problem simply does not exist in a single-threaded environment, and given that in the online examples with code you can rarely find the keyword volatile , this additional check may well be meaningless. If our task is to track a NullReferenceException , is it not possible to do without checking for null at all by assigning an empty delegate { } our event during the initialization of the class object?

This brings us to another solution.

 public event Action MyLittleEvent = delegate {}; 

MyLittleEvent will never be null , and you can simply not do an extra check. In a multithreaded environment, you only need to synchronize the addition and removal of event subscribers, but you can call it without fear of getting a NullReferenceException :

Option 4:

 public event Action MyLittleEvent = delegate {}; protected virtual void OnMyLittleEvent() { // ,   MyLittleEvent(); } 

The only disadvantage of this approach compared with the previous one is a small overhead on the challenge of an empty event (the overhead projector turned out to be approximately 5 nanoseconds per challenge). You might also think that in the case of a large number of different classes with different events, these empty “gags” for events will take up a lot of space in RAM, but according to John Skit, in response to SO , starting with version C # 3.0, the compiler uses the same the same empty delegate object for all gags. From myself I’ll add that when checking the resulting IL code this statement is not confirmed, empty delegates are created bit by bit on the event (checked using LINQPad and ILSpy). In a pinch, you can make a static field common to a project with an empty delegate that can be accessed from all program sections.

The Way of John Skit

Since we got to John Skit, it is worth noting his implementation of thread-safe events, which he described in C # in Depth in the Delegates and Events section ( online article and translation of Comrade Klotos )

The bottom line is to close add , remove and local “freeze” in lock , which will get rid of possible uncertainties while simultaneously subscribing to the event of several threads:



Despite the fact that this method is considered obsolete (the internal implementation of events starting from C # 4.0 looks completely different, see the list of sources at the end of the article), it clearly shows that you can’t just wrap an event call, a subscription and a reply in lock , because this is very likely to result in a deadlock (hereafter, deadlock). In lock only copying to a local variable is found; the event itself is called outside this structure.

But this does not completely solve the problem of calling handlers for already unsubscribing events.

Let's return to the question on SO. Daniel, in response to all our ways of preventing a NullReferenceException gives a very interesting thought:

Yes, I really figured out this advice about trying to prevent a NullReferenceException at all costs. I say that in our particular case, a NullReferenceException can occur only if another thread unsubscribes from the event. And he does this only to never receive events again , which we, in fact, do not achieve when using checks of local variables. Where we hide the race condition we can open it and correct the consequences. NullReferenceException allows NullReferenceException to determine the moment of mishandling your event. In general, I argue that this copying and verification technique is a simple Cargo-cult programming that adds confusion and noise to your code, but does not solve the problem of multi-threaded events at all.

Among others, John Skit answered the question, and this is what he writes.

John Skeet vs. Jeffrey Richter

The JIT compiler does not have the right to optimize the local reference to the delegate, because there is a condition. This information was “thrown in” some time ago, but this is not true (I clarified this question whether Joe Duffy or Vance Morrison). Without the volatile , the possibility simply arises that the local reference to the delegate will be a bit outdated, but in general this is all. This will not result in a NullReferenceException .

And yes, we definitely have a race condition, you are right. But it will always be present. Suppose we remove the check for null and just write

 MyLittleEvent(); 

Now imagine that our list of subscribers consists of 1000 delegates. It is possible that we will start to raise an event before one of the subscribers unsubscribes from it. In this case, it will still be called, since it will remain in the old list (do not forget that delegates are immutable). As I understand it, it is completely inevitable.
Using empty delegate {}; it saves us from having to check the event for null , but this will not save us from the next race condition. Moreover, this method does not guarantee that we will use the most recent version of the event.

Now it should be noted that this answer was written in the 2009th year, and CLR via C # 4th edition - in 2012. So who will you believe in the end?
In fact, I did not understand why Richter described the case of copying to a local variable through Volatile.Read , since he further confirms the words of Skit:

Although it is recommended to use the version with Volatile.Read as the best and technically correct, you can get by with Option 2 , since the JIT compiler knows that it can accidentally mess up by optimizing the local variable tempAction . Theoretically , in the future this may change, therefore it is recommended to use Option 3 . But in fact, Microsoft is unlikely to make such changes, because it can break a huge number of ready-made programs.

Everything becomes completely confusing - both options are equivalent, but the one with Volatile.Read more equivalent. And no option will save from the race condition when calling unsubscribing handlers.

Maybe there is no thread safe way to trigger events at all? Why NullReferenceException it NullReferenceException so much time and effort to prevent an unlikely NullReferenceException , but not to prevent an equally probable call to the unsubscribing handler? That I did not understand. But in the process of searching for answers, I understood a lot of other things, and here is a small result.

What we have in the end

• The most popular method is not thread - safe because of the possibility of the delegate becoming null after checking for inequality. There is a danger of NullReferenceException
  

   public event Action MyLittleEvent; ... protected virtual void OnMyLittleEvent() { if (MyLittleEvent != null) //  NullReferenceException MyLittleEvent(); } 

  
  
• The methods of Skit and Richter help to avoid the occurrence of a NullReferenceException , but are not thread-safe , since the probability remains to call the already unsubscribing handlers.
  
  Skete method

   SomeEventHandler someEvent; readonly object someEventLock = new object(); public event SomeEventHandler SomeEvent { add { lock (someEventLock) { someEvent += value; } } remove { lock (someEventLock) { someEvent -= value; } } } protected virtual void OnSomeEvent(EventArgs e) { SomeEventHandler handler; lock (someEventLock) { handler = someEvent; } if (handler != null) { handler (this, e); } } 

  
  
  
  
  Richter Method

   protected virtual void OnMyLittleEvent() { //      Action tempAction = Volatile.Read(ref MyLittleEvent); if (tempAction != null) tempAction (); } 

  
  
  
  
• Empty delegate {}; method delegate {}; allows you to get rid of NullReferenceException due to the fact that the event never goes to null , but is not thread-safe because the likelihood of invoking unsubscribing handlers remains. Moreover, without the volatile , we have the opportunity to get not the latest version of the delegate when invoking an event.
  
• You cannot simply wrap an add, delete, and raise event in a lock , since this will create a deadlock hazard. Technically, it can save unsubscribing handlers from being called, but we cannot be sure about what actions the subscriber object did before unsubscribing from an event, so we can still run into a “damaged” object (see the example with ObjectDisposedException ). This method is also not thread safe .
  
• An attempt to catch the unsubscribing delegates after the local “freezing” of the event is meaningless - with a large number of subscribers, the probability of calling the unsubscribing handlers (after the start of the event call) is even higher than with the local “freezing”.
  

Technically, none of the options presented is a thread - safe way to trigger an event. Moreover, adding a delegate verification method using local delegate copies creates a false sense of security . The only way to completely secure yourself is to force event handlers to check if they have already unsubscribed from a specific event. Unfortunately, unlike the common practice of preventing NullReferenceException when NullReferenceException events, there are no prescriptions about handlers. If you make a separate library, then most often you cannot influence its users in any way - you cannot force customers to assume that their handlers will not be called after the formal reply from the event.

After realizing all these problems, I had mixed feelings about the internal implementation of delegates in C #. On the one hand, since they are immutable, there is no chance to get an InvalidOperationException as in the case of InvalidOperationException changing collection through foreach , but on the other, there is no way to check if someone has unsubscribed from the event during a call or not. The only thing that can be done by the event holder is to protect against a NullReferenceException and hope that the subscribers will not spoil anything. As a result, the question can be answered as follows:

It is impossible to provide a thread-safe event call in a multi-threaded environment, since there always remains the probability of calling the handlers of already unsubscribing subscribers. This uncertainty contradicts the definition of the term “thread safety”, in particular clause

It is guaranteed that it can be used simultaneously.

Additional reading

Of course, I could not just copy / translate everything I found. Therefore, I will leave a list of sources that have been directly or indirectly used.

• CLR via C # (Jeffrey Richter)
• A question on SO, in which John Skit talks about the inevitability of a race condition. Still there are results of comparative tests at the time of the event call
• A small article that clearly and weightedly explains the shortcomings of various methods of triggering events (outdated)
• C # in Depth - Delegates and Events (Jon Skeet)
• Useful article with the statement that ensuring thread safety is the work of handlers.
• Another comparative analysis of the time of event calls using different approaches
• Details on empty delegates
• A little bit about Memory Barrier and Volatile (Joe Albahari)
• More about volatile
• On atomicity of operations with reference types
• Extension methods for pseudo-thread-safe event firing
• C # 5.0 in a Nutshell (for now, a favorite book on C #, highly recommended)