Should I await a synchronous Task<T> that uses TaskCompletionSource?

Question

I'm trying to write some library code that wraps a non-Task-based library (MSDeploy API). Essentially, I'm getting stuff from a remote server and analyzing it. I'm using TaskCompletionSource to wrap the synchronous MSDeploy API b/c although it's not Task-based, I can get the CancellationToken support to work with that library's CancelCallback delegate. The 'getting stuff' code is exposed as a synchronous API, but it is I/O bound. The analysis code is I/O & CPU intensive and is already Task-based. It doesn't spawn any tasks/threads (I will do that in the calling code).

I've been reading a lot of blogs about this stuff lately, but I'm still trying to get my head around async/await/ConfigureAwait() and to figure out the best way to write and call the composite method (GetAndAnalyzeStuff). A few questions:

1. Whether to await both the TaskCompletionSource-based call and the TPL async call in the GetAndAnalyzeStuff method or just the async call (should I use .Wait() on the call GetStuff).
2. Can you tell whether I need ConfigureAwait(false) somewhere? If so, how can you tell?
3. How do I deal with the Task.Run() call in the calling code (Main() below)? I prefer this to run in the background and I gather it's bad practice to fire up Tasks in the library/API code. I assume Task.Run is more appropriate than Task.Factory.StartNew b/c I'm doing async stuff.

Code below (using .NET 4.5.1):

public Task<Stuff> GetStuff(CancellationToken token) 
{
    var tcs = new TaskCompletionSource<Stuff>(tcs);
    try 
    {
        var stuff = new Stuff();
        using (var stuffGetter = new StuffGetter()) 
        {
            stuffGetter.CancelCallback = () => cancellationToken.IsCancellationRequested;

            for (var x = 0; x < 10; x++) 
            {
                if (token.IsCancellationRequested) 
                {
                    tcs.SetCancelled();
                    return tcs.Task;
                }

                // StuffGetter's doing IO & would ideally have an API w/Task<Stuff>, but
                // it doesn't and it's not my code.
                var thing = stuffGetter.GetSomething();
                stuff.AddThing(thing);
            }
            tcs.SetResult(stuff);
        }
    }
    catch (SomeNonTplFriendlyExceptionThatMeansSomethingWasCancelled sntfetmswc)
    {
        tcs.SetCancelled();        
    }
    catch (Exception exc) 
    {
        tcs.SetException(exc);
    }
    return tcs.Task;
}

public async Task<StuffAnalysis> AnalyzeStuff(Stuff stuff, CancellationToken token) 
{
    var allTasks = new List<Task>();

    using (var stuffAnalyzer = new StuffAnalyzer())
    {
        foreach (var thing in stuff) 
        {
            allTasks.Add(stuffAnalyzer.AnalyzeThingAsync(thing));
        }

        await Task.WhenAll(allTasks);
    }

    return stuffAnalyzer.Result;
}

public async Task<StuffAnalysis> GetAndAnalyzeStuff(CancellationToken token)
{
    var stuff = await GetStuff(token); // or should this be GetStuff.Wait() or maybe GetStuff.Result?
    var analysis = await AnalyzeStuff(stuff, token);
    return analysis;
}

public static void Main() 
{
    var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
    try 
    {
        // Since GetStuff is synchronous and I don't want to block, use Task.Run() -- right?

        var task = Task.Run(() => GetAndAnalyzeStuff(cts.Token), cts.Token);
        // or var task = Task.Run(async () => await GetAndAnalyzeStuff(cts.Token), cts.Token); ?

        Console.WriteLine("Getting and anlyzing stuff in the background.");

        await task;
    } 
    catch (OperationCancelledException)
    {
        Console.WriteLine("There was a problem.");
    }
    catch (Exception)
    {
        Console.WriteLine("There was a problem.");
    }
}

Answer 1

Since you don't have a naturally-asynchronous API, I do not recommend using TaskCompletionSource<T>. You can use the full CancellationToken support with synchronous APIs, as such:

public Stuff GetStuff(CancellationToken token) 
{
    var stuff = new Stuff();
    using (var stuffGetter = new StuffGetter()) 
    {
        stuffGetter.CancelCallback = () => token.IsCancellationRequested;

        for (var x = 0; x < 10; x++) 
        {
            token.ThrowIfCancellationRequested();
            var thing = stuffGetter.GetSomething();
            stuff.AddThing(thing);
        }
        return stuff;
    }
}

When you write task-returning methods, it's important to follow the TAP guidelines. In this case, the naming convention means your AnalyzeStuff should be called AnalyzeStuffAsync.

Can you tell whether I need ConfigureAwait(false) somewhere? If so, how can you tell?

You should use ConfigureAwait(false) unless you need the context in your method (the "context" is usually the UI context for client apps, or the request context for ASP.NET apps). You can find more information in my MSDN article on async best practices.

So, assuming that StuffAnalyzer.Result doesn't have any kind of UI thread dependency or anything like that, I'd write AnalyzeStuffAsync as such:

public async Task<StuffAnalysis> AnalyzeStuffAsync(Stuff stuff, CancellationToken token) 
{
  var allTasks = new List<Task>();

  using (var stuffAnalyzer = new StuffAnalyzer())
  {
    foreach (var thing in stuff) 
    {
      allTasks.Add(stuffAnalyzer.AnalyzeThingAsync(thing));
    }

    await Task.WhenAll(allTasks).ConfigureAwait(false);
  }

  return stuffAnalyzer.Result;
}

Your GetAndAnalyzeStuffAsync is a more complex situation, where you have both blocking and asynchronous code in the method. In this case, the best approach is to expose it as an asynchronous API but with a clear comment noting that it is blocking.

// <summary>Note: this method is not fully synchronous; it will block the calling thread.</summary>
public async Task<StuffAnalysis> GetAndAnalyzeStuffAsync(CancellationToken token)
{
  var stuff = GetStuff(token);
  var analysis = await AnalyzeStuff(stuff, token).ConfigureAwait(false);
  return analysis;
}

which can simplify to:

// <summary>Note: this method is not fully synchronous; it will block the calling thread.</summary>
public Task<StuffAnalysis> GetAndAnalyzeStuffAsync(CancellationToken token)
{
  var stuff = GetStuff(token);
  return AnalyzeStuff(stuff, token);
}

How do I deal with the Task.Run() call in the calling code (Main() below)?

You are using it correctly. I describe this situation on my blog. In a Console application it's not common to use Task.Run like this, but there's nothing wrong with doing it this way. Task.Run is usually used to free up the UI thread in a UI application.

I assume Task.Run is more appropriate than Task.Factory.StartNew b/c I'm doing async stuff.

Yes, it is.