Async Operation Cancelled but Still Taking Time to Update Grid

Question

I'm having a bit of trouble getting an async operation working (new to async). My goal is to have a button "Load Data" go out and retrieve some data from the database and populate a grid. The database can be somewhat far away for some users and this operation may take some time. Considering this, I want the users to be able to have the option to cancel and choose to retrieve a smaller set of data.

I have it mostly working with the current flow:

1. User clicks on "Load Data..." button
2. Button changes to "Cancel" and async operation to retrieve the data kicks off
3. Data is retrieved and the grid is populated

This is all working well EXCEPT, if the user clicks cancel, it still takes the same amount of time that it would have taken to get all the data for the grid to come up empty. This leads me to believe that the long running operation actually didn't get cancelled... however, when I debug in the "FindForLocationAsync" method, the cancellation token does stop the iterative operation and return from the method early if the user requests a cancellation.

I've been reading as much as I can on this for quite a while but, I'm at a bit of an impasse now. Any help would be very much appreciated.

Cancellation Token Source

CancellationTokenSource cancellationTokenSource = null;

Button Click Method

private async void btnSearch_Click(object sender, EventArgs e)
{
    gridLog.DataSource = null;
    Cursor = Cursors.WaitCursor;

    if (btnSearch.Text.ToLower().Contains("load"))
    {
        btnSearch.Text = "Cancel";
        btnSearch.ForeColor = Color.White;
        btnSearch.BackColor = Color.Red;

        //get params to pass
        /* snip */

        cancellationTokenSource = new CancellationTokenSource();
        await Task.Run(() =>
            {
                var ds = DocLog.FindForLocationAsync(docType, subType, days, currLocation.ID, cancellationTokenSource.Token).Result;
                gridLog.DataSource = ds;
            });

        btnSearch.Text = "Load Data...";
        btnSearch.ForeColor = Color.Black;
        btnSearch.BackColor = Color.FromArgb(225, 225, 225);
    }
    else
    {
        cancelSearch();
        btnSearch.Text = "Load Data...";
        btnSearch.ForeColor = Color.Black;
        btnSearch.BackColor = Color.FromArgb(225, 225, 225);
    }

    Cursor = Cursors.Default;
}

Cancel Method

private void cancelSearch()
{
    if (cancellationTokenSource != null) cancellationTokenSource.Cancel();
}

Long Running Method

public async static Task<BindingList<DocLog>> FindForLocationAsync(string DocType, string SubType, int? LastXDays, Guid LocationID, CancellationToken CancellationToken)
{
    BindingList<DocLog> dll = new BindingList<DocLog>();

    using (SqlConnection sqlConnection = new SqlConnection(Helper.GetConnectionString()))
    {
        sqlConnection.Open();
        using (SqlCommand sqlCommand = new SqlCommand((LastXDays == null) ? "DocLogGetAllForLocation" : "DocLogGetAllForLocationLastXDays", sqlConnection))
        {
            sqlCommand.CommandType = System.Data.CommandType.StoredProcedure;
            sqlCommand.Parameters.Add("@DocType", SqlDbType.NVarChar, 30).Value = DocType.Trim();
            sqlCommand.Parameters.Add("@SubType", SqlDbType.NVarChar, 30).Value = SubType.Trim();
            sqlCommand.Parameters.Add("@LocationID", SqlDbType.UniqueIdentifier).Value = LocationID;
            if (LastXDays != null) { sqlCommand.Parameters.Add("@NumberOfDays", SqlDbType.Int).Value = LastXDays; }

            SqlDataReader sqlDataReader = sqlCommand.ExecuteReader();

            await Task.Run(() =>
            {
                while (sqlDataReader.Read())
                {
                    if (CancellationToken.IsCancellationRequested)
                    {
                        dll = new BindingList<DocLog>();
                        break;
                    }
                    else
                    {
                        DocLog dl = readData(sqlDataReader);
                        dll.Add(dl);
                    }
                }
            });
        }
    }

    return dll;
}

Answer 1

Here is your code modified to be C# idiomatic async:

Note the following:

• Async code generally refers to operations involving asynchronous IO where the completion signal (and subsequent completion callback) is fundamentally made by a hardware interrupt and the OS - it should not be confused with concurrency (i.e. multithreading) even though code running on another thread can also be conceptually modelled as a Task too (indeed, Task is used for both multi-threading (Task.Run) and async-IO).
  • Anyway, the point is: if you're using an async-IO API (such as SqlDataReader, FileStream, NetworkStream, etc) then you probably don't want to use Task.Run.
• Outside of code that must run in the UI thread (i.e. WinForms and WPF UI code) you should always use .ConfigureAwait(false) to allow the completion callback to be invoked in an available background thread, which means the UI thread won't be forced to run background code.
  • The C# language designers are aware of the awful ergonomics of having to spam .ConfigureAwait(false) and are working on a solution.
• Generally speaking, never use Task<T>.Result or Task.Wait() as these block the thread and introduce a risk of deadlocking (because a continuation callback can't be run on a blocked thread). Only use Task<T>.Result after you've verified the Task has completed (or just do await task).
• You should pass the CancellationToken to every child Async method you call.

Other nitpicks:

• You can combine using() statements at the same indentation level and call SqlConnection.OpenAsync after you've created the SqlCommand.
• Parameters should be camelCase not PascalCase.
• References to instance members (fields, methods, properties, etc) should be prefixed with this. so they're visually distinguishable from local identifiers.
• Doing if( this.x != null ) this.x.Foo() is not entirely safe because in a multi-threaded program x could be replaced with another value in between the if and the .Foo() call. Instead use the ?. operator which keeps a local reference to prevent the carpet from being pulled-out from underneath you (it works like this: X lx = this.x; if( lx != null ) lx.Foo() which is guaranteed to be thread-safe).
• BindingList is (arguably) a UI component and should not be returned from a conceptually "background" function like your FindForLocationAsync method, so I return a List<T> instead and then the UI wraps the List<T> in a BindingList<T>.

Code:

private async void btnSearch_Click(object sender, EventArgs e)
{
    this.gridLog.DataSource = null;
    this.Cursor = Cursors.WaitCursor;

    if (this.btnSearch.Text.ToLower().Contains("load"))
    {
        this.btnSearch.Text = "Cancel";
        this.btnSearch.ForeColor = Color.White;
        this.btnSearch.BackColor = Color.Red;

        //get params to pass
        /* snip */

        this.cancellationTokenSource = new CancellationTokenSource();

        List<DocLog> list = await DocLog.FindForLocationAsync(docType, subType, days, currLocation.ID, cancellationTokenSource.Token);
        gridLog.DataSource = new BindingList<DocLog>( list );

        this.btnSearch.Text = "Load Data...";
        this.btnSearch.ForeColor = Color.Black;
        this.btnSearch.BackColor = Color.FromArgb(225, 225, 225);
    }
    else
    {
        CancelSearch();
        this.btnSearch.Text = "Load Data...";
        this.btnSearch.ForeColor = Color.Black;
        this.btnSearch.BackColor = Color.FromArgb(225, 225, 225);
    }

    this.Cursor = Cursors.Default;
}

private void CancelSearch()
{
    this.cancellationTokenSource?.Cancel();
}

public async static Task<List<DocLog>> FindForLocationAsync(string DocType, string SubType, int? LastXDays, Guid LocationID, CancellationToken cancellationToken)
{
    List<DocLog> dll = new List<DocLog>();

    using (SqlConnection sqlConnection = new SqlConnection(Helper.GetConnectionString()))
    using (SqlCommand sqlCommand = sqlConnection.CreateCommand())
    {
        await sqlConnection.OpenAsync(cancellationToken).ConfigureAwait(false);

        sqlCommand.CommandText = (LastXDays == null) ? "DocLogGetAllForLocation" : "DocLogGetAllForLocationLastXDays";
        sqlCommand.CommandType = System.Data.CommandType.StoredProcedure;
        sqlCommand.Parameters.Add("@DocType", SqlDbType.NVarChar, 30).Value = DocType.Trim();
        sqlCommand.Parameters.Add("@SubType", SqlDbType.NVarChar, 30).Value = SubType.Trim();
        sqlCommand.Parameters.Add("@LocationID", SqlDbType.UniqueIdentifier).Value = LocationID;
        if (LastXDays != null) { sqlCommand.Parameters.Add("@NumberOfDays", SqlDbType.Int).Value = LastXDays; }

        using( SqlDataReader sqlDataReader = await sqlCommand.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false) )
        {
            while (await sqlDataReader.ReadAsync(cancellationToken).ConfigureAwait(false))
            {
                if (cancellationToken.IsCancellationRequested) break;

                DocLog dl = readData(sqlDataReader);
                dll.Add(dl);
            }
        }
    }

    return dll;
}