Reputation: 1072
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.
Upvotes: 1
Views: 1344
Reputation: 457472
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;
Upvotes: 1
Reputation: 44026
This problem is surprisingly tricky. The cases are many, and the handling of each case is not always obvious.
Obvious:
Start
method may take a long time to complete. The same is true for the APIAbort
method.CancellationToken
could be cancelled before, during, or after the Start
method, or even after the OnReady
callback invocation.APIAbort
should not be called before or during Start
, or after the OnReady
callback invocation.APIAbort
should be called even if the cancellation occured before completion of the Start
method.Not obvious:
Task
complete as cancelled before or after the APIAbort
invocation?APIAbort
throws an exception? How to propagate this exception if the returned Task
is already cancelled?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.
Upvotes: 0
Reputation: 78209
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.
Upvotes: 3