views:

136

answers:

5

Hello,

refering to a lot of documentation on the net, particularly on SO, eg : http://stackoverflow.com/questions/178456/what-is-the-proper-way-to-re-throw-an-exception-in-c there should be a difference between "throw e;" and "throw;".

But, from : http://bartdesmet.net/blogs/bart/archive/2006/03/12/3815.aspx,

this code :

using System;

class Ex
{
   public static void Main()
  {
  //
  // First test rethrowing the caught exception variable.
  //
  Console.WriteLine("First test");
  try
  {
     ThrowWithVariable();
  }
  catch (Exception ex)
  {
     Console.WriteLine(ex.StackTrace);
  }

  //
  // Second test performing a blind rethrow.
  //
  Console.WriteLine("Second test");
  try
  {
     ThrowWithoutVariable();
  }
  catch (Exception ex)
  {
     Console.WriteLine(ex.StackTrace);
  }
}

 private static void BadGuy()
 {
   //
   // Some nasty behavior.
  //
   throw new Exception();
 }

   private static void ThrowWithVariable()
 {
   try
   {
         BadGuy();
   }
  catch (Exception ex)
  {
     throw ex;
  }
}

   private static void ThrowWithoutVariable()
{
  try
  {
     BadGuy();
  }
  catch
  {
     throw;
  }
   }
}

gives the following result :

$ /cygdrive/c/Windows/Microsoft.NET/Framework/v4.0.30319/csc.exe Test.cs
Microsoft (R) Visual C# 2010 Compiler version 4.0.30319.1
Copyright (C) Microsoft Corporation. All rights reserved.

$ ./Test.exe
First test
   at Ex.ThrowWithVariable()
   at Ex.Main()
Second test
   at Ex.ThrowWithoutVariable()
   at Ex.Main()

which is in complete contradiction with the blog post.

The same kind of result is obtained with the code from : http://crazorsharp.blogspot.com/2009/08/rethrowing-exception-without-resetting.html

Original question : what am I doing wrong ?

UPDATE : same result with .Net 3.5 / csc.exe 3.5.30729.4926

SUMUP : all your answers were great, thanks again.

So the reason is effectively inlining due to the 64-bit JITter.

I had to choose only one answer, and here is why I have choosen LukeH answer :

  • he guessed the inlining problem and the fact it may be related to my 64-bit architecture,

  • he provided the NoInlining flag which is the simplest way to avoid this behavior.

However this issue now rises another question : is this behavior compliant with all the .Net specifications : the CLR ones and the C# programming language ones ?

UPDATE : this optimization seems compliant according to : http://stackoverflow.com/questions/3552125/throw-vs-rethrow-same-result/3552244#3552244 (thanks 0xA3)

Thanks in advance for your help.

+1  A: 

Use a debug build and you will see the difference more clearly. With a debug build the first run will show the location as the throw ex line and the second as originating from the actual call to BadGuy. Obviously the 'problem' is the call to BadGuy - not the throw ex line and you'll chase fewer ghosts with the direct throw; statement.

In a stack trace this shallow the benefits aren't as immediatley obvious, in a very deep stack you'll mask the actual source of the problem and loose some fidelity by manually throwing exception instead of using the built in re-throw statement.

Paul Alexander
Yes it's true, debug build works as expected.Thanks.
Serious
+2  A: 

I have tried running this code myself and the debug build works as I expected but I got the same result as you in the release build.

I suspect what's happening is that the compiler inlining has simply replaced the BadGuy() call with throw new Exception(); because that's the only statement in BadGuy().

If you switch off the 'Optimize code' option in the project properties -> Build screen, then both Release and Debug build produces the same result which shows BadGuy() at the top of the stack trace.

theburningmonk
Inlining seems to be what effectively happen : great guess !
Serious
+2  A: 

It seems that the JIT optimizers does some work here. As you can see, the call stack in the second case is different than in the first case when you run the Debug build. However, in the Release build, both call stacks are identical due to the optimization.

To see that this is related to the jitter you can decorate the methods with a MethodImplAttribute attribute:

[MethodImpl(MethodImplOptions.NoOptimization)]
private static void ThrowWithoutVariable()
{
    try
    {
        BadGuy();
    }
    catch
    {
        throw;
    }
}

Note that the IL is still different for ThrowWithoutVariable and ThrowWithVariable:

.method private hidebysig static void  ThrowWithVariable() cil managed
{
  // Code size       11 (0xb)
  .maxstack  1
  .locals init ([0] class [mscorlib]System.Exception ex)
  .try
  {
    IL_0000:  call       void Ex::BadGuy()
    IL_0005:  leave.s    IL_000a
  }  // end .try
  catch [mscorlib]System.Exception 
  {
    IL_0007:  stloc.0
    IL_0008:  ldloc.0
    IL_0009:  throw
  }  // end handler
  IL_000a:  ret
} // end of method Ex::ThrowWithVariable

.method private hidebysig static void  ThrowWithoutVariable() cil managed
{
  // Code size       11 (0xb)
  .maxstack  1
  .try
  {
    IL_0000:  call       void Ex::BadGuy()
    IL_0005:  leave.s    IL_000a
  }  // end .try
  catch [mscorlib]System.Object 
  {
    IL_0007:  pop
    IL_0008:  rethrow
  }  // end handler
  IL_000a:  ret
} // end of method Ex::ThrowWithoutVariable

Update to answer your follow-up question whether this is compliant with the CLI specification

In fact it compliant, namely to allow for the JIT compiler to enable important optimizations. Annex F states on page 52 (emphasis by me):

Some CIL instructions perform implicit run-time checks that ensure memory and type safety. Originally, the CLI guaranteed that exceptions were precise, meaning that program state was preserved when an exception was thrown. However, enforcing precise exceptions for implicit checks makes some important optimizations practically impossible to apply. Programmers can now declare, via a custom attribute, that a method is “relaxed”, which says that exceptions arising from implicit run-time checks need not be precise.

Relaxed checks preserve verifiability (by preserving memory and type safety) while permitting optimizations that reorder instructions. In particular, it enables the following optimizations:

  • Hoisting implicit run-time checks out of loops.
  • Reordering loop iterations (e.g., vectorization and automatic multithreading)
  • Interchanging loops
  • Inlining that makes an inlined method as least as fast as the equivalent macro
0xA3
Downvoter, dare to leave a reason?
0xA3
Thanks, the NoOptimization flag gives the expected result in all cases.
Serious
Thanks again for your rich answer about the compliance of such an optimization.
Serious
+3  A: 

I can't replicate the problem -- using .NET 3.5 (32-bit) gives me the same results described in Bart's article.

My guess is that the .NET 4 compiler/jitter -- or maybe it's the 64-bit compiler/jitter if this is happening under 3.5 too -- is inlining the BadGuy method into the calling methods. Try adding the following MethodImpl attribute to BadGuy and see if that makes any difference:

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
private static void BadGuy()
{
    //
    // Some nasty behavior.
    //
    throw new Exception();
}
LukeH
You're right, using the NoInlining flag did the trick.Thanks.
Serious
A: 

On a side note, I found a hack posted on a blog once (I've since lost the reference) that allows you to retain the call-stack on rethrow. This is mainly useful if you catch an Exception in one context (say, in a thread running an asynchronous operation) and want to rethrow it in another (say, in the other thread that launched the asynchronous operation). It makes use of some undocumented functionality included to allow preservation of stack traces across remoting boundaries.

    //This terrible hack makes sure track trace is preserved if exception is re-thrown
    internal static Exception AppendStackTrace(Exception ex)
    {
        //Fool CLR into appending stack trace information when the exception is re-thrown
        var remoteStackTraceString = typeof(Exception).GetField("_remoteStackTraceString",
                                                                 BindingFlags.Instance |
                                                                 BindingFlags.NonPublic);
        if (remoteStackTraceString != null)
            remoteStackTraceString.SetValue(ex, ex.StackTrace + Environment.NewLine);

        return ex;
    }
Dan Bryant
No need to do that, that's what the InnerException is for. `throw new Exception( "I blew up", ex )`.
Paul Alexander
This is really terrible and should never be used.
0xA3
@Paul, the problem with InnerException is that catch clauses break down (they need to catch the outer Exception type). This breaks transparency if you want an asynchronous handler to be able to interpret exceptions in the same way as if the synchronous version were called. There are ways around this (you can make the synchronous version wrap the exception as well), but it's somewhat more awkward on the catch side.
Dan Bryant
@0xA3, I agree that it's terrible, but, to be fair, MS effectively did the same thing to support Remoting by letting the Remoting system manipulate the Exception state as part of its magic.
Dan Bryant
The biggest problem with this code is that *any* update or patch of mscorlib might break *your* application. There is no guarantee whatsoever that `System.Exception` has a private field named like this and that it behaves the way that you expect under all circumstances. For `System.Runtime.Remoting` it is perfectly fine to use this mechanisms as there are `internal` methods exposed within mscorlib to modify the remote stack trace.
0xA3