Understanding C#: Raising events using a temporary variable

By Andrew Stellman
September 9, 2010 | Comments: 2

We've been having a lively discussion about C# best practices on the Head First C# forum. One of our readers posted a question about the best practice for raising events inside a class:

I have question on C# Best Practices. I raise events so:

class Plane 
{
    public event EventHandler Land;

    protected void OnLand()
    {
        if ( null != Land ) 
        {
            Land( this, null );
        }
    }
}

I am told it is event handler best practice to do instead:

EventHandler temp = Land;
if ( null != temp ) 
{
    temp( this, null );
}

Is that truly necessary? In what case could temp be different from Land?

Head First C# Cover

A lot of C# developers notice that there's something odd about how we normally raise events in C#. We're always told to set a temporary variable equal to the event first, and then raise the event using that variable. It looks very strange—how could that variable do anything at all? But it turns out that there's a very good reason for using the temporary variable, and understanding that reason can help you become a better C# developer. This post shows a quick example of why you need that variable.

Always use a temporary reference when raising C# events

The answer to the reader's question is, yes, there is a really good reason to use the temporary variable. But before we go any further, I want to be really clear about about the right way to do things, and make sure we're on the same page when it comes to terminology. Here's the pattern to use for raising events in C#. If you have an event called EventToBeRaised using event arguments stored in a variable called eventArgs, you should raise the event with code that follows a pattern like this:

EventHandler temporaryVariable = EventToBeRaised;
if (temporaryVariable ! = null)
    temporaryVariable(this, eventArgs);

If you're used to writing code that looks like that, then take a minute and step back, and try to look at it through fresh eyes. That code looks really odd. Could you imagine if you had to use regular old value types this way? What if instead of i *= 3; you had to do this:

int temp = i;
temp *= 3;
i = temp;

That would be crazy, right? So I think it's really important to take a step back and recognize how odd the temporary variable pattern looks to someone who's never encountered it before.

That said, it's definitely important. I'll explain why you need it, and give you an example of how your code can break if you don't use it.

(A lot of novice and intermediate developers get a little nervous when they read about delegates and references to events, so I'll try to use language that is easy for novice and intermediate developers to absorb easily. If you want a really good technical explanation of what's going on, I recommend reading Eric Lippert's excellent post called Events and Races.)

Why you need to use temporary variables when raising events

The difficulty with event handlers starts with two basic facts about events and event handlers:

  • When your object tries to raise an event, if there are no event handlers added to it then it will throw a NullReferenceException.
  • If you've got an object with a public event, then any other object can add or remove an event handler to that event at any time.

It's the "at any time" bit that turns out to be a problem. If you only write single-threaded code, then you don't normally think about the sorts of trouble that pesky "at any time" can cause with your code. If you write a method, you expect that the first statement executes, then the second, then the third, etc.

But when you've got multithreaded code—code that uses classes in the System.Threading to execute more than one method at the same time—then you can run into a serious problem. Have another look at the code from the top of this post that you're not supposed to use:

if ( null != Land ) 
{
    Land( this, null );
}

That code could execute just fine the first 100,000 times, even in a multithreaded environment. But if the threads line up just right, you could see this problem:

  1. On thread #1, the first line executes: if ( null != Land )
  2. Land has an event handler attached to it, so null != Land returns true
  3. Meanwhile, on thread #2, the object that has its event handler attached to Land removes it from Land
  4. But thread #1 already did its null check, so it moves on to the next statement
  5. On thread #1, the next line executes: Land( this, null );
  6. But the Land event no longer has any event handlers attached to it, so it throws a NullReferenceException

That's where the temporary variable comes in. If you do this instead:

EventHandler temp = Land;
if ( null != temp ) 
{
    temp( this, null );
}

then in step #4, after the null check happens, instead of calling Land( this, null ) thread #1 calls temp( this, null );. The temp variable is keeping track of all of the event handlers that were attached to Land two statements earlier, so that statement won't throw a NullReferenceException.

If you're new to threading, this is a really good example of the kind of trap that you can step into. This particular problem is an example of a race condition. A race condition is a bug that happens when your code is dependent on a certain sequence or timing of statements that execute. It's way too easy to write multithreaded code that looks like it works but has nasty bugs that only happen occasionally and are very hard to reproduce. You can learn more about race conditions and how to prevent them on the Managed Threading Best Practices page on MSDN.

If you want to get a gentle, much safer introduction to threading, have a look at the BackgroundWorker class. BackgroundWorker is an easier way to create multithreaded programs in a way that avoids a lot of pitfalls. Our Head First C#, 2nd Edition readers can learn more about BackgroundWorker in leftover #3 in the Head First C# appendix. If you're using the first edition or not a Head First C# reader at all, have a look at my BackgroundWorker tutorial to learn more.

Where the incorrect event handler pattern breaks down

That all might seem very theoretical and not particularly useful. After all, if you've never used temporary variables when raising events and your code hasn't broken yet, is it really a problem?

That's why I wanted to take the broken code that was originally posted to the forum and turn it into a really simple cautionary example.

Here's a simple console application that uses the Plane class from the post to generate a race condition that throws a NullReferenceException:

Program.cs:

using System;
using System.Collections.Generic;
using System.Threading;

class Plane
{
     public event EventHandler Land;
 
     protected void OnLand()
     {
          if (null != Land)
          {
               Land(this, null);
           }
      }
 
     public void LandThePlane()
     {
          OnLand();
      }
}

class Program
{
     static void Main(string[] args)
     {
          Plane p = new Plane();
          ParameterizedThreadStart start = new ParameterizedThreadStart(Run);
          Thread thread = new Thread(start);
          thread.Start(p);
  
          while (true)
          {
               p.LandThePlane();
           }
      }
 
     static void Run(object o)
     {
          Plane p = o as Plane;
          while (p != null)
          {
               p.Land += p_Land;
               p.Land -= p_Land;
           }
      }
 
     static void p_Land(object sender, EventArgs e)
     {
          return;
      }
}

Here's what it looks like when I paste it into a file using Notepad, compile it using CSC, and run it.

Screenshot - Event with broken threading.png

Did you notice that I gave CSC the argument /platform:x86? That's because when I compile it for my native 64-bit machine, the way the CLR handles the threads seems to suppress this bug. To me, that actually makes this bug worse! Think about how frustrating it would be if someone running your program on a 32-bit machine reported a bug, but you couldn't reproduce it because you were running on a 64-bit machine.

Even if this makes sense, it's really instructive to actually debug through it yourself in Visual Studio and see exactly what's going on. Here's how to do it:

  1. Open up Visual Studio 2010 and create a new Console Application project.
  2. Open up Program.cs and paste in the code above.
  3. Put a breakpoint on the line Land( this, null );
  4. Press F5 to start the debugger. It might take a few seconds before the breakpoint triggers, or it might happen right away—that's the thing about multithreaded code, it's a little unpredictable.
  5. Notice how the line p.Land -= p_Land; (or another line near it) has a gray background? That's the line that's currently executing on the other thread.
  6. Place a breakpoint on the statement p.Land -= p_Land;.
  7. Press F10 to step over the current statement. If the debugger doesn't break on your other breakpoint, press F5 to continue and try again.
  8. Use F10 (step over) and F11 (step into) to keep stepping through the code. This should help you get a feel for how the threads run simultaneously.

Note that when you step through multithreaded code in the debugger, it has to interleave (or dovetail) the executing statements, executing one after another rather than running them simultaneously like it does when the program is running outside the debugger. It should be clear why it needs to do this—otherwise, there wouldn't be any way for the debugger to run. But this should be enough to give you an idea of what's going on.

There's still a potential problem!

There's one more thing. If you replace the code that raises the Land event with code that uses a temporary variable, it prevents the NullReferenceException. But there's still a race condition! The way it prevents the race condition is by using the temporary reference to keep track of all of the event handlers hooked up to the event. That way, if another thread unhooks its event handler and causes the event to become null, the temporary reference will still keep track of it and call the event handler. But do you see the problem?

The problem is that the event handler is being called after the it was unhooked from the event. You typically expect that once you use -= to remove an event handler from an event, it will never get called when that event is raised. But if this is happening in a multithreaded program, there's a very small—but nonzero—period of time that the event handler will get called after it's removed from the event. This may or may not be a problem depending on how your program is written. But it could lead to a bug that's devilishly difficult to reproduce.

This is a good example of why writing multithreaded code is harder than it looks!

Andrew Stellman is the author of Head First C# and other books from O'Reilly. You can read more from Andrew at Building Better Software.


You might also be interested in:

2 Comments

Awesome, I never knew about this. Really well explained.

Hi Andrew, thanks for underlining that my old habit to always raise events with a temporary variable has a reason. :-) Alas, for a long time it was still not clear to me why it is like this...

Isn't the temp variable just another object reference to the one and only EventHandler pool of event callback hooks that Land does also point to? Why does unhooking an event callback with -= affect the Land but *not*, at the very same moment in time, also affect the temp variable/reference?
Contrary to that, if I have two variables PersonA and PersonB pointing to the same instance of a Person class, setting a 'Name' property on any of them would implicitly and thereby *immediately* change the 'Name' value of the other, as there -strictly speaking- is only one 'Name' field at all. What is the difference to the EventHandler delegate here?

The answer lies in the 'event' keyword and is explained in Eric Lippert's Blog post you mentioned: multi-cast delegates are immutable. That is in your example, Land and temp are not pointers to the same memory all the time. As soon as -= happens on Land, the modified contents are copied to another instance of EventHandler delegates and from then on, temp points to a different, "old" version that Land does no longer reference. And thus, the complete story suddenly makes sense.
Nice. :-)

News Topics

Recommended for You

Got a Question?