1

I am new to Tasks in C# and I have come across an issue I don't under stand. In the following code can someone please explain why the WriteLine at the end of the Unsubscribe Method never gets called. It seems to be related to ContinueWith in the preceding foreach loop as if I comment that out it works fine. Thanks

using System;
using System.Threading;
using System.Linq;
using System.Threading.Tasks;
using System.Collections.Generic;

public class Example
{
    public static void Main()
    {

        Task.Run(() => Unsubscribe());

        Console.WriteLine("Method: Main. End");

        Console.ReadKey();

    }

    private static void Unsubscribe()
    {
        Dictionary<int, string> dicPairsBySubID = new Dictionary<int, string>();

        dicPairsBySubID.Add(1, "A");
        dicPairsBySubID.Add(2, "B");
        dicPairsBySubID.Add(3, "C");
        dicPairsBySubID.Add(4, "D");
        dicPairsBySubID.Add(5, "E");

        List<Task> taskList = new List<Task>();

        foreach (KeyValuePair<int, string> sub in dicPairsBySubID)
        {
            int chanID = sub.Key;

            Task task = Task.Run(() => SendUnsubscribe(chanID))
            .ContinueWith(tsk =>
            {
                var flattened = tsk.Exception.Flatten();

                flattened.Handle(ex =>
                {
                    Console.WriteLine(string.Format("Error unsubscribing pair {0} (chanID = {2})", sub.Value, chanID), ex);
                    return true;
                });

            }, TaskContinuationOptions.OnlyOnFaulted);

            taskList.Add(task);
        }

        Task.WhenAll(taskList).Wait();

        // WHY DOES THIS NEVER GET RUN?
        Console.WriteLine("Method: Unsubscribe. End");


    }

    private static async Task SendUnsubscribe(int chanID)
    {


        await SendAsync(chanID);


        Console.WriteLine("Method: SendUnsubscribe. Chan ID: " + chanID);


    }

    private static async Task SendAsync(int chanID)
    {

        await Task.Run(() => Thread.Sleep(1000));
        Console.WriteLine("Method: SendAsync. Chan ID: " + chanID);

    } 


}
altwood
  • 101
  • 1
  • 8
  • 1
    If you're going to use async, do it properly. Use C# 7.1, with an `async main`. Don't use `.ContinueWith()` or `.Wait()`. Those are code smells. You need to `await` the async calls. Read over [Async/Await - Best Practices in Asynchronous Programming](https://msdn.microsoft.com/en-us/magazine/jj991977.aspx). – mason Jan 18 '18 at 16:53
  • You don't need all those `Task.Run`s. Especially, what's the point in wrapping SendUnsubscribe in one when it's already a Task and not even awaiting it's result or exception? `Task.WhenAll(taskList).Wait();` can also be replaced by `await Task.WhenAll(taskList);` with declaring `private async Task Unsubscribe()`. – ckuri Jan 18 '18 at 16:55

2 Answers2

2

Your task throws an exception which you are not catching, because:

  • ContinueWith also returns a task (which is then placed in taskList)
  • You have stated that that task should only be run when the previous one is faulted (due to TaskContinuationOptions.OnlyOnFaulted)
  • The example you have given does not enter the faulted state.
  • When the ContinueWith task is not run due to these options it enters the state Cancelled
  • Waiting for a cancelled task throws an exception
  • In your Main method the return value of the Task.Run(() => Unsubscribe()) call is not awaited, thus it is never noticed.

Further on:

  • You should not use Wait() on Tasks. You have already started using async and await, use that through your program. The only reason really to use Wait() is when you certainly cannot use async. Do try to avoid it.

  • Always name your methods with the suffix Async when they are async. This shows a clear intent for the user using that method.

default
  • 11,485
  • 9
  • 66
  • 102
  • This has nothing to do with the problem. Marking a continuation as `OnlyOnFaulted` doesn't make it never complete if the antecedent doesn't fault, it just means that the delegate isn't run *and the task is marked as complete anyway*. The question doesn't ask why the code in that continuation doesn't run, it asks why `WhenAll` never finishes, and that has nothing to do with whether the continuation only runs on faulted tasks or whether those tasks fault. – Servy Jan 18 '18 at 17:08
  • @Servy thanks for the input! I edited the question after I actually tried the code. It should be more correct now (and with a given example) – default Jan 18 '18 at 17:22
  • @Servy Is there still something wrong with the answer? – default Jan 19 '18 at 10:37
  • Thank you Default, and thanks for the 'further on', I appreciate the advice. I think I am slowly starting to get the hang of Tasks and async/await. – altwood Jan 22 '18 at 10:01
1

I tested this out. I'm getting an exception at WhenAll saying a task is cancelled. That makes all of this makes sense:

  1. As Default's answer pointed out, you're adding the ContinueWith task to taskList.
  2. The ContinueWith task is getting cancelled, because it is supposed to run OnlyOnFaulted, and no exception happened.
  3. Since the exception is thrown (and not handled) the rest of your Unsubscribe is not run.

Here are some things you can do to improve this:

  1. Don't use Task.Run just to run an async method. You can do this instead:

    taskList.Add(SendUnsubscribe(chanID));

  2. Use try/catch inside your SendUnsubscribe method, instead of using ContinueWith on the task.

  3. Await WhenAll instead of using Wait():

    await Task.WhenAll(taskList);

You can always wrap await Task.WhenAll(taskList); in a try/catch block if you think any exceptions could happen that SendUnsubscribe won't handle.

Gabriel Luci
  • 38,328
  • 4
  • 55
  • 84