How to kill a thread in C# effectively?

Question

I am not trying to beat a dead horse, honestly. And I've read all the advice on thread killing, however, please consider the code. It does the following:

1. It starts a thread (via StartThread method)
2. It calls the database looking for anything in the ServiceBroker queue. Note the WAITFOR command - it means that it will sit there until there is something in the queue. All this in MonitorQueue method.

3. Kill the thread. I tried .Interrupt - it seems to do absolutely nothing. Then I tried .Abort, which should never be used, but even that did nothing.

   Thread thxMonitor = new Thread(MonitorQueue);
   void StartThread() {
       thxMonitor.Start();
   }
   
   void MonitorQueue(object obj) {
       var conn = new SqlConnection(connString);
       conn.Open();
       var cmd = conn.CreateCommand();
       cmd.CommandTimeout = 0; // forever and ever
       cmd.CommandType = CommandType.Text;
       cmd.CommandText = "WAITFOR (RECEIVE CONVERT(int, message_body) AS Message FROM SBQ)";
   
       var dataTable = new DataTable();
       var da = new SqlDataAdapter(command);
   
       da.Fill(dataTable);
       da.Dispose();
   }
   
   void KillThreadByAnyMeansNecessary() {
       thxMonitor.Interrupt();
       thxMonitor.Abort();
   }
   

Is it actually possible to kill a thread?

Senior

It is very possible to kill a thread. To me it looks like the WAITFOR is your problem. The WAITFOR is a blocking call outside of your application and won't be able to throw an exception which is what your Abort() is doing. – Pinto

@nmaait That's the entire point of WAITFOR - that I don't have to poll the database waiting for stuff. – Senior

TerminateThread – Violate

Hmm, now I'm kind of curious -- why aren't you using TerminateThread? – Violate

@Mehrdad - probably because, at best, it would leave the DB connections and ServiceBroker callbacks in an err.. 'unfriendly' state. – Montenegro

@Mehrdad Because TerminateThread "is a dangerous function", its right there in the documentation you linked. – Juanjuana

@Justin: Oh, I'm not encouraging him to do that, but it seems like he's really intent on killing the thread so it made me wonder why he's not using it.... – Violate

Answer 1

Set an Abort flag to tell the thread is needs to terminate. Append a dummy record to the ServiceBroker queue. The WAITFOR then returns. The thread then checks its 'Abort' flag and, finding it set, deletes the dummy record from the queue and exits.

Another variant would be to add a 'real' poison-pill record to the specification for the table monitored by the ServiceBroker - an illegal record-number, or the like. That would avoid touching the thread/s at all in any direct manner - always a good thing:) This might be more complex, especially if each work thread is expeceted to notify upon actual termination, but would still be effective if the work threads, ServiceBroker and DB were all on different boxes. I added this as an edit because, having thought a bit more about it, it seems more flexible, after all, if the threads normally only communicate via. the DB, why not shut them down with only the DB? No Abort(), no Interrupt() and, hopefully, no lockup-generating Join().

Montenegro

If it works, it's much cheaper checking a local boolean on every WAITFOR return than timeout-polling the DB. Also, your thread should commit suicide quite quickly - no need to wait for the next poll. A similar 'artificially satisfy the wait condition' approach works well on some other blocking calls too - opening a temporary localhost connection to force an accept() call to return and creating a temporary file to get a folder monitor API to return. – Montenegro

Of course, in many situations where one uses a queue, the whole purpose of the queue is that there are either multiple readers, or that you don't know if there is actually a reader listening. Stepping back for a second it's generally correct to say that one of the reasons for having a queue is to decouple readers from writers. While this solution may work in your case, this is, in the general case, not a good solution. – Weingarten

@DaveMarkle - The OP did not ask for explicit notification of termination, so a reader not actually being available when a termination is requested does not seem to be an issue. If it is, the thread should signal its expiry, eg. by calling an 'OnTerminate' event passed in as a creation parameter. Similarly, when handling multiple readers, (OnTerminate calls a countdown latch). My solution avoids continually opening a, (possibly sometimes high-latency), connection to the ServiceBroker, CPU-wasting polling and the shutdown latency associated with it. If this is not a good solution, what is? – Montenegro

This is just a DB-oriented variant of the 'poison pill' approach commonly used to signal threads to terminate when the only way of communicating with them is via a producer-consumer queue, (eg. threadpool threads). Another variant would be to add a 'real' poison-pill record to the specification for the table monitored by the ServiceBroker - an illegal record-number, or the like. That would avoid touching the thread/s at all in any direct manner. The poisonous record could be deleted by the thread that initiated the termination when it is notified that all work thread/s have terminated. – Montenegro

Any solution has tradeoffs. Poison pills have the issue that you're enqueueing them, so if there's a lot of work to be done, that work has to complete before the poison pill gets seen by the consumer, again causing a delay. And they can be an acceptable solution for in-memory queues or queues where you really know there is only one consumer, but IMO they're probably not a good solution in the general case for database queues, and IMO the lack of determinism about how long it will take the thread to shut down is a major downside as well. Either way I think this debate is enlightening... – Weingarten

Database systems are riddled with non-determinism when it comes to shutting down anyway. Shutting down Oracle often takes so long that Admins have died while waiting for it. One problem I see with polling is that it requires continual setting up of WAITFOR with the DB. Apart from the latency issue, there may be periods when records can be added to the table with no WAITFOR set up because it has just timed out? What happens then, (I do not know, not tried, maybe not an issue)? – Montenegro

The lack of determinism I referred to has nothing to do with the shutdown time of the server -- it has to do with the number of messages in the queue. If there are a lot of unprocessed messages in the queue, and you want the client to stop the thread immediately without processing any more messages, a poison pill doesn't "get you there" because in order to see the pill, you have to first sift through all of the messages in the queue. – Weingarten

..or purge the queue before adding the pill. – Montenegro

@MartinJames In retrospect, an obvious solution, especially since my custom web server class sitting next this code terminates in this same manner (e.g. sending it a "SHUTDOWN" message). Doh. – Senior

How do you get the new data out to the clients - AJAX? – Montenegro

Answer 2

I hate to not answer your question, but consider going about this a different way. T-SQL allows a TIMEOUT parameter to be specified with WAITFOR, such that if a message is not received in a certain period of time, the statement will quit and have to be tried again. You see this over and over again in patterns where you have to wait. The tradeoff is that you don't immediately get the thread to die when requested -- you have to wait for your timeout to expire before your thread dies.

The quicker you want this to happen, the smaller your timeout interval. Want it to happen instantly? Then you should be polling instead.

static bool _quit = false;

Thread thxMonitor = new Thread(MonitorQueue);
void StartThread() {
    thxMonitor.Start();
}

void MonitorQueue(object obj) {

    var conn = new SqlConnection(connString);
    conn.Open();
    var cmd = conn.CreateCommand();
    cmd.CommandType = CommandType.Text;
    cmd.CommandText = "WAITFOR (RECEIVE CONVERT(int, message_body) AS Message FROM SBQ) TIMEOUT 500";

    var dataTable = new DataTable();    

    while(!quit && !dataTable.AsEnumerable().Any()) {
        using (var da = new SqlDataAdapter(command)) {    
            da.Fill(dataTable);
        }
    }
}

void KillThreadByAnyMeansNecessary() {
    _quit = true;
}

EDIT:

Although this can feel like polling the queue, it's not really. When you poll, you're actively checking something, and then you're waiting to avoid a "spinning" condition where you're constantly burning up CPU (though sometimes you don't even wait).

Consider what happens in a polling scenario when you check for entries, then wait 500ms. If nothing's in the queue and 200ms later a message arrives, you have to wait another 300ms when polling to get the message. With a timeout, if a message arrives 200ms into the timeout of the "wait" method, the message gets processed immediately.

That time delay forced by the wait when polling vs. a constant high CPU when polling in a tight loop is why polling is often unsatisfactory. Waiting with a timeout has no such disadvantages -- the only tradeoff is you have to wait for your timeout to expire before your thread can die.

Weingarten

beat me by 48 seconds AND you used code. You are the winner! – Veal

Yeah, that's what I ended up doing, but I am hating this. It still feels like polling. – Senior

@DaveMarkle I appreciate the effort, but I fail to see how this is not polling - you are looping every 500ms. Granted, it's not using a hammer to beat the everliving crap out of the database, but it's at least one of those kiddy baseball bats hitting me on the head when I'd rather it didn't. – Senior

Fair enough. You could look at my example as not polling the queue, but instead polling on the static signal variable, which would be true. In the Windows API you can completely avoid polling by calling WaitForMultipleObjects and waiting for both a monitor and another event. But this is not the Windows API -- you don't have a function that will receive from the queue OR quit when another object signals. That makes this solution your simplest and most reliable choice. – Weingarten

I've used polling on many occasions, and I have to point out that polling is not evil, except in the cartoon version of computer science. Polling that uses resources, like CPU, that could otherwise be put to good use - is usually undesirable. It's always worth asking what is the actual cost of polling, and is avoiding that cost worth the engineering effort. – Mceachern

Answer 3

Don't do this! Seriously!

The function that you need to call to kill a thread is the TerminateThread function, which you can call via P/Invoke. All the reasons as to why you shouldn't use this method are right there in the documentation

TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination. For example, TerminateThread can result in the following problems:

• If the target thread owns a critical section, the critical section will not be released.
• If the target thread is allocating memory from the heap, the heap lock will not be released.
• If the target thread is executing certain kernel32 calls when it is terminated, the kernel32 state for the thread's process could be inconsistent.
• If the target thread is manipulating the global state of a shared DLL, the state of the DLL could be destroyed, affecting other users of the DLL.

The important thing to note is the bit in bold, and the fact that under the CLR / .Net framework you are never in the situation where you know exactly what the target thread is doing (unless you happen to write the CLR).

To clarify, calling TerminateThread on a thread running .Net code could quite possibly deadlock your process or otherwise leave in a completely unrecoverable state.

If you can't find some way to abort the connection then you are far better off just leaving that thread running in the background than trying to kill it with TerminateThread. Other people have already posted alternative suggestions on how to achieve this.

---

The Thread.Abort method is slightly safer in that it raises a ThreadAbortException rather than immediately tearing down your thread, however this has the disadvantage of not always working - the CLR can only throw the exception if the CLR is actually running code on that thread, however in this case the thread is probably sat waiting for some IO request to complete in native SQL Server Client code instead, which is why your call to Thread.Abort isn't doing anything, and won't do anything until control is returned to the CLR.

Thread.Abort also has its own problems anyway and is generally considered a bad thing to be doing, however it probably wont completely hose your process (although it still might, depending on what the code running is doing).

Juanjuana

Answer 4

instead of killing your thread, change your code to use WAITFOR with a small timeout.

after the timeout elapses, check to see if the thread has been interrupted.

if not, loop back around and do your waitfor again.

Yes, "the entire point" of waitfor is to wait for something. But if you want something to be responsive, you can't ask one thread to wait for Infinity, and then expect it to listen to anything else.

Veal

Answer 5

It is not just easy to terminate the thread right away. There is a potential potential problem associated with it:

Your thread acquire a lock, and then you kill it before it releases the lock. Now the threads who require the lock will get stuck.

You can use some global variable to tell the thread to stop. You have to manually, in your thread code, check that global variable and return if you see it indicates you should stop.

Please refer to this question discussing the same thing: How to kill a thread instantly in C#?

Emmet