2

I'm trying to make use of cancellation tokens as described in this FAQ. This was my initial thought:

private async void OnLoginButtonClicked(object sender, EventArgs e)
{
    if (this.cancelToken == null)
    {
        this.cancelToken = new CancellationTokenSource();
    }

    try
    {
        bool loginSuccess = await AsyncLoginTask(this.cancelToken.Token);

        if (loginSuccess)
        {
            // Show main page
        }
    }
    catch (OperationCanceledException ex)
    {
        System.Diagnostics.Debug.WriteLine(ex.Message);
    }
    catch (Exception ex)
    {
        System.Diagnostics.Debug.WriteLine(ex.Message);
    }
    finally
    {
        this.cancelToken = null;
    }
}

private async Task<bool> AsyncLoginTask(CancellationToken cancellationToken = default(CancellationToken))
{
    // Pass the token to HttpClient()
}

Now I adapted it and this is the result:

private async void OnLoginButtonClicked(object sender, EventArgs e)
{
    this.cancelToken?.Dispose();
    this.cancelToken = new CancellationTokenSource();

    try
    {
        var ui = TaskScheduler.FromCurrentSynchronizationContext();
        var loginTask = Task.Factory.StartNew(async () =>
        {
            bool loginSuccess = await AsyncLoginTask(this.cancelToken.Token);
        }, this.cancelToken.Token);

        var displayResults = loginTask.ContinueWith(resultTask =>
                            {
                                // How do I know if the login was successful?
                                // Because AsyncLoginTask() returns bool.
                                System.Diagnostics.Debug.WriteLine("done");
                            },
                             CancellationToken.None,
                             TaskContinuationOptions.OnlyOnRanToCompletion,
                             ui);

        var displayCancelledTasks = loginTask.ContinueWith(resultTask =>
                                    {
                                        System.Diagnostics.Debug.WriteLine("canceled");
                                    },
                                   CancellationToken.None,
                                   TaskContinuationOptions.OnlyOnCanceled, ui);
    }
    catch (Exception ex)
    {
        System.Diagnostics.Debug.WriteLine(ex.Message);
    }
}

Questions:

  • How do I know if the login was successful? Because AsyncLoginTask() returns bool.
  • How to create and destroy the token correctly to allow a operation to be started and cancelled multiple times?
  • How to handle the task in the task? "done" is shown in the console, while the task (AsyncLoginTask) hasn't finished yet.
testing
  • 19,681
  • 50
  • 236
  • 417
  • `How do I know if the login was successful? Because AsyncLoginTask() returns bool` well, you can return `loginSuccess` in that task, that would give `loginTask` a result. But... my question here is why you want to wrap your login task in a task? – Stefan May 30 '17 at 10:49
  • My old code (`AsyncLoginTask`) was built this way (async/await with returning true/false). Now I want to be able that the user can cancel the operation. In the linked article it is the recommended way to work with the task and `ContinueWith`. So I wrapped my task. Should I stay with catching `OperationCanceledException`? – testing May 30 '17 at 10:53
  • Well, I haven't gone into depth with the linked article, and I must say I am not an expert in this area... But, since the login already has an option to provide a cancellation token, and your logic later on requires the login result, a simple `await` seems more intuitive to me.You can handle that cancellation in the appropriate way. Can you explain your actual goal and why you think this refactoring will help to achieve it? – Stefan May 30 '17 at 11:00
  • You might be better of looking at [Microsoft Docs](https://learn.microsoft.com/en-us/dotnet/csharp/async) for asynchronous programming. `Task.Factory.StartNew` and `.ContinueWith` is kind of obsolete and replaced by the `async await` pattern – default May 30 '17 at 11:12
  • 2
    @testing, check this blog post: [Async re-entrancy, and the patterns to deal with it](https://blogs.msdn.microsoft.com/lucian/2014/03/03/async-re-entrancy-and-the-patterns-to-deal-with-it/). – noseratio May 30 '17 at 11:15
  • @Stefan: The actual goal is to enable cancellation (e.g. the user can cancel the operation, or if the user navigates away to minimize weird behavior, no duplicated calls, less null reference exceptions and so on). I wanted to use this method because of the statement *"Now I get the same results as I did with the try-catch block, and I don’t have to deal with exceptions at all. Let me emphasize that this is a more natural way of working with TPL and tasks"*. But I think I'll stay with my old approach, except one has a better best practice method. – testing May 30 '17 at 12:24
  • What's wrong with your first version? `StartNew` and `ContinueWith` may lead to unexpected results in some cases. And `done` is written in console because `await` in inner task exits until you got the result from http request. – VMAtm May 30 '17 at 12:28
  • @VMAtm: I haven't worked with `CancellationTokenSource` before and I tried something without knowing if this is the correct way. Then I found the article, which led to the second version. Furthermore, when do I have to catch `TaskCanceledException`, when `OperationCanceledException`? The exception I got was a kind of restricted one (I was not really able to debug), but it seems it was a `OperationCanceledException`. I know that the inner task exits until I get the result, but I don't understand here why. Since I'll abandon the second approach, that question is now less relevant. – testing May 30 '17 at 12:39
  • 1
    Inner task exits because this is how `await` works - it returns from the first `await`, saves the context and frees the thread. You should catch the `OperationCanceledException`, it's the base class, https://stackoverflow.com/a/13040503/213550 – VMAtm May 30 '17 at 12:45

1 Answers1

2

I'm trying to make use of cancellation tokens as described in this FAQ.

That blog post is using Dynamic Task Parallelism (StartNew and ContinueWith). Dynamic Task Parallelism is when you have a lot of CPU-bound operations to do and you don't know how many you have until you're already processing them (i.e., each one you process can add zero or more additional tasks to the same process).

In your case, you have a single asynchronous operation. As such, the approach in that article is completely wrong for your use case. Your original thought was much more correct.

You'd want to do it more like this:

private async void OnLoginButtonClicked(object sender, EventArgs e)
{
  // Cancel the previous attempt (if any) and start a new one.
  this.cts?.Cancel();
  this.cts = new CancellationTokenSource();

  try
  {
    bool loginSuccess = await AsyncLoginTask(this.cts.Token);
    // Resolve race condition where user cancels just as it completed.
    this.cts.Token.ThrowIfCancellationRequested();
    if (loginSuccess)
    {
      // Show main page
    }
  }
  catch (OperationCanceledException ex)
  {
    System.Diagnostics.Debug.WriteLine(ex.Message);
  }
  catch (Exception ex)
  {
    System.Diagnostics.Debug.WriteLine(ex.Message);
  }
}

private async Task<bool> AsyncLoginTask(CancellationToken cancellationToken = default(CancellationToken))
{
  // Pass the token to HttpClient()
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810