There can only ever be one Application.ThreadException handler. Yes, you read that correctly.
To give some context, the core of our error reporting code in SmartAssembly adds event handlers to AppDomain.CurrentDomain.UnhandledException (for general exceptions) and Application.ThreadException (for winforms exceptions) to display our error reporting dialog, or send the unhandled exception details back to the application developers as a bug report.
I was recently debugging an issue where any exceptions thrown after a call to Microsoft.Data.ConnectionUI.DataConnectionDialog.Show would not be picked up by our error reporting code. In other words, this:
|
1 2 |
throw new Exception(); DataConnectionDialog.Show(new DataConnectionDialog()); |
would display our error reporting dialog, and this:
|
1 2 |
DataConnectionDialog.Show(new DataConnectionDialog()); throw new Exception(); |
would display the standard .NET winforms error dialog. At some point during the call to Show, our exception handler was disappearing.
After quite a bit of digging, I noticed something odd about the definition of ThreadException:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
public static event ThreadExceptionEventHandler ThreadException { add { IntSecurity.AffectThreadBehavior.Demand(); ThreadContext current = ThreadContext.FromCurrent(); lock(current) { current.threadExceptionHandler = value; } } remove { ThreadContext current = ThreadContext.FromCurrent(); lock(current) { current.threadExceptionHandler -= value; } } } |
Did you spot it? The add method uses = rather than += to assign the new event handler, replacing any existing handlers! And indeed, the DataConnectionDialog.Show was adding & removing it’s own exception handler during the method, deleting ours in the process. This bug is obviously a typo, as the remove method uses -=, as it should.
Microsoft has known about this since 2005, but (as of .NET 4) have yet to fix this typo. In the meantime, don’t trust that any ThreadException handlers you add will still be there if an exception gets thrown!
Load comments