What's wrong with using Thread.Abort()
Asked Answered
I

7

74

So I know that you shouldn't use

Thread.Abort()

But I've never been given a good explanation. Is there a performance penalty or some hidden gotcha?

I know you can't ignore/swallow the ThreadAbortException (which makes sense)

Impala answered 13/10, 2009 at 10:1 Comment(1)
There are many threads on SO: google.com/search?q=thread.abort()+site%3Astackoverflow.comManiacal
L
96

In addition to all of the other good answers here, let me add that there is no guarantee whatsoever that a call to Thread.Abort will actually abort the thread in question, ever. It is possible (though not particularly easy) to "harden" a thread against being aborted. If, for example, you are aborting a thread because you believe it to be running hostile code then the hostile code could be resisting its own destruction.

If you have a long-running operation involving code that you do not own that must be taken down cleanly, the correct way to do this is to put that code in its own process, not its own thread. (And preferably in a highly security-restricted appdomain in that process.) You can then cleanly kill the process.

In short, Thread.Abort is at best indicative of bad design, possibly unreliable, and extremely dangerous. It should be avoided at all costs; the only time you should ever even consider aborting a thread is in some sort of "emergency shutdown" code where you are attempting to tear down an appdomain as cleanly as possible.

Laryngeal answered 13/10, 2009 at 14:24 Comment(5)
I have one of the situations you highlight. Which is someone elses code (an IronPython Console) that has a Run(...) function which is expecting to be run in a thread. BTW, Do you have an example of 'hardening' a Thread?Impala
Do a web search on "Constrained Execution Regions" and you'll learn more than you want to know about how to mess around with thread abort exception propagation.Laryngeal
Shouldn't you just use AppDomain.Unload to tear down an app domain? AppDomain.Unload will call Thread.Abort on all threads running in the domain for you.Birr
Ahhh, I get it, you use Thread.Abort so that you don't get a CannotUnloadAppDomainException if the aborting takes some time. By using Thread.Abort, you can wait for the thread to really abort, and only then call AppDomain.Unload.Birr
out of interest have you clocked that ASP.NET WebForms thread.Abort()s by design on a redirect to avoid running the code for the rest of the page? :DPickwickian
P
21

Because if you know that the thread is in some safe state in which it can be aborted, surely you can arrange better communication and have the thread exit cleanly.

The thread could have taken a lock and be in the middle of changing some shared state, and the Thread.Abort will undo the lock and leave the shared state corrupted.

Prog answered 13/10, 2009 at 10:5 Comment(4)
This can be prevented with the use of msdn.microsoft.com/en-us/library/…, although I'd consider this quite brittle!Chronogram
It is possible to wrap the body of your thread in a try/finally and them clean up your resources in the finally block. But in general I see what you mean.Impala
@RobFonseca-Ensor Not just brittle. Have you ever tried writing abort-safe code? It makes mere thread-safety look trivial. The Abort can happen anywhere (in managed code) - even the lock statement itself actually isn't abort-safe. So you could not only be left with a corrupted state, you could in fact have a lock that's impossible to release - it will be released when the thread dies, of course, but that doesn't actually have to happen, ever (until the process dies).Federicofedirko
@JanBannister It's a lot harder than you might think - you have to realize that the abort can happen in any part of the code, even parts where you don't expect any exceptions whatsoever. Can you really write a finally clause that correctly handles all that? It can be made much easier by avoiding shared state, of course, and only updating the shared state when in a safe, constrained region... but then you're well on your way to not needing the explicit shared state already.Federicofedirko
P
14

It's easier to hurt yourself. As others have stated, it raises an exception in the code, which can occur at any point. This might be fine if you expect this and have coded in a way that elegantly handles this exception at any point, but some people don't:

Monitor.Enter(obj);
// some code - if exception is raised here, then the lock isn't released
Monitor.Exit(obj)

IDisposable someCriticalResource = GetResource();
// some code - if exception is raised here, then the object isn't disposed
someCriticalResource.Dispose();

Additionally, if you're working with many people on a team, unless you have good code reviews, you cannot guarantee the quality of the code you'll be working with. Hence it is a good idea to preach the gospel of "no Thread.Abort()" than it is to get people to remember to write code that is robust against exceptions occurring anywhere within that code.

Pickwickian answered 13/10, 2009 at 12:26 Comment(1)
An important point here is that using is not abort-safe either - there's a (tiny) window of opportunity between creating the instance and the start of the try-finally block. It's a great recipe for subtle bugs you will not ever be able to reproduce and that can easily corrupt your data, hang or kill your application etc.Federicofedirko
M
10

In short. Any IDisposable object may not be disposed. Any locked object may not be unlocked. Anything that must be 100% performed will never be done.

Maillol answered 13/10, 2009 at 11:6 Comment(5)
Anything that must be 100% performed will fail sooner or later due to a power outage...Misreport
This is incorrect, Abort will execute finally blocks and hence it will dispose IDisposable also it releases any locks due to same reason.Multicolor
@ByteBlast What do you mean? using statement is irrelevant here. key is finally will run. Doesn't matter you use using statement or not.Multicolor
@SriramSakthivel: I am rather late to be making this correction, but better late than never. Finally blocks only run if the try is entered. using(acquire) body; is { acquire; try { body } finally { dispose }}. What if the abort happens after the acquire step but before the try is entered? Or, worse: what if the acquire step is new Foo() where the constructor opens a file handle and assigns it to a field; what if the abort happens after the open but before the assignment? This answer is absolutely correct; an abort can cause a disposable resource to go undisposed.Laryngeal
@SriramSakthivel: Also, again, locks are only released if the finally runs. Before C# 4, there was a possibility that the monitor was entered before the try block; in C# 4 we changed the codegen to prevent this possibility. But remember, some people write monitor enter/exit pairs without try-finally, and I would argue that this is actually safer; an abort can indeed cause a monitor to be entered but never exited. But that is actually not the bad case. The bad case is that we enter a lock, we mutate state to violate an invariant, thread abort before fixing, unlock, and expose the mutation!Laryngeal
S
6

When you call Thread.Abort() on another thread, a ThreadAbortException is injected in the flow of that thread. If you're lucky, the code will handle this well and abort in a well-defined state. The problem is that you have no way to figure out if you will be lucky in every case, so if you prefer safe over sorry, calling Thread.Abort on other threads is not a good idea.

Sinnard answered 13/10, 2009 at 10:5 Comment(0)
G
4

Thread.Abort stops your thread in an uncontrolled fashion. thread.Abort will throw an exception, which will cause that your thread stops immediately.

What is wrong with that: in most cases, you want to gracefully stop the operation that you're performing. For instance, if you are executing an ACID operation, you might want to complete the current operation before ending the thread, so that your system remains in a stable state.

Goshawk answered 13/10, 2009 at 10:4 Comment(0)
S
2

Thread.Abort rises an exception in the target thread. Target thread in the meantime can be performing some critical operations and rising an exception can break your application state.

Suffice answered 13/10, 2009 at 10:5 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.