Calling Thread.Abort on a thread from a ThreadPool
Asked Answered
A

5

20

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?

Asthenic answered 25/2, 2010 at 15:36 Comment(4)
"[Aborting a ThreadPool thread] is perilous, however. By the time the notification is received and the thread is aborted, the original work item could have already finished, such that the pool's thread already moved on to another work item. You would be aborting that secondary work item rather than the desired one, opening yourself up to a world of hurt. " - Stephen Toub, MSDN Magazine 06-03Syllabism
@Syllabism There is nothing in the OPs most about ThreadPool.QueueUserWorkItem that would make that March 2006 MSDN Magazine quote relevantBengaline
@MickyD: the quote perfectly illustrates the following concern raised by the OP: "are there any special problems caused by these being ThreadPool threads"Syllabism
@Syllabism That Stephen Toub quote from .NET Matters was written in March 2006 which predates the .NET 4.0 announcement and 1st .NET 4.0 Public beta in 2008 and 2009 respecively. .NET 4.0 introduced TPL (which introduced async/await and Task). The article refers to the explcit class ThreadPool and ThreadPool.QueueUserWorkItem as per .NET 2. Most importantly the article refers to faults that can happen in the ThreadPool class if used with QueueUserWorkItem when threads are aborted, something OP isn't usingBengaline
H
8

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.

Hackett answered 25/2, 2010 at 15:40 Comment(6)
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.)Asthenic
@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.Hackett
-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).Shealy
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 AppDomainRobedechambre
@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.Hackett
@Mark: I see your point, and I like your edit. -1 changed to +1. :-)Shealy
V
8

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 cope with 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.

Vinificator answered 25/2, 2010 at 16:1 Comment(2)
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?Asthenic
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.Vinificator
L
4

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

Leatherleaf answered 8/7, 2010 at 18:43 Comment(2)
Thanks. This was just the thing I was looking for. I too have some third-party code (word to PDF conversion) that can get stuck in a loop for certain Word docs. Only way to abort is, as per the vendor, Thread.Abort(). They won't adjust to take CancellationTokens. This AbortableThreadPool at least lets me abstract it away a bit. I've updated it to take in Action/Func, as well as a TimeSpan indicating the time by which Cancel should be called automatically. A TaskCompletionSource lets me treat it as a Task so I can "pause" by using async/await knowing there's a max pause timeThespian
Tested this today, note that it doesn't work as expected in most cases: it uses the deprecated Thread.Abort() when abort is required (forced). All other cases do not support awaitable functions. So it's really quite limited. See this Fiddle (note: JSFiddle doesn't work properly with awaitables, so download the script and try it locally on e.g. LinqPad).Thiourea
R
3

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.

Robedechambre answered 25/2, 2010 at 16:30 Comment(0)
T
1

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.

Taite answered 25/2, 2010 at 16:35 Comment(3)
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.Asthenic
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?Taite
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.Asthenic

© 2022 - 2024 — McMap. All rights reserved.