views:

810

answers:

4

I've seen a number of examples that have a thread procedure that looks like this.

    private void ThreadProc()
    {
        while (serviceStarted)
        {
            // do some work

            Thread.Sleep(new TimeSpan(0, 0, 5));
        }

        Thread.CurrentThread.Abort();
    }

Is the Abort() really necessary at the end? I thought once the procedure exited that it cleaned up after itself. Additionally doesn't calling Abort() throw an exception, which is generally more resource intensive than just exiting a procedure. I'd like to read an explanation for why this is or isn't a good practice.

+1  A: 

Well for one, yes calling Thread.Abort() does raise an exception, and if you're writing code that will be re-used (or part of a base library) it's difficult for other developers to handle ThreadAbortExcpetions.

It's explained in this article about Reliability Best Practices.

I've always heard that calling Thread.Join() is a better way to do it, if you can wait until the thread is completed processing.

In general, I don't know if anyone think's it's really a good practice. It can cause deadlocks (because unmanaged resources aren't properly cleaned up when you throw an exception)

Here's another article about it, and other methods to deal with the issue.

Hope that helps!

Zachary Yates
A: 

Interesting question. But i would advise against it since such a statement would prevent the method from being reused easily.

Y Low
+1  A: 

Calling Abort() on one's own thread is safe, but apart from that it should generally be avoided because you can't be sure other threads will terminate gracefully. In many cases you don't need to abort the thread. Just let it finish and it will be reclaimed.

Brian Rasmussen
+1  A: 

Once the loop exits, the thread will terminate on its own. There is not need to abort the thread.

The CurrentThread.Abort is not only superfluous, but genuinely harmful since it raises a ThreadAbortException. If another thread attempts to Join() your service loop thread, it will have to handle an exception unnecessarily. Your best bet is just to delete the line "CurrentThread.Abort()".

Juliet