8
\$\begingroup\$

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?

\$\endgroup\$
4
\$\begingroup\$

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();
        }
    }
    
\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – Chris Pickford Aug 8 '14 at 9:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.