Catching System.Exception in thread

Question

I know it is not good practice to catch System.Exception unless on the top level of an application. What about a thread? Is it okay to catch System.Exception on the top level of the thread?

Update: The thread is a long running thread that should only be terminated when the application stops. So in order to make sure that the application does not crash I simply catch System.Exception and log the error. Everything is recreated.

        while (Terminate == false)
        {
            var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());

            try
            {
                var criteria = new FindCriteria(typeof(T));
                criteria.Scopes.Add(new Uri(Scope));
                var discovered = discoveryClient.Find(criteria);
                discoveryClient.Close();
                discoveryClient = null;

                // do something with the endpoints
            }
            catch (OutOfMemoryException e)
            {
                m_Logger.LogException(e, "Exception when trying to discover clients (Contract: {0})", typeof(T).Name);
                throw;
            }
            catch (Exception e)
            {
                m_Logger.LogException(e, "Exception when trying to discover clients (Contract: {0})", typeof(T).Name);

                if (discoveryClient != null)
                    (discoveryClient as IDisposable).Dispose();
            }

        }

Answer 1

It depends on what the thread is doing and the context of the thread in your application. Generally speaking, you should follow the golden rule of thumb: do not catch an exception unless you know how to handle it. (I am simplifying of course, but it's a rule of thumb).

Since we are talking about System.Exception and not some subclass, I assume that you would not in fact know how to handle the exception. Logging the error and letting the application terminate (the only legitimate reason to catch an exception you can't handle) can be done without catching the exception from within the thread that raised it, so the short answer is no, it's no OK.

If I remember correctly .NET 1 actually caught and swallowed all exceptions raised on background threads. This led to so many problems in badly written programs that MS changed the behavior in .NET 2 to let the exceptions crash the app -- and you can imagine they had very good reason to make such a breaking change.

Update regarding BackgroundWorker:

Please do not mistake the usage model of BackgroundWorker for "swallowing System.Exception is OK". Here's what the docs for BackgroundWorker.RunWorkerCompleted say:

Your RunWorkerCompleted event handler should always check the AsyncCompletedEventArgs.Error and AsyncCompletedEventArgs.Cancelled properties before accessing the RunWorkerCompletedEventArgs.Result property. If an exception was raised or if the operation was canceled, accessing the RunWorkerCompletedEventArgs.Result property raises an exception.

Choosing to disregard this suggestion or ignoring the return value on purpose (otherwise an exception will still be thrown!) is, simply put, bad programming practice.

Answer 2

Yes - it's an excellent idea :)

Umm, I can't see why people are suggesting it's not a good idea to catch top-level exceptions. Seriously, you don't catch them?

From an actual day-to-day development standpoint, if your background thread has a top-level exception, you want to know about it. You don't want the app to crash, the default behaviour - users don't like that, surprisingly. This is one of the few places you definitely want to catch exceptions and log/recover.

Answer 3

Officially it's not good practice, but sometimes it is done.

The arguments are exactly the same as they are for doing this on a "main" thread.

One major problem is that if you swallow errors like this, then your application may function very incorrectly - e.g. it might overwrite crucial user data - rather than terminating.

---

If you do chose to go this route, you might want to be careful to exclude ThreadAbortException from your catching - this is an "expected exception" if anyone ever aborts a worker thread (which may or may not be the case in your app)

Answer 4

Only handle exception you could do something about. To not show yellow screen I personally prefer to catch unhandeld excetpion in the global.asax event Application_error

Answer 5

We are generally not meant to create new thread directly. Only in a few circumstances it is acceptable.

So if you do use new Thread() especially in UI , that would be frowned upon.

It is encouraged to use BackgroundWorker or Task<T> which encapsulates thread exception handling so no exception handling in terms of catch block would be necessary.

---

These 4 scenarios are acceptable cases where you want your own thread (From CLR via C#):

I highly recommend that you use the thread pool to execute asynchronous compute-bound operations whenever possible. However, there are some occasions when you might want to explicitly create a thread dedicated to executing a particular compute-bound operation. Typically, you'd want to create a dedicated thread if you're going to execute code that requires the thread to be in a particular state that is not normal for a thread pool thread. For example, I'd create a dedicated thread if I wanted the thread to run at a special priority (all thread pool threads run at normal priority, and you should not alter a thread pool thread's priority). I would also consider creating and using my own thread if I wanted to make the thread a foreground thread (all thread pool threads are background threads), thereby preventing the application from dying until my thread has completed its task. I'd also use a dedicated thread if the compute-bound task were extremely long running; this way, I would not be taxing the thread pool's logic as it tries to figure out whether to create an additional thread. Finally, I'd use a dedicated thread if I wanted to start a thread and possibly abort it prematurely by calling Thread's Abort method (discussed in Chapter 21, "CLR Hosting and AppDomains").