C# 5 Async Await .Task.Factory.StartNew cancellation

Question

I have the async code that implements cancellation token. It's working but Im not pretty sure if this is the right way to do it so I just want feedback about it.

Here is the actual code:

/// <summary>
/// 
/// </summary>
private async void SaveData() {
 if (GetActiveServiceRequest() != null)
 {
 var tokenSource = new System.Threading.CancellationTokenSource();
 this.ShowWizardPleaseWait("Saving data...");
 var someTask = System.Threading.Tasks.Task<bool>.Factory.StartNew(() =>
 {
 bool returnVal = false;
 // Set sleep of 7 seconds to test the 5 seconds timeout.
 System.Threading.Thread.Sleep(7000);
 if (!tokenSource.IsCancellationRequested)
 {
 // if not cancelled then save data
 App.Data.EmployeeWCF ws = new App.Data.EmployeeWCF ();
 returnVal = ws.UpdateData(_employee.Data);
 ws.Dispose();
 }
 return returnVal;
 }, tokenSource.Token);
 if (await System.Threading.Tasks.Task.WhenAny(someTask, System.Threading.Tasks.Task.Delay(5000)) == someTask)
 {
 // Completed
 this.HideWizardPleaseWait();
 if (someTask.Result)
 {
 this.DialogResult = System.Windows.Forms.DialogResult.OK;
 }
 else
 {
 this.DialogResult = System.Windows.Forms.DialogResult.Abort;
 }
 btnOK.Enabled = true;
 this.Close();
 }
 else
 {
 tokenSource.Cancel();
 // Timeout logic
 this.HideWizardPleaseWait();
 MessageBox.Show("Timeout. Please try again.")
 }
 }
}

Does async / await / cancellation code is well implemented?

Answer 1

It's better to use CancellationTokenSource(TimeSpan) constructor to set the cancellation after 5 seconds.

Also, Task.Run method is a recommended way to run compute-bound tasks (see remark here).

Other issues worth noting:

• by conventions asynchronous methods should have a suffix Async
• using keyword is a recommended way to correctly dispose IDisposable objects

• if a task to be cancelled cannot return correct results (according to business requirements) it's recommended to throw OperationCanceledException by calling token.ThrowIfCancellationRequested() method.

  private async void SaveDataAsync() 
  {
   if (GetActiveServiceRequest() == null)
   return;
   var cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(5)).Token;
   this.ShowWizardPleaseWait("Saving data...");
   var someTask = Task.Run<bool>(() =>
   {
   // Set sleep of 7 seconds to test the 5 seconds timeout.
   System.Threading.Thread.Sleep(7000);
   cancellationToken.ThrowIfCancellationRequested();
   // if not cancelled then save data
   using (App.Data.EmployeeWCF ws = new App.Data.EmployeeWCF())
   {
   return ws.UpdateData(_employee.Data);
   }
   }, cancellationToken);
   try
   {
   this.DialogResult = await someTask
   ? DialogResult.OK
   : DialogResult.Abort;
   btnOK.Enabled = true;
   this.Close();
   }
   catch(OperationCanceledException)
   {
   MessageBox.Show("Timeout. Please try again.")
   }
   finally
   {
   this.HideWizardPleaseWait();
   }
  }
  

• Chris Pickford: I had been pulling my hair out using the Task.Delay() method in the question where tasks would RunToCompletion regardless of a call to CancellationTokenSource.Cancel(). Refactored my Task manager to use this method and it all just works. Thanks.