0

Ive been coding for a while now, and just noticed something interesting. So normally when I'm implementing events in a class, I want to check the event's null status before I try to call it, so I'd use the following syntax:

private void OnEvent(EventArgs e)
{
   if (Event != null) Event(this, e);
}

But I noticed something today in Visual Studio, it it's code simplification suggestions, it suggested the following syntax as a simplification:

private void OnEvent(EventArgs e)
{
   Event?.Invoke(this, e);
}

Is anyone familiar with this "?" syntax? Is it a generic shorthand for checking null status of anything or just delegates? Its not part of Linq framework, its a built in syntax. Any insight into this and what's its used for would be helpful. I've dont quite a bit of searching but can't find anything on it specifically.

Wanabrutbeer
  • 676
  • 6
  • 11
  • 1
    It's called a 'null conditional operator', and is part of C# 6.0: https://msdn.microsoft.com/en-us/library/dn986595.aspx – Rufus L Mar 27 '17 at 20:31
  • It's called the [Null-Conditional Operator](https://msdn.microsoft.com/en-us/library/dn986595.aspx). – Igor Mar 27 '17 at 20:31
  • It should be noted that it is better because your first example is not thread safe. What if another thread were to set `Event` to null after your if but before your invocation? If you didn't want to use it the correct syntax would be something like `var localEvent = Event; if (localEvent != null) localEvent(this, e);` to guarantee thread safety. – dmeglio Mar 27 '17 at 20:33
  • @dman2306 why would you set it to `null`? – Jeroen van Langen Mar 27 '17 at 20:38
  • @JeroenvanLangen If someone `-=` the event handlers – dmeglio Mar 27 '17 at 20:41
  • @dman2306 It won't become `null`, even if the last one is removed. But the multithreaded thing is true.. (under the hood the `event?.Invoke()` makes a local copy of the instance) – Jeroen van Langen Mar 27 '17 at 20:43
  • @JeroenvanLangen According to the comment by Jon Skeet on this answer http://stackoverflow.com/a/672654/4508432 it sounds as though you are incorrect. – dmeglio Mar 27 '17 at 20:45
  • @dman2306 Your right. Tested it. Always thought it would become an empty delegate/list – Jeroen van Langen Mar 27 '17 at 20:53
  • @EricLippert so, you should use locking? – Jeroen van Langen Mar 27 '17 at 20:54
  • @JeroenvanLangen: Absolutely not! Oh my goodness that is a terrible idea. You are suggesting that someone *invoke a handler that they don't control while under a lock*; what happens when that handler, which can run **arbitrary** code, itself waits on any other lock that a different thread already possesses? You can get a deadlock very easily. – Eric Lippert Mar 27 '17 at 20:55
  • I don't know. Never mixed threading and events. I always bind my events before I start the thread it self. (like sockets etc) – Jeroen van Langen Mar 27 '17 at 20:57
  • @JeroenvanLangen: No, what you should do is (1) don't write multithreaded code in the first place, (2) if you must, don't make events that can be subscribed and unsubscribed on different threads, (3) if you must, then you are **required** to ensure that it is **never** wrong for a stale handler to be invoked if there is a race between an event firing and a handler being unsubscribed. How you do that is up to you, but you are required to do it. – Eric Lippert Mar 27 '17 at 20:57
  • Like I said, I never mix them, I'd rather use a concurrent queue to pass information between threads. So the event only adds items to a queue and I don't register/deregister while the thread is running – Jeroen van Langen Mar 27 '17 at 20:59
  • @dman2306: I realized I was unclear in my earlier comment and deleted it. To be clear: the naive if-check is neither null safe nor thread safe; a threading race can cause null to be dereferenced. Your "local" version is exactly equivalent to the `?.` operator, and is null safe, but not thread safe. Though there is no race which causes null to be dereferenced, there is still a race which causes an unsubscribed handler to be invoked. – Eric Lippert Mar 27 '17 at 21:01

3 Answers3

6

I left some comments above, but I feel that they are important enough to be called out into an answer.

First, as others have noted, this is the null conditional operator.

This version of the code is considered poor style because the event could become null after the check if modified on another thread:

private void OnEvent(EventArgs e)
{
  if (Event != null) Event(this, e);
}

However, I note that multithreaded programs which invoke, subscribe and unsubscribe events on arbitrary threads are relatively rare.

The standard advice before the null conditional operator was added was to write the clumsy:

private void OnEvent(EventArgs e)
{
  var ev = Event;
  if (ev != null) ev(this, e);
}

This eliminates the null dereference if there is a race.

The null conditional operator is the same:

private void OnEvent(EventArgs e)
{
  Event?.Invoke(this, e);
}

This is just a shorthand for var ev = Event; if (ev != null) ev.Invoke(this, e);.

Now, people will tell you that the latter two versions, either with the local, or more concisely, with the ?., are "thread safe". They are not. These will not dereference null, but that is not the interesting race. The interesting race is:

Thread Alpha: create disposable object X -- say, a log file writer
Thread Alpha: subscribe X.H to event
Thread Bravo: Cause event to fire
Thread Bravo: Save X.H into local and check for null. It's not null.
Thread Alpha: Unsubscribe X.H from event
Thread Alpha: Call Dispose on X -- the log file is now closed.
Thread Bravo: Invoke X.H via the local

And now we have invoked a method on a disposed object, which, if the object is implemented correctly, should throw an "object disposed" exception. And if not, hey, it tries to write to a closed file and everything breaks, yay!

In a multithreaded environment, event handlers are required to be safe to call forever, even after they have been unsubscribed from the event. No amount of "thread safety" on the call site is going to solve that problem; this is a requirement of the handler. But how does the author of the handler know that they are going to be used in a multi-threaded environment, and plan accordingly? Often, they don't. So things break.

If you don't like that then don't write programs that use events on multiple threads. It is really hard to get right, and everyone -- sources and handlers both -- has to cooperate to make sure it works.

Now, some might say, isn't the solution to put a lock on the handler?

private object HandleLocker = new object();
...
private void OnEvent(EventArgs e)
{
  lock (HandleLocker)
  {
    if (Event != null) 
      Event(this, e);
  }
}

And then similarly add locks to the add and remove of Event.

This is a cure that is worse than the disease.

Thread Bravo takes out a lock on some object Foo and obtains it.
Thread Charlie wishes to fire the event.
Thread Charlie takes out a lock on HandleLocker and obtains it.
Thread Charlie calls the handler. The handler blocks trying to obtain Foo.
Thread Bravo attempts to subscribe a new handler, and blocks on HandleLocker.

Now we have two threads each of which is waiting for the other to complete. Bravo cannot move until HandleLocker is released by Charlie, and Charlie cannot move until Foo is released by Bravo.

Never do this. Again: you are required to ensure that handlers may be invoked while stale if you are in a multithreaded event handling program, and you may not use locks to do it. That's a bad situation to be in; locks are the standard mechanism for managing threads.

Multithreading is hard. Don't write multithreaded programs if you don't understand all the ways that things can go wrong, and how to avoid them.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
0

The ? / 'null conditional operator' is simply a shorthand way of testing for null before performing a member access or index operation.

You can read more here: https://msdn.microsoft.com/en-us/library/dn986595.aspx

Trivaxy
  • 41
  • 1
  • 5
0

This is called the Null-Conditional operator and is useful in a bunch of situations. It and the Null-Coalescing operator (??) are really clean, expressive ways of checking for null before dereferencing something or providing clean defaults.

For more info https://msdn.microsoft.com/en-CA/library/dn986595.aspx

Chris Nantau
  • 38
  • 1
  • 4