HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Extension methods for safely firing events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eventsextensionmethodsfiringforsafely

Problem

I wrote a set of extension methods for the EventHandler class that add the method Fire, which raises the event after creating a local reference for thread safety, and checking to make sure the event had subscribers.

``
public static class EventExtensions
{
///
/// Raises an event thread-safely if the event has subscribers.
///
/// The event handler to raise.
/// The object that sent this event.
/// The event args.
[SuppressMessage("Microsoft.Design",
"CA1030:UseEventsWhereAppropriate",
Justification = "This warning comes up when you use the word
Fire in a method name. This method specifically raises events, and so does not need changing.")]
public static void Fire(this EventHandler me, object sender, EventArgs args)
{
var handler = me;
if (handler != null)
{
handler(sender, args);
}
}

///
/// Raises an event thread-safely if the event has subscribers.
///
/// The type of EventArgs the event takes.
/// The event handler to raise.
/// The object that sent this event.
/// The event args.
[SuppressMessage("Microsoft.Design",
"CA1030:UseEventsWhereAppropriate",
Justification = "This warning comes up when you use the word
Fire in a method name. This method specifically raises events, and so does not need changing.")]
public static void Fire(this EventHandler me, object sender, T args) where T: EventArgs
{
var handler = me;
if (handler != null)
{
handler(sender, args);
}
}

///
/// Raises a static event thread-safely if the event has subscribers.
///
/// The event handler to raise.
/// The event args.
[SuppressMessage("Microsoft.Design",
"CA1030:UseEventsWhereAppropriate",
Justification = "This warning comes up when you use the word
Fire` in a method name. This method specifically raises events,

Solution

var handler = me;


is not needed - you are already guaranteed that the passed in instance can never change.

So if is null it will stay null, but if it is non-null you are also guaranteed it will stay non-null even under any kind of multithreading situation. (note that the null-check itself is still needed!)

For some further explanation:

This assignment was only needed in "pre-extension-method" times when it was common to have separate EventRaisers directly in the defining class:

event EventHandler Foo;

void OnFoo()
{
   if (Foo != null) Foo(this, EventArgs.Empty);
}


Clearly, if another thread coincidentally executed Foo -= LastHandler; just after the if block but before the Foo() call, you would run into a NullReferenceException, which was commonly avoided by adding the following assignment:

event EventHandler Foo;

void OnFoo()
{
   var temp = Foo;
   if (temp != null) temp(this, EventArgs.Empty);
}


I know that immutable reference types can be a bit mind-boggling, but it should be clear that your passed in "me" parameter can never "spontaneously" become null, even if another thread performed Foo -= LastHandler; in this mentioned "worst-case-moment".

Code Snippets

var handler = me;
event EventHandler Foo;

void OnFoo()
{
   if (Foo != null) Foo(this, EventArgs.Empty);
}
event EventHandler Foo;

void OnFoo()
{
   var temp = Foo;
   if (temp != null) temp(this, EventArgs.Empty);
}

Context

StackExchange Code Review Q#63591, answer score: 5

Revisions (0)

No revisions yet.