views:

648

answers:

5

My co-worker is using a third-party .NET library for which we don't have the source code. We're using a ThreadPool to have a lot of threads call into this library, and occasionally one of the threads will just hang forever while the rest of them merrily chug along.

So we want to use the dreaded Thread.Abort to kill such threads. I've done this before when spinning up my own threads, but I've never used a ThreadPool. If we track the start times of each task like this:

static Dictionary<Thread, DateTime> started = new Dictionary<Thread, DateTime>();

static void DoSomeWork(object foo)
{
    lock(started)
        started[Thread.CurrentThread] = DateTime.Now;

    SomeBuggyLibraryThatMightInfiniteLoopOrSomething.callSomeFunction(doo);

    lock(started)
        started.Remove(Thread.CurrentThread);
}

then can we lock and iterate over the running threads and call Thread.Abort to kill them? And if we do, then will we need to add a new thread to the ThreadPool to replace the one that we just killed, or will the ThreadPool handle that for us?

EDIT: I'm very aware of all of the potential problems with Thread.Abort. I know that it should ideally never be used in production code, and that it doesn't necessarily even stop the thread, and that if you abort a thread while the thread has acquired a lock, then you can hang up other threads, etc. But right now we're on a tight deadline and we have decent reason to believe that in this one particular case, we can call Thread.Abort without putting the entire process in jeopardy, and we'd like to avoid rewriting this program to eliminate the ThreadPool unless we absolutely have to.

So what I want to know is this: given that we WILL be calling Thread.Abort on a thread that belongs to a ThreadPool, are there any special problems caused by these being ThreadPool threads, and do we have to manually spin up a new thread to replace the one that got killed or will the ThreadPool do that for us?

+9  A: 

No, you shouldn't call Abort on threads in the thread pool. From my local testing, it seems that the ThreadPool does recreate threads if you abort them - I aborted 1000 thread pool threads and it was still working. I don't know if you should rely on this behaviour, but maybe you can get away with it in this case. In general though using Thread.Abort is not the right way to do this.

The correct way to call a function you don't trust to behave well is to start it in a new process and kill the process if necessary.

Mark Byers
Why is this? I'm not very familiar with .NET, so can you please explain how ThreadPool threads are different than normal Threads? For what it's worth, I've killed threads in the past that hang on this library when not using a ThreadPool. Given that, is there any reason to believe that it wouldn't work to call Thread.Abort on a ThreadPool thread here? (I know that this is bad practice in general, but we're on a deadline and don't have time to rewrite much of this, so a quick fix would be useful right now.)
Eli Courtwright
Can you explain it better?
Mauro Destro
@Eli: You shouldn't call Thread.Abort on normal threads either. For a start, Thread.Abort isn't even guaranteed to terminate the thread. Even if it does successfully abort the thread, you don't know what mess you've left behind or how to clean it up. You could end up with undisposed resources. It's really not a good idea.
Mark Byers
-1. Sorry, I usually don't downvote answers whose intentions are good, but Eli clearly *knows* that `Thread.Abort` is evil and asked if the situation was any different (i.e. more evil) with thread pool threads. Just repeating the (well-known) fact that `Thread.Abort` is evil does not help, neither does it answer the question (i.e. whether a ThreadPool thread will return to the pool after a Thread.Abort).
Heinzi
If the buggylibrary is a .NET assembly you would assume it's not intended to be launched in a new process. If you're really paranoid it will hang your app you can make the threads Background, or even put it in a new AppDomain
Chris S
@Heinzi: Sorry, but I think this had to be said by someone. Even if this answer is too late to help the OP, I hope it helps someone else.
Mark Byers
@Mark: I see your point, and I like your edit. -1 changed to +1. :-)
Heinzi
+2  A: 

The use of Thread.Abort is not adviced, because it can leave your application in an invalid state. The reason for this is that when demanding a thread to abort, an exception can be raised in almost any possible place in that 3rd party library. It's possible that there are code segments that aren't written to be able to coop these asynchronous aborts.

So while it is generally not advised to abort threads, there are hosts that are very aggressive in aborting threads. One of them is ASP.NET. When a request takes too long, it will abort the thread for you. So with this in mind it's silly to say "never abort threads".

I advice you to find out where this code hangs (the stack trace of ThreadAbortException should give you a lot of information). If it always hangs on the same place (it's probably a deadlock), find out with Reflector if aborting a thread at that point will result in some state corruption. With that information you can perhaps already fix the problem (perhaps you lock on a object of that library) or can send a mail to the writer of that library. If this all doesn't help and you see there is not a risk in aborting it, be pragmatic and kill it :-)

However, if there is a change of any state corruption, you should try to go with Mark Byers' answer. That is: try running that library in its own AppDomain. This way you can unload the complete AppDomain and there is no change in it affecting your application.

Steven
Thanks for the analysis. So if I do call Thread.Abort on a ThreadPool thread, then will I need to create a new thread to replace it, or will the ThreadPool see that the thread exited and create a new one automatically?
Eli Courtwright
I just opened Reflector to see what's going on inside the ThreadPool, but it's not clear to me. I suspect the aborted threads to simply return to the thread pool, but I'm not sure. It will not be hard to test this. Create a lot of threads that sleep and abort those and see if the threadpool starts throwing exceptions or returns false.
Steven
+1 for "It's silly to say 'never abort threads'."
Eamon Nerbonne
+3  A: 

To clarify what Mark is saying, if you call Thread.Abort you have no idea where it will abort in the 3rd party component due to the special nature of the ThreadAbortException - it could leave a FileStream open for example.

I would personally create the Threads myself, referenced in a IList or Queue (as the ThreadPool is better suited for fire and forgets or WaitHandles), and depending on if you think aborting a thread that is using the 3rd party component isn't that dangerous, Abort it.

If you think aborting may leave the 3rd party library in an unacceptable state, Join the threads that haven't completed one by one, after a set amount of time using a System.Threading.Timer

Alternatively

To stick with using ThreadPool, you could use this Smart Thread Pool instead.

Chris S
+1  A: 

You have another option (which I would take if I had a free day in front of me):

reverse-engineer the third-party component using reflector and actually fix the blocking call to make it asynchroneous.

Florian Doyon
I agree that this would be a better solution, but unfortunately we don't have time for that right now. Maybe when this project is over.
Eli Courtwright
I think you might just be running into a race condition in the 3rd party code. Can you try to make the calls to this API mutually-exclusive?
Florian Doyon
@Florian: Because calls into this library from other threads continue to work, I'm guessing it's a networking problem. Like a socket was set to never time out, or maybe it's looping waiting for some input from the wacky proprietary network protocol it uses. And we really need to be concurrently running lots of threads concurrently, since we're simultaneously connecting to lots of different computers on our corporate intranet.
Eli Courtwright
+1  A: 

Read 'Abortable Thread Pool' by Stephen Toub. He provides source code for an abortable thread pool. Its an interesting read.

He calls his own callback 'HandleItem' when queueing the threadpool. Inside 'HandleItem' he then executes the actual callback, after adding the current thread to a dictionary list inside his wrapper class.

ThreadPool.QueueUserWorkItem(new WaitCallback(HandleItem));

HandleItem(...) {
...
_threads.Add(item, Thread.CurrentThread);
...
}

He uses a Dictionary to Link his WorkItems to Threads, when a user wants to cancel a thread it does a look up and cancels that particular thread.

Dictionary<WorkItem, Thread>() _threads = New Dictionary<WorkItem, Thread>();

http://msdn.microsoft.com/en-us/magazine/cc163644.aspx

Shiftbit