Why use this construct - PropertyChangedEventHandler handler = this.PropertyChanged?

Question

The article http://msdn.microsoft.com/en-us/magazine/dd419663.aspx has the following code sample:

public event PropertyChangedEventHandler PropertyChanged;

protected virtual void OnPropertyChanged(string propertyName)
{       
    PropertyChangedEventHandler handler = this.PropertyChanged;
    if (handler != null)
    {
        var e = new PropertyChangedEventArgs(propertyName);
        handler(this, e);
    }
}

My question is what is gained by introducing the variable 'handler' - the following code seems to work fine:

public event PropertyChangedEventHandler PropertyChanged;

protected virtual void OnPropertyChanged(string propertyName)
{   
    if (PropertyChanged!= null)
    {
        var e = new PropertyChangedEventArgs(propertyName);
        PropertyChanged(this, e);
    }
}

Vivienviviene

Is it not possible that the listner might remove its handle in a multi-threaded envrionment after the != null call? – Sandarac

You should make this an answer. – Allhallowtide

Answer 1

The reasoning behind the local variable is that in a multi-threaded environment, the event could be devoid of subscribers (ie, become null) in the gap between checking for null and firing the event.

By taking a local variable, you are avoiding this potential issue - checking the event in a thread-safe way. It does raise the issue that the event might be thrown for an item that had previously unhooked.

Modestia

Hmm, it is exactly as rare as getting the null reference exception. – Antinomian

Actually, it is possibly less rare - the null situation occurs when the list becomes empty and becomes empty during the gap, the false calling wouldn't need an empty list, just the gap lol – Modestia

I wonder if, Adam or Hans, you have any comment on the point I raised under MattDavey's answer above. – Vivienviviene

@Vivienviviene delegates are immutable so you are grabbing a copy, not a reference in this instance. Hence even if the source is null, your copy isn't. – Modestia

Adam, thanks, I just read on MSDN that "Delegates are immutable; once created, the invocation list of a delegate does not change". I take that to mean that when a method is added to/removed from a delegate, .Net creates a new delegate object. So handler in the code sample would still reference the old object even if a listener had unsubscribed from PropertyChanged in the meantime (PropertyChanged would then point to a newly created object). Is that understanding correct? – Vivienviviene

Yes. That is one of the side-effects of grabbing a copy, your list might be stale. The copy only defends you against exceptions from accessing a null object. You will find that you can do this in code: myClass.MyEvent += MethodSignature, and myClass.MyEvent -= MethodSignature without specifying the new EventHandler( syntax; but I cannot remember why lol! – Modestia

What would happen if a listener unregistered itself and is destroyed between the handler's null check and execution. The function to be called would not even exist in memory anymore. Would there be an exception within OnPropertyChanged? – Analphabetic

@Kurian This pattern does not defend against registered items no longer being accessible. However, I cannot say with certainty what it would do as I've never encountered the situation. I am not sure it's entirely possible to be honest, I think the act of having an entry in a delegate invocation list somewhere will mean it cannot be garbage collected. So you may end up calling into an otherwise unreferenced object. – Modestia

@AdamHouldsworth If the object isn't destroyed yet, event handlers are supposed to gracefully handle being called even after they've been unregistered. I think you're right about the event object holding a reference so the handler will still be called even if the object is out of scope elsewhere and it should handle it gracefully. – Analphabetic

@Kurian It may not be graceful, it entirely depends on what the event handler does. If the object has been "disposed" in user code, a stray reference in a copy of an invocation list somewhere might cause bugs. – Modestia

@AdamHouldsworth Developers are supposed to write safe event handlers. I read it somewhere written by one of the .NET engineers. Obviously, practically no one does. – Analphabetic

Answer 2

This is done for thread safety. In the second example is it possible that the invocation list of PropertyChanged could become null during the if block, resulting in an exception.

Adelinaadelind

Thanks Matt, 'handler' is referencing the same object as 'PropertyChanged', so if a sole listener detaches from 'PropertyChanged' on another thread after the null check, would that not make handler null too? Or has it something to do with handler being a local variable? – Vivienviviene

Answer 3

This is to ensure that the OnPropertyChanged method is thread safe. See Use of null check in event handler for another discussion.

Tamaru