Use of null check in event handler

The problem is that if nobody subscribes the the event, it is null. And you can't invoke against a null. Three approaches leap to mind:

• check for null (see below)
• add a "do nothing" handler: public event EventHandler MyEvent = delegate {};
• use an extension method (see below)

When checking for null, to be thread-safe, you must in theory capture the delegate reference first (in case it changes between the check and the invoke):

protected virtual void OnMyEvent() {
    EventHandler handler = MyEvent;
    if(handler != null) handler(this, EventArgs.Empty);
}

Extension methods have the unusual property that they are callable on null instances...

    public static void SafeInvoke(this EventHandler handler, object sender)
    {
        if (handler != null) handler(sender, EventArgs.Empty);
    }
    public static void SafeInvoke<T>(this EventHandler<T> handler,
        object sender, T args) where T : EventArgs
    {
        if (handler != null) handler(sender, args);
    }

then you can call:

MyEvent.SafeInvoke(this);

and it is both null-safe (via the check) and thread-safe (by reading the reference once only).

It's really not clear what you mean I'm afraid, but if there's the possibility of the delegate being null, you need to check that separately on each thread. Typically you'd do:

public void OnSeven()
{
    DivBySevenHandler handler = EventSeven;
    if (handler != null)
    {
        handler(...);
    }
}

This ensures that even if EventSeven changes during the course of OnSeven() you won't get a NullReferenceException.

But you're right that you don't need the null check if you've definitely got a subscribed handler. This can easily be done in C# 2 with a "no-op" handler:

public event DivBySevenHandler EventSeven = delegate {};

On the other hand, you might want some sort of locking just to make sure that you've got the "latest" set of handlers, if you might get subscriptions from various threads. I have an example in my threading tutorial which can help - although usually I'd recommend trying to avoid requiring it.

In terms of garbage collection, the event publisher ends up with a reference to the event subscriber (i.e. the target of the handler). This is only a problem if the publisher is meant to live longer than the subscriber.

I want to append some short information about the C# 6.0-Syntax:

It is now possible to replace this:

var handler = EventSeven;

if (handler != null)
    handler.Invoke(this, EventArgs.Empty);

with this:

handler?.Invoke(this, EventArgs.Empty);

Combining it with expression-bodied members, you can shorten the following code:

protected virtual void OnMyEvent()
{
    EventHandler handler = MyEvent;
    handler?.Invoke(this, EventArgs.Empty);
}

down to a one-liner:

protected virtual void OnMyEvent() => MyEvent?.Invoke(this, EventArgs.Empty);

See MSDN for more information about the null-conditional operator
See this blog about expression-bodied members

It is always good practice to check an event handler before firing it. I do this even if I initially "guarantee" myself that it is always set. If I later change this I don't have to check all my event firing. So for each event I always have an accompanying OnXXX method like this:

private void OnEventSeven()
{
    var handler = EventSeven;
    if (handler != null)
    {
        handler(this, EventArgs.Empty);
    }
}

This is especially important if the event handler is public to your class since external callers can add and remove event handlers at will.

If you mean this:

public static void OnEventSeven(DivBySevenEventArgs e)
    {
        if(EventSeven!=null)
            EventSeven(new object(),e);
    }    

piece of code, then the answer is:

If nobody subscribes to the "EventSeven" event handler then you'll get a null-reference exception on "EventSeven(new object(),e);"

And the rule:

The subscriber is responsible for adding a handler (+=) and removing it (-=) when he doesn't want to receive the events any longer. Garbage collection goes by the default rules, if an object is no longer referenced it can be cleaned.

Using PostSharp it is possible to adjust the compiled assembly in a post-compilation step. This allows you to apply 'aspects' to the code, resolving cross-cutting concerns.

Although the null checks or empty delegate initialization might be a very minor issue, I wrote an aspect which resolves it by adding an empty delegate to all events in an assembly.

It's usage is quite easy:

[assembly: InitializeEventHandlers( AttributeTargetTypes = "Main.*" )]
namespace Main
{
   ...
}

I discussed the aspect in detail on my blog. In case you have PostSharp, here is the aspect:

/// <summary>
///   Aspect which when applied on an assembly or class, initializes all the event handlers (<see cref="MulticastDelegate" />) members
///   in the class(es) with empty delegates to prevent <see cref="NullReferenceException" />'s.
/// </summary>
/// <author>Steven Jeuris</author>
[AttributeUsage( AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Event )]
[MulticastAttributeUsage( MulticastTargets.Event, AllowMultiple = false )]
[AspectTypeDependency( AspectDependencyAction.Commute, typeof( InitializeEventHandlersAttribute ) )]
[Serializable]
public class InitializeEventHandlersAttribute : EventLevelAspect
{
    [NonSerialized]
    Action<object> _addEmptyEventHandler;


    [OnMethodEntryAdvice, MethodPointcut( "SelectConstructors" )]
    public void OnConstructorEntry( MethodExecutionArgs args )
    {
        _addEmptyEventHandler( args.Instance );
    }

    // ReSharper disable UnusedMember.Local
    IEnumerable<ConstructorInfo> SelectConstructors( EventInfo target )
    {
        return target.DeclaringType.GetConstructors( BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic );
    }
    // ReSharper restore UnusedMember.Local

    public override void RuntimeInitialize( EventInfo eventInfo )
    {
        base.RuntimeInitialize( eventInfo );

        // Construct a suitable empty event handler.
        MethodInfo delegateInfo = DelegateHelper.MethodInfoFromDelegateType( eventInfo.EventHandlerType );
        ParameterExpression[] parameters = delegateInfo.GetParameters().Select( p => Expression.Parameter( p.ParameterType ) ).ToArray();
        Delegate emptyDelegate
            = Expression.Lambda( eventInfo.EventHandlerType, Expression.Empty(), "EmptyDelegate", true, parameters ).Compile();

        // Create a delegate which adds the empty handler to an instance.
        _addEmptyEventHandler = instance => eventInfo.AddEventHandler( instance, emptyDelegate );
    }
}

... and the helper method it uses:

/// <summary>
///   The name of the Invoke method of a Delegate.
/// </summary>
const string InvokeMethod = "Invoke";


/// <summary>
///   Get method info for a specified delegate type.
/// </summary>
/// <param name = "delegateType">The delegate type to get info for.</param>
/// <returns>The method info for the given delegate type.</returns>
public static MethodInfo MethodInfoFromDelegateType( Type delegateType )
{
    Contract.Requires( delegateType.IsSubclassOf( typeof( MulticastDelegate ) ), "Given type should be a delegate." );

    return delegateType.GetMethod( InvokeMethod );
}

Sign up or log in

Post as a guest