deft flux

A portal into the creative workings of David Meyer

About the author

David Meyer (a.k.a. "deft flux") is a software developer fluent in C#, C++, Java and others. He also programs in his spare time and enjoys playing instruments and making electronic music.
E-mail me Send mail

Recent comments

Authors

Disclaimer

The opinions expressed by the author are his alone and do not represent any other person or organization unless stated otherwise. The opinions given in comments are solely those of the person who wrote the comment and are not necessarily the opinions of the author of this site. The author of a post is not responsible for the content of its comments.

© Copyright 2008
David Meyer

Safely invoking event delegates

Yesterday, my friend and I had an interesting conversation.  He was venting about the code one of his co-workers wrote which caused a problem he spent some time tracking down.  Here's the gist of it:

void RaiseEvent(EventArgs e) { Event(this, e); }

Now most .NET developers should immediately notice what's wrong here.  I certainly hope they do.  What if Event has not been subscribed to?  Then it is null, and invoking it will cause a NullReferenceException.  So this is how I have always invoked event delegates:

if (Event != null) { Event(null, e); }

But one thing my friend pointed out, which was actually new to me, is that the above code is actually not thread-safe, because another thread can unsubscribe from Event after the check for null and before the invocation.  So to make it thread safe, you would first copy the delegate reference to a local variable:

EventHandler handler = Event; if (handler != null) { handler(this, e); }

Now I thought, "This is a lot of code to write just to raise an event.  How can I make this shorter?"  So thanks to C# 3.0 extension methods and lambda expressions, I was able to reduce it to this:

Event.SafeInvoke(d => d(this, e));

Here's the extension method:

public static void SafeInvoke<T>(this T obj, Action<T> action) where T : class { if (obj != null) { action(obj); } }

This accomplishes both objectives: 1) Copy the delegate reference.  This is implicitly done when it is passed as a method parameter.  And 2) Check the delegate for null.  Now I know calling Event.SafeInvoke if Event is null looks a little odd, because you would think it would cause a NullReferenceException, but this is actually not the case with extension methods.  You can call an extension method on null.  This is because the compiler translates this to:

SafeInvoke(Event, d => d(this, e));

Now, unfortunately, you cannot set a restraint on generic types to restrict them to the Enum or Delegate types.  (This makes absolutely no sense to me, I think you should be able to.)  So the above extension method applies to all reference types.  At first I was afraid this would clutter the IntelliSense for non-delegate types, but actually, this extension method would be useful for any reference type.  What if, for instance, you want to call a method on an object but only if it isn't null?

obj.SafeInvoke(o => o.MyMethod());

And the best part is that this will also make the check for null thread-safe if the object in question has a greater scope than a local variable!  So, to wrap up this post, here are all the extension methods I wrote to support both return values and dispatching to another thread (feel free to plagiarize!):

public static void SafeInvoke<T>(this T obj, Action<T> action) where T : class { if (obj != null) { action(obj); } } public static TResult SafeInvoke<T, TResult>(this T obj, Func<T, TResult> func) where T : class where TResult : class { return obj.SafeInvoke(func, null); } public static TResult SafeInvoke<T, TResult>(this T obj, Func<T, TResult> func, TResult defaultResult) where T : class { TResult result = defaultResult; if (obj != null) { result = func(obj); } return result; } public static void SafeDispatch<T>(this T obj, Action<T> action) { if (obj != null) { action.BeginInvoke(obj, null, null); } }

Be the first to rate this post

  • Currently 0/5 Stars.
  • 1
  • 2
  • 3
  • 4
  • 5

Posted by deftflux on Monday, August 11, 2008 2:49 PM
Permalink | Comments (4) | Post RSSRSS comment feed

Related posts

Comments

shovavnik il

Monday, September 29, 2008 9:45 PM

Gravatar

I think you're solving the wrong problem here. You might technically make it locally thread-safe by creating a copy of the delegate, but you might be calling event handlers that should have been de-referenced by a different thread or not call event handlers that should have been.

There's actually a pattern that solves this, but it requires synchronization. It looks something like this:

public InvokeSafely()
{
if(MyEvent != null)
{
lock(_eventSyncObjectDefinedElsewhere)
{
if(MyEvent != null)
{
var sender = this;
var e = CreateEventArgs();
MyEvent(sender, e);
}
}
}
}

Note that the double nullity check, once outside the lock and once inside. The outside check is an optimizer, so you don't have to fire up the heavy synchronization logic. The inside check is to ensure the event is STILL not null, because it could have been de-referenced just before the synchronization, or while the current thread was waiting on a busy lock.

I think you can still implement this logic with extension methods by using a hash table for the synchronization objects, but it can get complex.

Jeremy Gray us

Wednesday, October 01, 2008 2:55 PM

Gravatar

@shovavnik - Unless you also use that same lock during subscription and unsubscription, you are still exposed to the very same race condition that the original code was exposed to, and even then one cannot really consider it that big of a problematic race condition. Additionally, unless you go crazy and implement all sorts of locks all over the place, your "should have been de-referenced by a different thread..." "issues" won't get solved. Even if you did, you'd kill the threaded performance of your app through using so much locking.

At the end of the day, either approach has to pick an instance in time at which to fire the event based on its subscribers at that moment. Similarly, neither approach can do a damn thing about subscriptions and unsubscriptions that happen just after the chosen instance in time. Given this, the effectively equivalent, simpler, and far more performant option presented in the original post is highly preferable.

Jeremy Gray us

Wednesday, October 01, 2008 2:56 PM

Gravatar

@deftflux - your comment engine "Live preview" is respecting line breaks more than your actual comment rendering. The comment previewed with separate paragraphs in it but is not rendering them now. Just so ya know. ;)

deftflux us

Wednesday, October 01, 2008 3:21 PM

Gravatar

Haha, locking in this case would definitely be over-kill. I don't think calling an event handler one atomic value of processing time after it is unsubscribed would hurt anything. However, what my original post addresses is not invoking a delegate that has been removed, but avoiding a NullReferenceException. If, after the check for null, the last event handler is unsubscribed, the value of the event delegate becomes null, and the following invocation of it would result in an exception, which would be problematic. However, by copying the event delegate to a local variable first, the value of the local variable would be isolated from any outside extrathread influence, which would avoid invoking a null delegate. The possibility of invoking an unsubscribed handler is an unfortunate side-effect, but hardly problematic in my opinion.

Add comment


(Will show your Gravatar icon)  

  Country flag




Live preview

Friday, November 21, 2008 8:05 AM

Gravatar