4

Faced with code that make one exception instance and then probably throw it multiple times.

private readonly Exception exceptionInstance = new Exception("message");

Is it correct to throw same exception instance multiple times?

10
  • 2
    Out of curiosity, why do you want to only use a specific instance? Commented Mar 28, 2018 at 12:21
  • 1
    You mean throw exceptionInstance from your code? An exception is sually just a data-object. You could use the same instance again and again, but I can´t see any reasn to do so, as it will allways contain the exact same message and thus won´t have a very generic use. Commented Mar 28, 2018 at 12:21
  • Singleton? But why? Commented Mar 28, 2018 at 12:25
  • @Crowcoder, I don't know why author do this. My question is about possible issues of this code. Commented Mar 28, 2018 at 12:33
  • 1
    You should certainly refactor this code. For example I'm quite sure that this is not thread-safe, so when you will throw this exception from multiple threads - who knows what can happen. Commented Mar 28, 2018 at 12:39

2 Answers 2

5

It's bad practice for various already stated reasons, but it will fail especially hard in multithreaded code, because Exception class is (obviously) not thread safe, and it's not immutable.

Consider this code:

class Program {
    static readonly Exception _test = new Exception("test");

    static void Main(string[] args) {
        ThreadPool.SetMinThreads(10, 8);
        var random = new Random();
        int num1 = 0;
        int num2 = 0;
        var tasks = new List<Task>();
        for (int i = 0; i < 10; i++) {
            tasks.Add(Task.Run(() => {
                try {
                    if (random.Next(0, 2) == 0) {
                        Interlocked.Increment(ref num1);
                        Throw1();
                    }
                    else {
                        Interlocked.Increment(ref num2);
                        Throw2();
                    }
                }
                catch (Exception ex) {
                    Console.WriteLine(ex);
                }
            }));
        }

        Task.WaitAll(tasks.ToArray());
        Console.WriteLine("num1: " + num1);
        Console.WriteLine("num2: " + num2);
        Console.ReadKey();
    }

    static void Throw1() {
        throw _test;
    }

    static void Throw2() {
        throw _test;
    }
}

Here we have 2 methods, Throw1() and Throw2() which both throw the same instance of exception from private field. Then we run 10 threads which call either Throw1() or Throw2() randomly and print what has been thrown. Example output of such code is:

System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
System.Exception: test
   в ConsoleApp8.Program.Throw1() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 52
   в ConsoleApp8.Program.<>c__DisplayClass1_0.<Main>b__0() в H:\VSProjects\SoHelp\ConsoleApp8\Program.cs:строка 32
num1: 6
num2: 4

So while Throw1() has been called 6 times and Throw2() has been called 4 times - all 10 stack traces we printed refer to Throw1() method.

So just don't ever do that, because there is absolutely no reasons to.

Sign up to request clarification or add additional context in comments.

11 Comments

@P.Brian.Mackey I don't agree, because if you try to invoke Throw1() and Throw2() from my example sequentially (from the same thread) - catch will print 2 different stack traces, even though exception is same. And Exception has many mutable fields, one example is Message itself, but there are much more.
@P.Brian.Mackey I don't see how it's related to static constructors at all. _test exception is created once and thrown multiple times, then various calls mutate its internal state. And no - throw does not create new exception, which you can verify by comparing exception caught in catch with _test, using object.ReferenceEquals. And what you call a "side effect" is basically what this question is about.
@P.Brian.Mackey note that question is not about rethrowing caught exception - that's perfectly fine of course. Question is about storing one exception instance in a field and then throw it from multiple completely unrelated places, instead of just 'throw new Exception("test")'. This one is bad.
@P.Brian.Mackey field initialization is already kind of in static constructor. Static field initializers behave the same way. Anyway, problem is not how Exception instance is created. It's created only once, on one thread. Problem is it's being modified later, from multiple threads. When you do Console.WriteLine(ex) for example, this is the same as Console.WriteLine(ex.ToString()). ToString() of Exception calls various methods, and those methods modify private fields on exception. This is not thread safe.
@P.Brian.Mackey for example, ToString() calls GetClassName private method, which does roughly this: if (_className == null) _className = GetClassNameIntl(); return _className. This way, innocent call to ToString() modifies private fields of Exception. And we are doing this from multiple threads in this case, which leads to observed behavior. All that because Exception is not thread-safe, and it should not be, because you should not use it like this.
|
3

No. Throwing the same instance of Exception from multiple places in your code (not counting rethrows!) is not recommended.

An Exception contains more then just it's message - it contains valuable information for debugging such as the stack trace. (Update: just tested that now. the stack trace is probably added to the exception in the throw statement, so it's not relevant to this answer) and TargetSite (from my tests, seems like it is being populated the first time the exception is thrown, but never again after that).

Throwing the same instance of Exception in different places in your code would deny you the ability to use some of this data.

2 Comments

Stack trace will actually be different. If you throw that same exception from different methods - you will see in catch block that stack trace (and properties like TargetSite) changes every time. Not that I argue that this is a bad practice though...
@Evk Yes, just tested that now. the stack trace is probably added to the exception in the throw statement

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.