0

I was looking at these questions from 2009: C# Events and Thread Safety

I also looked at Eric Lippert's blog post on the same topic: http://blogs.msdn.com/ericlippert/archive/2009/04/29/events-and-races.aspx

The problem seems to boil down to, in a multithreaded environment you will occasionally encounter the situation where a different thread has unregistered the delegate between your thread's (thread A) 'if' statement and 'call delegate' statement.

As I was reading, this question came to my mind: If the alternates seem to be a choice between either a race condition or a thrown exception, then why not just wrap it in a try/catch block and not worry about either? If you catch the nullReferenceException, just ignore it in the catch block (simply to suppress the exception) and move on.

Now, I understand Eric Lippert and John Skeet know a little bit about C#, multithreading, and delegates, so could someone please take a moment and explain what I am missing here?

Community
  • 1
  • 1
philologon
  • 2,093
  • 4
  • 19
  • 35
  • 2
    What's the point of try/catching if all you're gonna do in the catch block is to rethrow? – Darin Dimitrov Jan 08 '13 at 22:42
  • The point would be to suppress an exception which, in this limited case, you know that you can ignore, and you don't want your end user to have to restart your application. Also, I did not say "rethrow". Where I said "throw it away". Clearly I need to clarify that statement. – philologon Jan 08 '13 at 22:45
  • 6
    I'm confused by the question. How does catching an exception eliminate the race? – Eric Lippert Jan 08 '13 at 22:46
  • Actually Eric, I am the one who is confused. That is why I posted the question, which is really, "what am I missing?" You have answered it, and it is so simple that I am now a little embarrased. – philologon Jan 08 '13 at 22:48
  • 4
    @EricLippert Didn't you know? Try/Catch is the solution to all programming errors – Sam I am says Reinstate Monica Jan 08 '13 at 22:48
  • @Sam I am : AppDomain.UnhandledException is a far better solution as you do not need to write any try/catch block anymore, so you do not need to care about errors anymore! A real life-changing tool! – Pragmateek Jan 08 '13 at 23:06

4 Answers4

2

The main problem with this kind of exception swallowing is that you can't be sure that the NullReferenceException was caused by the event delegate being null. It could have been thrown from within the delegate itself.

Just to be sure - what you're saying is, instead of doing this:

Action temp = Foo;
if (temp != null)
      temp();

To do this:

try {
    Foo();
}
catch (NullReferenceException e)
{
    // magically ignore it
}

So again, the Foo() call could cause a similar exception for many reasons, and you'll be masking unrelated exceptions.

sinelaw
  • 16,205
  • 3
  • 49
  • 80
  • Yes, your code samples are what I am describing. – philologon Jan 08 '13 at 22:53
  • @philologon, ok, then as I said there's no way to guarantee that the exception you are catching is due to the race condition - could be another cause entirely – sinelaw Jan 08 '13 at 22:56
2

Generally speaking, when an exception is thrown, it means that something wrong is happening.

Catching and exception doesn't make the wrongness go away. Your operation still failed. All it does is gives you a chance to respond to what's happening that's wrong.

Sam I am says Reinstate Monica
  • 30,851
  • 12
  • 72
  • 100
  • If there was a specific exception that could be caught in this case **only** when the specific race condition occurred, it would be OK to catch it. The main point is that even that is not true here. – sinelaw Jan 08 '13 at 22:55
2

If the alternates seem to be a choice between either a race condition or a thrown exception

I don't think that's the point of the article.

As Eric points out, there are two issues:

  1. The delegate could be null
  2. The handler itself could be susceptible to a race condition; Just because the unregistering code is something like event -= handler; DisposeRequiredResources(); does not mean that the handler code will not ever run after the dispose has been called.

Issue 1 is commonly solved with a temporary variable. (it is required, if (Foo != null) Foo(); doesn't work due to a race condition). Other ways of solving include initializing with an empty delegate. Using try/catch here is not only more typing, it also has the danger of swallowing real NullReferenceExceptions in the event handler.

Issue 2 is solved by making the handler robust. Putting the entire call in a try/catch block does not alleviate the issue.

Jimmy
  • 89,068
  • 17
  • 119
  • 137
1

Maybe I understand you wrong, but you can't catch every race conditions with just try-catch. See the code below

int count = 0;
for (int i = 0; i < 1000; i++) 
            Task.Factory.StartNew(()=>count++,TaskCreationOptions.LongRunning);
Console.WriteLine(count);

this may print other that 1000 when run several times.

I4V
  • 34,891
  • 6
  • 67
  • 79
  • Note my comment to Eric Lippert in comments to my OP. I was not thinking it through clearly, and might have thought another moment before posting the question. Thus, I was not thinking correctly about how the try/catch statement interacts with the race condition. – philologon Jan 08 '13 at 22:59
  • I chose this as the accepted answer because "what I am missing" is that try/catch blocks do not stop race conditions from happening, and of the comments at the Answer level, this is the closest to pointing that out to me. The first to do that, though, was EricLippert, but he did so in a Comment, not an Answer, so I can't mark that as Accepted. Also, the other posts also point out information I did not realize, so thank you all for the kind comments. – philologon Jan 09 '13 at 02:21