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();
      }
  }