wekempf

This query is brought up by the excelent article http://www.codeproject.com/csharp/event_fundamentals.asp forumid=460928&select=2234627&df=100&msg=2234627.

I've read in many places (some of them I believe were direct from Microsoft, but I'm too lazy right now to dig up the references), including from this article, that you should always assign an event to a temporary delegate before checking for null, in order to avoid a race condition. Something like this:

Code Snippet

protected virtual void SendPropertyChanged(string propertyName)

{

PropertyChangedEventHandler handler = this.PropertyChanged;

if (handler != null)

{

handler(this, new PropertyChangedEventArgs(propertyName));

}

}

However, the generated code from LINQ to SQL produces the following code:

Code Snippet

protected virtual void SendPropertyChanged(string propertyName)

{

if ((this.PropertyChanged != null))

{

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

}

}

I don't understand why we have the extra parens in the "if" statement, but there is no temporary, so it's my understanding that this will result in the possibility of a race condition. Can anyone clear up whether or not it's true that this pattern results in a race condition, and why the generated code doesn't use the "recommended" pattern



Re: LINQ Project General Race condition with SendPropertyChanged (and other) events?

ben2004uk

I remember reading this but never put the two together.

Blog post from Brad Abrams on this subject (from 2005 so it's not a recent change).

http://blogs.msdn.com/brada/archive/2005/01/14/353132.aspx

Wonder if its too late to change the code generation. I think its important to get this right.

But there again, if you read this post then you could say even Brad's approach is wrong.

http://weblogs.asp.net/rosherove/articles/DefensiveEventPublishing.aspx






Re: LINQ Project General Race condition with SendPropertyChanged (and other) events?

wekempf

The post from Brad Abrams was at least one of the posts I was referring to, but was too lazy to look up, so thanks for adding it to the discussion Smile.

The other post doesn't really talk about this subject as all, as far as I can tell from reading it and most of the comments (haven't finished wading through yet). However, I find that post to be utter drivel any way. There are too many things he's done there that I consider to be bad practice. Swallowing exceptions is hardly ever the correct solution, and never the correct solution in a framework, such as his "EventHelper" class. Correct defensive programming here would dictate that subscribers of the event should do everything in their power to not throw exceptions in the first place, and if they must (again, remember that it's bad practice to blindly swallow exceptions), this is a situation in which there's a catastrophic and/or unknown exception and the state of the system is now in question and the exception should continue up the call stack where proper cleanup can occur. Then there's calling the event handlers asynchronously. This will hardly ever be what you want to do, and it adds a TREMENDOUS amount of complexity that should be avoided at all costs.

I haven't finished with that post, or the part 2 it refers to, but right now I'd be inclined to ignore anything said there.

I'd still like to hear from the LINQ developers though. Why is the generated code the way it is A bug that should be fixed before release A design decision that should be documented (entities, especially events on the entities, are not thread safe, for instance) Knowledge that we're not aware of that indicates this defensive pattern isn't really needed Something else