Backgroundworker cancel the worker

Question

I'm facing some troubles when trying to cancel the Backgroundworker. I've read dozens os similiar topics such as How to stop BackgroundWorker correctly, How to wait correctly until BackgroundWorker completes? but I'm not reaching anywhere.

What's happening is that I have a C# application that uses a PHP WebService to send info to a MySQL database. If the user, for some reason, clicks (in the form) on the "back" or "stop" button, this is the code that is fired:

BgWorkDocuments.CancelAsync();
BgWorkArticles.CancelAsync();

I do understand that the request is Asynchronous, therefore the cancelation might take 1 or 2 seconds, but it should stop..and that doesn't happen at all. Even after clicked "back" (the current form is closed and a new one is opened) the backgroundworker continues to work, because I keep seeing data being inserted into MySQL.

foreach (string[] conn in lines)
 {

    string connectionString = conn[0];

    FbConnection fbConn = new FbConnection(connectionString);
    fbConn.Open();

    getDocuments(fbConn);

    // Checks if one of the backgrounds is currently busy
    // If it is, then keep pushing the events until stop.
    // Only after everything is completed is when it's allowed to close the connection.
    // 
    // OBS: Might the problem be here?
    while (BgWorkDocuments.IsBusy == true || BgWorkArticles.IsBusy == true)
    {
        Application.DoEvents();
    }

    fbConn.Close();
}

The above code is needed because I might have multiple databases, that's why I have the loop.

private void getDocuments(FbConnection fbConn)
{
    BgWorkDocuments.RunWorkerAsync();

    BgWorkDocuments.DoWork += (object _sender, DoWorkEventArgs args) =>
    {

        DataTable dt = getNewDocuments(fbConn);

        for (int i = 0; i <= dt.Rows.Count - 1; i++)
        {

            // Checks if the user has stopped the background worker
            if (BgWorkDocuments.CancellationPending == false)
            {
                // Continue doing what has to do..
                sendDocumentsToMySQL((int)dt.Rows[i]["ID"]);
            }
        }

        // After the previous loop is completed, 
        // start the new backgroundworker

        getArticles(fbConn);

    };
}

private void getArticles(FbConnection fbConn)
{
    BgWorkArticles.RunWorkerAsync();

    BgWorkArticles.DoWork += (object _sender, DoWorkEventArgs args) =>
    {

        DataTable dt = getNewArticles(fbConn);

        for (int i = 0; i <= dt.Rows.Count - 1; i++)
        {

            // Checks if the user has stopped the background worker
            if (BgWorkArticles.CancellationPending == false)
            {
                // Continue doing what has to do..
                sendArticlesToMySQL((int)dt.Rows[i]["ID"]);
            }
        }

    }; 
}

Answer 1

I agree with the comments expressing surprise that the code even works, due to the apparent ordering problem of the call to RunWorkerAsync() vs when you actually subscribe to the DoWork event. Additionally, your use of DoEvents() is unwarranted and should be removed (as is the case with any use of DoEvents()).

I also note that your workers don't actually exit when you try to cancel them. You just skip the processing, but continue to loop on the rows. Without seeing the rest of the code, it's impossible to know what's going on, but it's possible that after you cancel, the CancellationPending property gets reset to false, allowing the loops to start doing things again.

The lack of a complete code example is a real impediment to understanding the full detail of what's going on.

That said, IMHO this does not seem to be a case where you actually need BackgroundWorker at all, not with the new async/await feature in C#. Given that network I/O is involved, my guess is that each call to sendDocumentsToMySQL() and sendArticlesToMySQL() can be executed individually in the thread pool without too much overhead (or may even be able to be written as async I/O methods…again, lacking detail as to their specific implementation prevents any specific advise in that respect). Given that, your code could probably be rewritten so that it looks more like this:

private CancellationTokenSource _cancelSource;

private void stopButton_Click(object sender, EventArgs e)
{
    if (_cancelSource != null)
    {
        _cancelSource.Cancel();
    }
}

private async void startButton_Click(object sender, EventArgs e)
{
    using (CancellationTokenSource cancelSource = new CancellationTokenSource)
    {
        _cancelSource = cancelSource;

        try
        {
            foreach (string[] conn in lines)
            {
                string connectionString = conn[0];

                FbConnection fbConn = new FbConnection(connectionString);
                fbConn.Open();

                try
                {
                    await getDocuments(fbConn, cancelSource.Token);
                    await getArticles(fbConn, cancelSource.Token);
                }
                catch (OperationCanceledException)
                {
                    return;
                }
                finally
                {
                    fbConn.Close();
                }
            }
        }
        finally
        {
            _cancelSource = null;
        }
    }
}

private async Task getDocuments(FbConnection fbConn, CancellationToken cancelToken)
{
    DataTable dt = await Task.Run(() => getNewDocuments(fbConn));

    for (int i = 0; i <= dt.Rows.Count - 1; i++)
    {
        cancelToken.ThrowIfCancellationRequested();

        await Task.Run(() => sendDocumentsToMySQL((int)dt.Rows[i]["ID"]));
    }
}

private async Task getArticles(FbConnection fbConn, CancellationToken cancelToken)
{
    DataTable dt = await Task.Run(() => getNewArticles(fbConn));

    for (int i = 0; i <= dt.Rows.Count - 1; i++)
    {
        cancelToken.ThrowIfCancellationRequested();

        await Task.Run(() => sendArticlesToMySQL((int)dt.Rows[i]["ID"]));
    }
}