Wrap a callback into async Task

Question

I have the following method from an API which I have no control over.

public void Start(Action OnReady);

In general I am fine with callbacks but sometimes you have callback that triggers a callback and so on. Therefore I would like to wrap it into a async method and maybe also include the possibility to cancel the action. Something like this:

await Start(cancellationToken);

This is what I came up with:

public Task Start(CancellationToken cancellationToken)
{
     return Task.Run(() =>
     {
           cancellationToken.ThrowIfCancellationRequested();

           var readyWait = new AutoResetEvent(false);

           cancellationToken.Register(() => readyWait?.Set());

           Start(() => { readyWait.Set(); }); //this is the API method

           readyWait.WaitOne();
           readyWait.Dispose();
           readyWait = null;

           if(cancellationToken.IsCancellationRequested)
           {
                APIAbort(); //stop the API Start method from continuing
                cancellationToken.ThrowIfCancellationRequested();
            }

      }, cancellationToken);
}

I think there is room for improvement but one thing that comes to my mind is what does this method in this context?

readyWait.WaitOne(); 

I wanted to write an async method to not block any thread, but that's exactly what WaitOne does. Of course it does not block the calling thread because of the task, but does the task gets its own thread? I would be fine if only the task would be blocked, but I don't want to block a thread that might be in use somewhere else.

Answer 1

I would like to wrap it into a async method and maybe also include the possibility to cancel the action.

I recommend keeping those separate. The reason is that your "cancel" doesn't actually cancel the Start. It only cancels the wait for the Start to complete. So having a cancel at this level would be misleading.

You can wrap a delegate callback into a Task using a similar approach to the pattern for wrapping an event into a Task:

public static Task StartAsync(this ApiObject self)
{
  var tcs = new TaskCompletionSource<object>();
  self.Start(() => tcs.SetResult(null));
  return tcs.Task;
}

Now that you have a way to call StartAsync and get back a Task, you can choose not to continue waiting if you want:

var startTask = apiObject.StartAsync();
var timeoutTask = Task.Delay(TimeSpan.FromSeconds(10));
var completedTask = await Task.WhenAny(startTask, timeoutTask);
if (completedTask == timeoutTask)
  return;

Answer 2

This problem is surprisingly tricky. The cases are many, and the handling of each case is not always obvious.

Obvious:

1. The Start method may take a long time to complete. The same is true for the APIAbort method.
2. The CancellationToken could be cancelled before, during, or after the Start method, or even after the OnReady callback invocation.
3. The APIAbort should not be called before or during Start, or after the OnReady callback invocation.
4. The APIAbort should be called even if the cancellation occured before completion of the Start method.

Not obvious:

1. Should the returned Task complete as cancelled before or after the APIAbort invocation?
2. What if APIAbort throws an exception? How to propagate this exception if the returned Task is already cancelled?
3. What if Start throws an OperationCanceledException exception? Is this a fault or a cancellation?

The implementation below cancels the Task before invoking the APIAbort method, suppresses exceptions that may occur during APIAbort, and treats an OperationCanceledException during Start as cancellation.

public Task StartAsync(CancellationToken cancellationToken)
{
    if (cancellationToken.IsCancellationRequested)
        return Task.FromCanceled(cancellationToken);

    var tcs = new TaskCompletionSource<bool>();

    var cancellationRegistration = cancellationToken.Register(() =>
        tcs.TrySetCanceled(cancellationToken));

    var fireAndForget = Task.Run(() =>
    {
        if (cancellationToken.IsCancellationRequested) return;
        try
        {
            Start(() =>
            {
                cancellationRegistration.Dispose(); // Unregister
                tcs.TrySetResult(true);
            });
        }
        catch (OperationCanceledException)
        {
            tcs.TrySetCanceled();
            return;
        }
        catch (Exception ex)
        {
            tcs.TrySetException(ex);
            return;
        }

        // At this point Start is completed succesfully. Calling APIAbort is allowed.
        var continuation = tcs.Task.ContinueWith(_ =>
        {
            try
            {
                APIAbort();
            }
            catch { } // Suppressed
        },  default, TaskContinuationOptions.OnlyOnCanceled
        | TaskContinuationOptions.RunContinuationsAsynchronously,
        TaskScheduler.Default);
    }, cancellationToken);

    return tcs.Task;
}

The reason for setting the option TaskContinuationOptions.RunContinuationsAsynchronously is for avoiding the possibility of running the APIAbort synchronously (in the same thread) with the code after await StartAsync(). I run to this problem initially when I used async-await as a continuation mechanism.

Answer 3

public Task StartAsync(CancellationToken c)
{
    var cs = new TaskCompletionSource<bool>();

    c.Register(() => { Abort(); cs.SetCanceled(); } );
    Start(() => { cs.SetResult(true); });

    return cs.Task;
}

using (var ct = new CancellationTokenSource(1000))
{
    try
    {
        await StartAsync(ct.Token);

        MessageBox.Show("Completed");
    }
    catch (TaskCanceledException)
    {
        MessageBox.Show("Cancelled");
    }
}

There is no point to use the cancellation token because the only points where it can fire is immediately after the method is called and immediately before it's finished, at which point it's a race condition.