Handle completed tasks in ApmAsyncFactory.

[fiseni]

This fixes #281.

I couldn't really find documentation on how to handle sync operations in APM. I tested various options, and it seems the IAsyncResult response should have the CompletedSynchronously set to true to indicate that this is a sync flow. In that case, we're not obliged to call the callback, and that's exactly what we need. But, the Task has a hard-coded false value for this property, so returning Task will never work in this scenario. I just ended up using a new internal type CompletedAsyncResult for this scenario.

This fixes all issues, at least the usages I can think of.

[StephenCleary]

CompletedSynchronously was my first thought as well. However, that cannot be the whole story because the alternative solution also uses a Task implementation of IAsyncResult, which has CompletedSynchronously set to false.

[fiseni]

For ToBegin

• If the task is already completed, we're returning the newly defined IAsyncResult implementation CompletedAsyncResult, which has set the CompletedSynchronously to true.
• If the task is not completed, the flow remains as it is. The ToBegin returns (since we have async void) and the callback will be invoked once the original task is completed.

For ToEnd

• If the IAsyncResult is CompletedAsyncResult then we know it was a sync flow, and we're returning the Result.
• If the IAsyncResult is Task then we unwrap and return the result.

What am I missing here? 🤔

[fiseni]

I might have misunderstood your comment. What do you mean by "alternative solution"? Are you referring to the solution in this article? It's using a continuation, which is dispatched in a separate thread. That's why it always works and we don't need the sync flow (even if the task returns synchronously).

public static IAsyncResult AsApm<T>(this Task<T> task,
                                    AsyncCallback callback,
                                    object state)
{
    if (task == null)
        throw new ArgumentNullException("task");

    var tcs = new TaskCompletionSource<T>(state);
    task.ContinueWith(t =>
                      {
                         if (t.IsFaulted)
                            tcs.TrySetException(t.Exception.InnerExceptions);
                         else if (t.IsCanceled)
                            tcs.TrySetCanceled();
                         else
                            tcs.TrySetResult(t.Result);

                         if (callback != null)
                            callback(tcs.Task);
                      }, TaskScheduler.Default);
    return tcs.Task;
}

[Pretasoc]

I saw you already found a solution. So please do not feel pressured in any way by this comment. It just happened, that i stumbled apon an other solution for this problem earlier this day. You may wanna have a look at this Task to apm wrapper.

The difference is not that large. It also uses an async result wrapper. It might be a little more lightweight, thatn allocation a new task completion source, but i cannot judge if thats a noticable difference (i highly doubt it).

[fiseni]

Thanks. I'm just a contributor and created the PR only as a suggestion.
@StephenCleary will review it at some convenient time. Perhaps he has other ideas for resolving the issue at hand.