CODEWITHSUNDEEP
c#wpfmvvm
Igor Popov
Igor Popov

Reputation: 10101

MVVM INotifyPropertyChanged - Thread issues?

I'm just starting to learn MVVM and WPF so sorry for asking stupid questions.

I'm using different tutorials and examples to learn and I come across this example (read Example 2) which I don't understand.

private void RaisePropertyChanged(string propertyName)
{
    // take a copy to prevent thread issues
    PropertyChangedEventHandler handler = PropertyChanged;
    if (handler != null)
    {
        handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

Basically, the comment doesn't make much sense to me... "take a copy to prevent thread issues".

This line:

PropertyChangedEventHandler handler = PropertyChanged;

doesn't create a new, totally different handler object (it's not cloned). It's just a new reference to the same PropertyChanged object, right?

I did some tests to find out what is really happening:

PropertyChangedEventHandler handler = PropertyChanged;
var message = "PropertyChanged: " + PropertyChanged.GetHashCode() + "\n";
message += "handler: " + handler.GetHashCode() + "\n";
message += "are equal (1): " + (PropertyChanged.Equals(handler)) + "\n";
message += "are equal (2): " + (PropertyChanged == handler) + "\n";

MessageBox.Show(message);

Here's the result:

screenshot-wpf-mvvm-inotifypropertychanged

This confirms my theory that these 2 objects are really the SAME and the assignment is just a NOOP. What I don't understand is how is this related at all with "thread issues" (from the comment)?!?

And one more thing: after a bit of testing (using a really simple example) I found out that the PropertyChanged event is never null. Why do we need the null check at all?

In my view the previous method can be condensed to just this:

private void RaisePropertyChanged(string propertyName)
{
    PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
}

I tested a bit (again, on a really simple example) and it seems to work very well... Is there some catch or something that I didn't find? Maybe I just found bad examples?

Anyway, there's a lot of stuff I don't know since as I said I just started learning WPF and MVVM, but I want to understand what is really happening, not just take some code and just paste it without understanding why and how it works. See cargo cult programming and magic programming.

EDIT

OK, based on the answers, the PropertyChanged event can be changed between verification and call. More, the PropertyChanged event can be null. However, I haven't been able to reproduce these behaviors...

Can someone actually give me an example where both statements happen? It would certainly help recognize similar situations.

Upvotes: 1

Views: 977

Answers (3)

dkozl
dkozl

Reputation: 33394

If you would do only 

PropertyChanged(this, new PropertyChangedEventArgs(propertyName));

you risk that PropertyChanged will be null and you'll get null reference exception hence you should check before if event handler is not null. Now if you would do 

if (PropertyChanged != null)  
    PropertyChanged(this, new PropertyChangedEventArgs(propertyName));

in multi threaded situation you risk that PropertyChanged will become null between check and call. To avoid potential race condition you keep current delegate in local variable and check and call that. 

var handler = PropertyChanged;
if (handler != null)
    handler(this, new PropertyChangedEventArgs(propertyName));

this is safe because

Delegates are immutable; once created, the invocation list of a delegate does not change

so even if PropertyChanged was to change in between new delegate will be created but handler will still have your invocation list as it was at the moment you did

var handler = PropertyChanged;

Update: C# 6 Fixed Syntax

This argument has been solved in C# 6, where there is a new opertator to assist with dealing with this level of confusion, since C#6, all event handlers should use the following syntax as a standard:

https://codeblog.jonskeet.uk/2015/01/30/clean-event-handlers-invocation-with-c-6/

PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));

Upvotes: 7

Blablablaster
Blablablaster

Reputation: 3348

Compare these 2 code samples:

First:

private void RaisePropertyChanged(string propertyName)
{
    // take a copy to prevent thread issues
    PropertyChangedEventHandler handler = PropertyChanged;
    if (handler != null)
    {
        handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

Second:

private void RaisePropertyChanged(string propertyName)
{
    if (PropertyChanged != null)
    {
        PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
    }
}

The problem in the second sample is that PropertyChanged field can became null just right after the if. So the call PropertyChanged(this, new PropertyChangedEventArgs(propertyName)); will became null(this, new PropertyChangedEventArgs(propertyName));, so it will lead you to the land of troubles. First sample does not have such a flaw and it will work as you expect even if any other thread will affect PropertyChanged field.

Upvotes: 0

neptunao
neptunao

Reputation: 607

Your first question has been answered here.

About your second question. PropertyChanged can be null if there are no subscribers or all subscribers will unsubscribe from it.

When you start WPF app with any bindings to object that implement INotifyPropertyChanged, wpf binding system immediately susbscribe to PropertyChanged event and it will not be null after that.

Upvotes: 1

Related Questions