Too much reuse

Too much reuse

Rate This
  • Comments 71

A recent user question:

I have code that maintains a queue of pending work items waiting to be completed on various different worker threads. In certain unfortunate fatal error situations I complete each of these by throwing an exception. Can I create just one exception object? Are there any issues throwing the same exception object multiple times on multiple threads?

Anyone who has ever seen this in a code review knows the answer:

catch(Exception ex)
{
   Logger.Log(ex);
   throw ex;
}

This is a classic “gotcha”; almost always the right thing to do is to say “throw;” rather than “throw ex;” – the reason being that exceptions are not completely immutable in .NET. The exception object’s stack trace is set at the point where the exception is thrown, every time it is thrown, not at the point where it is created. The “throw;” does not reset the stack trace, “throw ex;” does.

Don’t reuse a single exception object. Every time gets thrown the stack trace will be reset, which means that any code up the stack which catches the exception and logs the trace for later analysis will almost certainly be logging someone else’s trace. Making exceptions is cheap, and you’re already in a fatal error situation; it doesn’t matter if the app crashes a few microseconds slower. Take the time to allocate as many exceptions as you plan on throwing.

  • I know a lot about exceptions and exception handling, but this was actually new to me. Thank you.

  • Would it be a good idea to create a new exception and assign the caught exception to InnerException?

  • I have seen this pattern more times than I can count, and it is a mistake that is made very often even by good, experienced, knowledgeable developers.  I think a lot of people don't even realize that catch(Exception){ throw; } is valid syntax. The similarity of the C# throw statement to Java probably doesn't help since java requires the "throw somethingThrowable" syntax.  Also, people become so accustomed to writing catch(Exception e){ throw new Exception("Uh oh!", e); } that they assume they must "throw anException;".  It makes me wonder if there is something that the compiler (or some other verification tool) could do to make the correct pattern more obvious.  Since it is perfectly valid code and could very well be intended, a warning doesn't really make sense.  I know new syntax is definitely low on the list of things to be considered, but the source of the confusion seems to be two behaviors from what is interpreted to be identical code.

  • So how expensive are exceptions to throw? Are they cheap enough to use them for nonlocal transfer of control?

  • I agree with Chris B that catch(Exception) { throw; } is valid and better than throw ex, but isn't it simply better to not catch the exception at all.  Doesn't that do the same thing?  In Eric's example it's being logged, so you need the catch, but if you're not doing anything with it, why catch it.

  • Again, this is another great candidate for the Messages pane.

  • @Gabe... using exceptions (even if they were "Free") for "nonlocal transfer of control" is pure EVIL. In some environments, every exception (even those that are caught) are treated as "support alerts".

    Under "normal" circumstances the PerformanceCounters for Exceptions should remain at 0. Unfortunately even Microsoft's CLR/BCL violates this principal. Just set "Break on Exception" to true for many application types, and watch how many exceptions you have to step through just during application startup!!!!!

  • Brian: There are plenty of reasons why you might want to inspect an exception before possibly rethrowing it. Perhaps you know how to handle some kinds of IOExceptions but not others, or you want to look for a certain type of InnerException.

    Or you might just want to optionally ignore it, like here:

    while (true)

       try {

          ....

       } catch (Exception) {

           if (!retry) throw;

       }

  • @Gabe... understand, but if you're not doing anything with it, don't catch it.  Maybe I'm just being pedantic... or not pedantic enough.

  • @Brian,  I didn't intend the code to be a good example of exception handling, so I omitted the actual handling (logging, wrapping, cleanup, whatever) for brevity. Sorry for not being as clear as I should have.

  • @Gabe:

    Your original question of how cheap they are hasn't been looked at recently from what I could see.

    http://www.codeproject.com/KB/exception/ExceptionPerformance.aspx

    http://www.yoda.arachsys.com/csharp/exceptions2.html

    The second one is actually a better way of describing the situation.  

    "If you ever get to the point where exceptions are significantly hurting your performance, you have problems in terms of your use of exceptions beyond just the performance."

    Your answer is :"depends."

  • "it doesn’t matter if the app crashes a few microseconds slower"

    This should go on a t-shirt or something.

  • A little off topic but I’ll ask anyway.

    I sometimes catch an exception in a utility method and then re ‘throw;’ it, but before rethrowing it I use another aspect of an exceptions ‘mutability’ the ‘Data’ property.

    I add local contextual information to it. My base exception handler logs all the information it can about the exception i.e. message, stack, type, innerexception etc Plus the Data property.

    Is this a good idea? Do others use this pattern? Does anyone out there use Exception.Data at all?

    private void ExecuteSqlExample(string sql)

    {

       try

       {

           //To stuff here

           //.......

       }

       catch (Exception ex)

       {

           ex.Data["Sql"] = sql;

           throw;

       }

    }

  • Unfortunately, Microsoft documentation does not help the situation.  Below is the documentation in the try-catch section of Visual Studio C# MSDN Library:

    A throw statement can be used in the catch block to re-throw the exception, which has been caught by the catch statement. For example:

    catch (InvalidCastException e)

    {

       throw (e);    // Rethrowing exception e

    }

  • One extension method we have at my office is this

    public static TException Rethrow<TException>(this TException exception) where TException : Exception

    {

        var field = typeof(Exception).GetField("_remoteStackTraceString", BindingFlags.Instance | BindingFlags.NonPublic);

        field.SetValue(exception, exception.StackTrace);

        return exception;

    }

    This allows us to do

    throw ex.Rethrow();

    without reseting the stack. This is useful if we hand an exception off to a function that decides whether it should be rethrown.

Page 1 of 5 (71 items) 12345