23

Sometimes, once I have requested the cancellation of a pending task with CancellationTokenSource.Cancel, I need to make sure the task has properly reached the cancelled state, before I can continue. Most often I face this situation when the app is terminating and I want to cancel all pending task gracefully. However, it can also be a requirement of the UI workflow specification, when the new background process can only start if the current pending one has been fully cancelled or reached its end naturally.

I'd appreciate if someone shares his/her approach in dealing with this situation. I'm talking about the following pattern:

_cancellationTokenSource.Cancel();
_task.Wait();

As is, it is known to be capable of easily causing a deadlock when used on the UI thread. However, it is not always possible to use an asynchronous wait instead (i.e. await task; e.g., here is one of the cases when it is possible). At the same time, it is a code smell to simply request the cancellation and continue without actually observing its state.

As a simple example illustrating the problem, I may want to make sure the following DoWorkAsync task has been fully cancelled inside FormClosing event handler. If I don't wait for the _task inside MainForm_FormClosing, I may not even see the "Finished work item N" trace for the current work item, as the app terminates in the middle of a pending sub-task (which is executed on a pool thread). If I do wait though, it results in a deadlock:

public partial class MainForm : Form
{
    CancellationTokenSource _cts;
    Task _task;

    // Form Load event
    void MainForm_Load(object sender, EventArgs e)
    {
        _cts = new CancellationTokenSource();
        _task = DoWorkAsync(_cts.Token);
    }

    // Form Closing event
    void MainForm_FormClosing(object sender, FormClosingEventArgs e)
    {
        _cts.Cancel();
        try
        {
            // if we don't wait here,
            // we may not see "Finished work item N" for the current item,
            // if we do wait, we'll have a deadlock
            _task.Wait();
        }
        catch (Exception ex)
        {
            if (ex is AggregateException)
                ex = ex.InnerException;
            if (!(ex is OperationCanceledException))
                throw;
        }
        MessageBox.Show("Task cancelled");
    }

    // async work
    async Task DoWorkAsync(CancellationToken ct)
    {
        var i = 0;
        while (true)
        {
            ct.ThrowIfCancellationRequested();

            var item = i++;
            await Task.Run(() =>
            {
                Debug.Print("Starting work item " + item);
                // use Sleep as a mock for some atomic operation which cannot be cancelled
                Thread.Sleep(1000); 
                Debug.Print("Finished work item " + item);
            }, ct);
        }
    }
}

That happens because the UI thread's message loop has to continue pumping messages, so the asynchronous continuation inside DoWorkAsync (which is scheduled on the thread's WindowsFormsSynchronizationContext) has a chance to be executed and eventually have reached the cancelled state. However, the pump is blocked with _task.Wait(), which leads to the deadlock. This example is specific to WinForms, but the problem is relevant in the context of WPF, too.

In this case, I don't see any other solutions but to organize a nested message loop, while waiting for the _task. In a distant way, it is similar to Thread.Join, which keeps pumping messages while waiting for a thread to terminate. The framework doesn't seem to offer an explicit task API for this, so I've eventually come up with the following implementation of WaitWithDoEvents:

using System;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WinformsApp
{
    public partial class MainForm : Form
    {
        CancellationTokenSource _cts;
        Task _task;

        // Form Load event
        void MainForm_Load(object sender, EventArgs e)
        {
            _cts = new CancellationTokenSource();
            _task = DoWorkAsync(_cts.Token);
        }

        // Form Closing event
        void MainForm_FormClosing(object sender, FormClosingEventArgs e)
        {
            // disable the UI
            var wasEnabled = this.Enabled; this.Enabled = false;
            try
            {
                // request cancellation
                _cts.Cancel();
                // wait while pumping messages
                _task.AsWaitHandle().WaitWithDoEvents();
            }
            catch (Exception ex)
            {
                if (ex is AggregateException)
                    ex = ex.InnerException;
                if (!(ex is OperationCanceledException))
                    throw;
            }
            finally
            {
                // enable the UI
                this.Enabled = wasEnabled;
            }
            MessageBox.Show("Task cancelled");
        }

        // async work
        async Task DoWorkAsync(CancellationToken ct)
        {
            var i = 0;
            while (true)
            {
                ct.ThrowIfCancellationRequested();

                var item = i++;
                await Task.Run(() =>
                {
                    Debug.Print("Starting work item " + item);
                    // use Sleep as a mock for some atomic operation which cannot be cancelled
                    Thread.Sleep(1000); 
                    Debug.Print("Finished work item " + item);
                }, ct);
            }
        }

        public MainForm()
        {
            InitializeComponent();
            this.FormClosing += MainForm_FormClosing;
            this.Load += MainForm_Load;
        }
    }

    /// <summary>
    /// WaitHandle and Task extensions
    /// by Noseratio - https://stackoverflow.com/users/1768303/noseratio
    /// </summary>
    public static class WaitExt
    {
        /// <summary>
        /// Wait for a handle and pump messages with DoEvents
        /// </summary>
        public static bool WaitWithDoEvents(this WaitHandle handle, CancellationToken token, int timeout)
        {
            if (SynchronizationContext.Current as System.Windows.Forms.WindowsFormsSynchronizationContext == null)
            {
                // https://stackoverflow.com/a/19555959
                throw new ApplicationException("Internal error: WaitWithDoEvents must be called on a thread with WindowsFormsSynchronizationContext.");
            }

            const uint EVENT_MASK = Win32.QS_ALLINPUT;
            IntPtr[] handles = { handle.SafeWaitHandle.DangerousGetHandle() };

            // track timeout if not infinite
            Func<bool> hasTimedOut = () => false;
            int remainingTimeout = timeout;

            if (timeout != Timeout.Infinite)
            {
                int startTick = Environment.TickCount;
                hasTimedOut = () =>
                {
                    // Environment.TickCount wraps correctly even if runs continuously 
                    int lapse = Environment.TickCount - startTick;
                    remainingTimeout = Math.Max(timeout - lapse, 0);
                    return remainingTimeout <= 0;
                };
            }

            // pump messages
            while (true)
            {
                // throw if cancellation requested from outside
                token.ThrowIfCancellationRequested();

                // do an instant check
                if (handle.WaitOne(0)) 
                    return true;

                // pump the pending message
                System.Windows.Forms.Application.DoEvents();

                // check if timed out
                if (hasTimedOut())
                    return false;

                // the queue status high word is non-zero if a Windows message is still in the queue
                if ((Win32.GetQueueStatus(EVENT_MASK) >> 16) != 0) 
                    continue;

                // the message queue is empty, raise Idle event
                System.Windows.Forms.Application.RaiseIdle(EventArgs.Empty);

                if (hasTimedOut())
                    return false;

                // wait for either a Windows message or the handle
                // MWMO_INPUTAVAILABLE also observes messages already seen (e.g. with PeekMessage) but not removed from the queue
                var result = Win32.MsgWaitForMultipleObjectsEx(1, handles, (uint)remainingTimeout, EVENT_MASK, Win32.MWMO_INPUTAVAILABLE);
                if (result == Win32.WAIT_OBJECT_0 || result == Win32.WAIT_ABANDONED_0)
                    return true; // handle signalled 
                if (result == Win32.WAIT_TIMEOUT)
                    return false; // timed out
                if (result == Win32.WAIT_OBJECT_0 + 1) // an input/message pending
                    continue;
                // unexpected result
                throw new InvalidOperationException();
            }
        }

        public static bool WaitWithDoEvents(this WaitHandle handle, int timeout)
        {
            return WaitWithDoEvents(handle, CancellationToken.None, timeout);
        }

        public static bool WaitWithDoEvents(this WaitHandle handle)
        {
            return WaitWithDoEvents(handle, CancellationToken.None, Timeout.Infinite);
        }

        public static WaitHandle AsWaitHandle(this Task task)
        {
            return ((IAsyncResult)task).AsyncWaitHandle;
        }

        /// <summary>
        /// Win32 interop declarations
        /// </summary>
        public static class Win32
        {
            [DllImport("user32.dll")]
            public static extern uint GetQueueStatus(uint flags);

            [DllImport("user32.dll", SetLastError = true)]
            public static extern uint MsgWaitForMultipleObjectsEx(
                uint nCount, IntPtr[] pHandles, uint dwMilliseconds, uint dwWakeMask, uint dwFlags);

            public const uint QS_KEY = 0x0001;
            public const uint QS_MOUSEMOVE = 0x0002;
            public const uint QS_MOUSEBUTTON = 0x0004;
            public const uint QS_POSTMESSAGE = 0x0008;
            public const uint QS_TIMER = 0x0010;
            public const uint QS_PAINT = 0x0020;
            public const uint QS_SENDMESSAGE = 0x0040;
            public const uint QS_HOTKEY = 0x0080;
            public const uint QS_ALLPOSTMESSAGE = 0x0100;
            public const uint QS_RAWINPUT = 0x0400;

            public const uint QS_MOUSE = (QS_MOUSEMOVE | QS_MOUSEBUTTON);
            public const uint QS_INPUT = (QS_MOUSE | QS_KEY | QS_RAWINPUT);
            public const uint QS_ALLEVENTS = (QS_INPUT | QS_POSTMESSAGE | QS_TIMER | QS_PAINT | QS_HOTKEY);
            public const uint QS_ALLINPUT = (QS_INPUT | QS_POSTMESSAGE | QS_TIMER | QS_PAINT | QS_HOTKEY | QS_SENDMESSAGE);

            public const uint MWMO_INPUTAVAILABLE = 0x0004;

            public const uint WAIT_TIMEOUT = 0x00000102;
            public const uint WAIT_FAILED = 0xFFFFFFFF;
            public const uint INFINITE = 0xFFFFFFFF;
            public const uint WAIT_OBJECT_0 = 0;
            public const uint WAIT_ABANDONED_0 = 0x00000080;
        }
    }
}

I believe the described scenario ought to be pretty common for the UI apps, yet I have found very little material on this subject. Ideally, the background task process should be designed in the way it doesn't require a message pump to support synchronous cancellation, but I don't think this is always possible.

Am I missing something? Are there other, perhaps more portable ways/patterns to deal with it?

Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • Just as a quick note shouldn't you be passing the cts into your wait call? Ie use await Task.Delay(_cts) or something such as that rather than Thread.Sleep – undefined Jan 02 '14 at 05:08
  • 1
    @LukeMcGregor, I've put `Sleep` there on purpose, just as a mock for some atomic CPU-bound operation started by `Task.Run`, which doesn't support instant cancellation. Indeed, if I used `await Task.Delay(1000, ct)` instead of `await Task.Run(...)` in this sample code, it would not cause the deadlock. However, the following still would do: `await Task.Run(async () => { /* do some pool thread work first, then delay */ await Task.Delay(1000, ct); });` – noseratio Jan 02 '14 at 05:30
  • Blocking UI method with Wait is an anti-pattern. Why don't you register callback on the token and let it come back to you when async task is done and process your result. http://msdn.microsoft.com/en-us/library/dd321663%28v=vs.110%29.aspx – Random Jan 02 '14 at 09:39
  • @Random, `ct.Register(callback, useSynchronizationContext: false)` is indeed helpful when cancelling a background IO task. I do use it, e.g. to cancel a pending HTTP request by calling `HttpClient.CancelPendingRequests` from `callback`. However, I don't see how it can be applied to a case like `DoWorkAsync` above, where the chunks of CPU-bound background work are started from the UI thread. Feel free to post an answer if you can elaborate on this. – noseratio Jan 02 '14 at 09:57
  • Ok, I increased sleep to much higher number and I see your point... – Random Jan 02 '14 at 10:18

4 Answers4

10

I disagree that it's a code smell to issue a cancellation request without waiting for the cancellation to take effect. Most of the time, waiting isn't necessary.

In fact, in UI scenarios, I'd say that's the common approach. If you need to avoid side effects (e.g., debug prints, or more realistically, IProgress<T>.Report or a return statement), then simply insert an explicit check for cancellation before performing them:

Debug.Print("Starting work item " + item);
// use Sleep as a mock for some atomic operation which cannot be cancelled
Thread.Sleep(10000);
ct.ThrowIfCancellationRequested();
Debug.Print("Finished work item " + item);

This is particularly useful in a UI context because there's no race conditions around cancellation.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    Stephen, that's a very interesting view... I acknowledge it, yet it's a bit hard for me to accept it, with all respect. I'd be very reluctant to let allow my app to exit in the middle of the `Sleep` above, especially if there are some opaque resources waiting to be properly shut down after that `Sleep` (e.g. legacy COM objects). Anyway, would you extend this idea of unobserved cancellation to an asynchronous wait too (something we discussed [here](http://stackoverflow.com/q/18999827))? – noseratio Jan 02 '14 at 11:55
  • 1
    Yes, if I'm understanding you right. There are some async APIs that aren't cancelable - annoying, but that's life. So I have used the explicit check after-the-fact to either cancel (throwing the exception) or ignore the results (just returning). I don't have much COM interop experience but .NET is quite good at cleaning up pretty much everything; if your app is still running, there's still an STA thread pumping, and if not, then the OS will clean everything up anyway. – Stephen Cleary Jan 02 '14 at 12:03
  • Hmm, I mean if I have a recurring task which I restart without observing the cancellation of its previous instance (i.e., `cts.Cancel()` without following `await pendingTask`), I may end up with an ever growing list of pending tasks. – noseratio Jan 02 '14 at 12:41
  • Also, beyond the OS resources, if I have a pending task with an open networking session, I'd prefer to terminate the session gracefully by issuing the relevant protocol command, before I'd start a new task and establish a new session. – noseratio Jan 02 '14 at 12:41
  • 1
    I assume that, once canceled, the tasks will complete eventually, so the number of tasks won't grow indefinitely. Also, almost any kind of server can accept multiple connections, so I don't see anything wrong with a bit of overlap. To me, this kind of "wait until the resource is cleaned up" logic is only necessary when the resource cannot be shared and it's likely that the newer task is going to need the same resource. – Stephen Cleary Jan 02 '14 at 12:50
  • 1
    I guess I could agree with this point: if there are no shared or costly resources involved, it's OK to request the cancellation and let the task reach the end (or the closest `ct.ThrowIfCancellationRequested()`) without observing this. – noseratio Jan 02 '14 at 12:59
  • @Noseratio, doesn't this approach have a race condition that you are trying to avoid? Consider this sequence: (1) background thread gets past ct.ThrowIfCancellationRequested, (2) Main thread issues cancellation and form closes, (3) background thread runs Debug.Print, or IProgres.Report, or return; resulting in the undesired behavior. – Matt Smith Jan 06 '14 at 14:06
  • 1
    @MattSmith: If you're talking about an actual background thread, then yes, there will always be a race condition (usually benign) between the cancellation request and the task completion. OTOH, if it's an `async` method in a UI/ASP.NET context, then there's no race condition. I have a couple of relevant blog posts: [`async` disposal](http://blog.stephencleary.com/2013/03/async-oop-6-disposal.html) (the section "Option 1: Dispose Means Cancel") and [callback contexts](http://blog.stephencleary.com/2009/04/asynchronous-callback-contexts.html) (we're using the CT as "callback context", sort of). – Stephen Cleary Jan 06 '14 at 15:10
10

So we don't want to be doing a synchronous wait as that would be blocking the UI thread, and also possibly deadlocking.

The problem with handling it asynchronously is simply that the form will be closed before you're "ready". That can be fixed; simply cancel the form closing if the asynchronous task isn't done yet, and then close it again "for real" when the task does finish.

The method can look something like this (error handling omitted):

void MainForm_FormClosing(object sender, FormClosingEventArgs e)
{
    if (!_task.IsCompleted)
    {
        e.Cancel = true;
        _cts.Cancel();
        _task.ContinueWith(t => Close(), 
            TaskScheduler.FromCurrentSynchronizationContext());
    }
}

Note that, to make the error handling easier, you could at this point make the method async as well, instead of using explicit continuations.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • @Servy, how would you hande timed wait? Say we don't want to wait forever, if the task misbehaves. I don't think you can do it with this approach; you need another task/thread to take care of that. – gwiazdorrr Jan 03 '14 at 09:09
  • 1
    @gwiazdorrr That's a trivial case to add. Just add a second cancellation token to the continuation in my code that's set to be cancelled after whatever timeout time you want. – Servy Jan 03 '14 at 15:05
  • 1
    That whole condition could be simplified to just `if (!_task.IsCompleted)`. That's because `IsCompleted` means that the `Task` completed in any way, not just successfully; it's *not* the same as checking whether the `Status` is `RanToCompletion`, which is quite confusing. – svick Jan 03 '14 at 21:12
  • @svick I couldn't remember offhand, so I was being conservative. Thanks for pointing it out. – Servy Jan 03 '14 at 21:14
  • @Servy, what's the proper way of doing it with exception handling? – Gabrielius Nov 15 '14 at 11:22
7

Inspired by @Servy's answer, here is another idea: show a temporary modal dialog with a "Please wait..." message, and utilize its modal message loop to wait asynchronously for the pending task. The dialog automatically disappears when the task has been fully cancelled.

That's what ShowModalWaitMessage does below, called from MainForm_FormClosing. I think this approach is a bit more user-friendly.

Wait Dialog

using System;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WinformsApp
{
    public partial class MainForm : Form
    {
        CancellationTokenSource _cts;
        Task _task;

        // Form Load event
        void MainForm_Load(object sender, EventArgs e)
        {
            _cts = new CancellationTokenSource();
            _task = DoWorkAsync(_cts.Token);
        }

        // Form Closing event
        void MainForm_FormClosing(object sender, FormClosingEventArgs e)
        {
            ShowModalWaitMessage();
        }

        // Show a message and wait
        void ShowModalWaitMessage()
        {
            var dialog = new Form();

            dialog.Load += async (s, e) =>
            {
                _cts.Cancel();

                try
                {
                    // show the dialog for at least 2 secs
                    await Task.WhenAll(_task, Task.Delay(2000));
                }
                catch (Exception ex)
                {
                    while (ex is AggregateException)
                        ex = ex.InnerException;
                    if (!(ex is OperationCanceledException))
                        throw;
                }

                dialog.Close();
            };

            dialog.ShowIcon = false; dialog.ShowInTaskbar = false;
            dialog.FormBorderStyle = FormBorderStyle.FixedToolWindow;
            dialog.StartPosition = FormStartPosition.CenterParent;
            dialog.Width = 160; dialog.Height = 100;

            var label = new Label();
            label.Text = "Closing, please wait...";
            label.AutoSize = true;
            dialog.Controls.Add(label);

            dialog.ShowDialog();
        }

        // async work
        async Task DoWorkAsync(CancellationToken ct)
        {
            var i = 0;
            while (true)
            {
                ct.ThrowIfCancellationRequested();

                var item = i++;
                await Task.Run(() =>
                {
                    Debug.Print("Starting work item " + item);
                    // use Sleep as a mock for some atomic operation which cannot be cancelled
                    Thread.Sleep(1000);
                    Debug.Print("Finished work item " + item);
                }, ct);
            }
        }

        public MainForm()
        {
            InitializeComponent();
            this.FormClosing += MainForm_FormClosing;
            this.Load += MainForm_Load;
        }
    }
}
Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • 1
    1) Are you prepared for the case where the user closes the dialog? You'd either want to disable that option, or ensure it works properly if they do. 2) Rather than calculating a tick count of how long to wait just `await Task.WhenAny(Task.Delay(2000), task)` to essentially add a "timeout" to the task. The other option is to tack a `ContinueWith(t => {}, new TaskCompletionSource(2000).Token);` onto whatever task *before assigning it to `_task` instead of adding the timout functionality here. – Servy Jan 02 '14 at 21:03
  • 1
    @Servy, right, these are good points. That's just a concept code which can be improved in many way. Also, if user prematurely closes the "Wait..." dialog, I may want to take on [Stephen's answer](http://stackoverflow.com/a/20881747/1768303) and just let the app close. In that case, at least, an attempt would have been made to shut down gracefully. – noseratio Jan 02 '14 at 21:11
  • @Servy, *2) Rather than calculating a tick count of how long to wait just await Task.WhenAny(Task.Delay(2000), task)* - I'd rather do `await Task.WhenAll(Task.Delay(2000), task)`, the goal is to make the dialog stay for at least 2 sec so it doesn't flash and go away instantly. – noseratio Jan 02 '14 at 21:15
  • 1
    Then you'd use `WhenAll` for that rather than `WhenAny`. – Servy Jan 02 '14 at 21:23
  • 2
    This is the approach I prefer to use when dealing with 'shutdown' of critical resources (like waiting for a piece of physical machinery to come to a controlled stop before allowing its control application to terminate.) – Dan Bryant Jan 03 '14 at 00:40
  • Related: [Call async method on UI thread](https://stackoverflow.com/q/53508160/1768303). – noseratio Mar 05 '19 at 08:16
0

How about using the older way:

    public delegate void AsyncMethodCaller(CancellationToken ct);

    private CancellationTokenSource _cts;
    private AsyncMethodCaller caller;
    private IAsyncResult methodResult;

    // Form Load event
    private void MainForm_Load(object sender, EventArgs e)
    {
        _cts = new CancellationTokenSource();

        caller = new AsyncMethodCaller(DoWorkAsync);
        methodResult = caller.BeginInvoke(_cts.Token,
            ar =>
            {

            },
            null);

    }

    // Form Closing event
    private void MainForm_FormClosing(object sender, FormClosingEventArgs e)
    {
        _cts.Cancel();          
        MessageBox.Show("Task cancellation requested");    
    }

    // async work
    private void DoWorkAsync(CancellationToken ct)
    {
        var i = 0;
        while (true)
        {
            var item = i++;

            Debug.Print("Starting work item " + item);
            // use Sleep as a mock for some atomic operation which cannot be cancelled
            Thread.Sleep(10000);
            Debug.Print("Finished work item " + item);

            if (ct.IsCancellationRequested)
            {
                return;
            }
        }
    }


    private void MainForm_FormClosed(object sender, FormClosedEventArgs e)
    {
        methodResult.AsyncWaitHandle.WaitOne();
        MessageBox.Show("Task cancelled");
    }

You can do some further modification to keep user busy with a nice animation

Random
  • 4,519
  • 2
  • 38
  • 46
  • 1
    We don't need `BeginInvoke` for this, `Task.Run` would work too, but see my thoughts on this in the comments [here](http://stackoverflow.com/a/20879791/1768303). – noseratio Jan 02 '14 at 11:58