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?
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.
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._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.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.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.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.
catch block that stack trace (and properties like TargetSite) changes every time. Not that I argue that this is a bad practice though...
throw exceptionInstancefrom 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.