Guaranteed way to cancel a hanging Task?

Question

I often have to execute code on a separate thread that is long running, blocking, instable and\or has a potential to hang forever. Since the existence of TPL the internet is full of examples that nicely cancel a task with the cancellation token but I never found an example that kills a task that hangs. Code that hangs forever is likely to be expected as soon as you communicate with hardware or call some third party code. A task that hangs cannot check the cancellation token and is doomed to stay alive forever. In critical applications I equip those tasks with alive signals that are sent on regular time intervals. As soon as a hanging task is detected, it is killed and a new instance is started.

The code below shows an example task that calls a long running placeholder method SomeThirdPartyLongOperation() which has the potential to hang forever. The StopTask() first checks if the task is still running an tries to cancel it with the cancellation token. If that doesn’t work, the task hangs and the underlying thread is interrupted\aborted old school style.

    private Task _task;
    private Thread _thread;
    private CancellationTokenSource _cancellationTokenSource;

    public void StartTask()
    {
        _cancellationTokenSource = new CancellationTokenSource();
        _task = Task.Factory.StartNew(() => DoWork(_cancellationTokenSource.Token), _cancellationTokenSource.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
    }

    public void StopTask()
    {
        if (_task.Status == TaskStatus.RanToCompletion)
            return;
        _cancellationTokenSource.Cancel();
        try
        {
            _task.Wait(2000); // Wait for task to end and prevent hanging by timeout.
        }
        catch (AggregateException aggEx)
        {
            List<Exception> exceptions = aggEx.InnerExceptions.Where(e => !(e is TaskCanceledException)).ToList(); // Ignore TaskCanceledException
            foreach (Exception ex in exceptions)
            {
                // Process exception thrown by task
            }
        }
        if (!_task.IsCompleted) // Task hangs and didn't respond to cancellation token => old school thread abort
        {
            _thread.Interrupt();
            if (!_thread.Join(2000))
            { 
                _thread.Abort();
            }
        }
        _cancellationTokenSource.Dispose();
        if (_task.IsCompleted)
        {
            _task.Dispose();
        }
    }

    private void DoWork(CancellationToken cancellationToken)
    {
        if (string.IsNullOrEmpty(Thread.CurrentThread.Name)) // Set thread name for debugging
            Thread.CurrentThread.Name = "DemoThread";
        _thread = Thread.CurrentThread; // Save for interrupting/aborting if thread hangs
        for (int i = 0; i < 10; i++)
        {
            cancellationToken.ThrowIfCancellationRequested();
            SomeThirdPartyLongOperation(i);
        }
    }

Although I’ve been using this construct for some years now, I want to know if there are some potential mistakes in it. I’ve never seen an example of a task that saves the underlying thread or gives it a name to simplify debugging, so I’m a bit unsure if this is the right way to go. Comment on any detail is welcome!

Answer 1

The question is why is this task even hanging at all? I think there's no universal solution to this problem but you should focus on the task to be always responsible and not on forcing to interrupt it.

In this code, it looks like you're looking for a simple thread rather than a task - you shouldn't link tasks to threads - it's very likely that the task will switch to another thread after some async operations and you will end up on killing an innoccent thread that is not connected to your task anymore. If you really need to kill the whole thread then make a dedicated one just for this job.

You shouldn't also name or do anything with any thread that is used for tasks' default pool. Consider this code:

static void Main(string[] args)
{
    Task.Run(sth);
    Console.Read();
}

static async Task sth()
{
    Thread.CurrentThread.Name = "My name";
    Console.WriteLine(Thread.CurrentThread.ManagedThreadId);
    await Task.Delay(1);
    Console.WriteLine(Thread.CurrentThread.ManagedThreadId);
    Console.WriteLine(Thread.CurrentThread.Name ?? "No name");
}

the output is:

3
4
No name

Answer 2

Code that hangs forever is likely to be expected as soon as you communicate with hardware or call some third party code.

Communication: absolutely not. There's always a way to timeout with communication APIs, so even with misbehaving hardware, there's no need to force-kill an I/O operation.

Third-party code: only if you're paranoid (or have high demands such as 24x7 automation).

Here's the bottom line:

• There's no way to force-kill a task.
• You can force-kill a thread, but this can easily cause serious problems with application state, possibility if introducing deadlocks in other parts of the code, and resource leaks.
• You can force-kill an appdomain, which solves a large portion of app state / deadlock issues with killing threads. However, it doesn't solve them all, and there's still the problem of resource leaks.
• You can force-kill a process. This is the only truly clean and reliable solution.

So, if you choose to trust the third-party code, I recommend that you just call it like any other API. If you require 100% reliability regardless of third-party libraries, you'll need to wrap the third-party dll into a separate process and use cross-process communication to call it.

Your current code force-kills a thread pool thread, which is certainly not recommended; those threads belong to the thread pool, not to you, and this is still true even if you specify LongRunning. If you go the kill-thread route (which is not fully reliable), then I recommend using an explicit thread.