Proper way to cancel WebClient in BackgroundWorker

Question

I have a WPF application that needs to get a XML document using a web request. The request is based on an id that the user enters. If the user enters a second id, before the first returns, I would like to cancel the first request, and start over with the second id. The following works, but I'm not sure if there is an easier way, or if this is correct. I am basically passing a ManualResetEvent reset event to a BackgroundWorker so that I can signal the thread to cancel the web request.

Is this approach poor form? Is passing in a reset event better than having a loop within the function that tests the DoWorkEventArgs::Cancel property every tenth of a second? Also, is there a way to delay the call to RunWorkerAsync until the background worker isn't busy, without blocking the thread? Or should I just create a new background worker every time?

Any input is much appreciated.

This is what I have in my window.xaml.cs.

    BackgroundWorker itemBackgroundWorker = new BackgroundWorker();
ManualResetEvent itemCancel = new ManualResetEvent(false);
private void tbID_TextChanged(object sender, TextChangedEventArgs e)
{
    int id;
    if (!int.TryParse(textBoxID.Text, out id))
        return;

    while (itemBackgroundWorker.IsBusy)
    {
        itemCancel.Set();
    }

    itemCancel.Reset();
    itemBackgroundWorker.RunWorkerAsync(new object[] { id, itemCancel });

}
void baselineSessionBackgroundWorker_DoWork(object sender, System.ComponentModel.DoWorkEventArgs e)
{
    Object[] args = (Object[])e.Argument;

    int id = (int)args[0];
    ManualResetEvent cancel = (ManualResetEvent)args[1];
    try
    {
        currentItem = webFunctions.getItem(id, cancel);
    }
    catch (Exception exx)
    {
        System.Diagnostics.Debug.WriteLine("Find run {0} failed with {1}", id, exx.Message);
        currentItem = null;
    }
}
void baselineSessionBackgroundWorker_RunWorkerCompleted(object sender, System.ComponentModel.RunWorkerCompletedEventArgs e)
{
    currentItemContentControl.GetBindingExpression(ContentControl.ContentProperty).UpdateTarget();
}

This is what I have for the static getItem method.

static string detailXMLQuery = "http://....";
public static myItem getItem(int id, ManualResetEvent cancelEvent = null)
{
    ManualResetEvent readFinishedEvent = new ManualResetEvent(false);
    if (cancelEvent == null)
        cancelEvent = new ManualResetEvent(true);
    else
        cancelEvent.Reset();

    Stream result = null;
    WebClient client = new WebClient();
    client.OpenReadCompleted += delegate(object sender, OpenReadCompletedEventArgs e)
    {
        if (e.Error != null)
            System.Diagnostics.Debug.WriteLine("Error getting XML:" + e.Error.Message);
        else
            result = e.Result;

        System.Diagnostics.Debug.WriteLine("Set finished state");
        readFinishedEvent.Set();
    };
    client.OpenReadAsync(new Uri(string.Format(detailXMLQuery, id)));

    ManualResetEvent.WaitAny(new WaitHandle[] { cancelEvent, readFinishedEvent }, 10000);

    if (!readFinishedEvent.WaitOne(0))
    {
        client.CancelAsync();
        System.Diagnostics.Debug.WriteLine("Error getting XML: The request timed out or was canceled");
        throw new Exception("The request timed out or was canceled");
    }

    XmlDocument doc = new XmlDocument();
    doc.Load(result);

    //parse xml, test cancelEvent.Wait(0) to see if cancel has been signaled.

    return new myItem();
}

Answer 1

Is this approach poor form?

I think yes

Is passing in a reset event better than having a loop within the function that tests the DoWorkEventArgs::Cancel property every tenth of a second?

Both are wrong. Event is a kernel object! Using a kernel object for this type task is wasteful. It would be better if you used something like this:

webClient.DownloadStringCompleted += new DownloadStringCompletedEventHandler(DownloadStringCallback2);
client.DownloadStringAsync (uri);

In this case, you don't need to block any thread with a loop. When your request completes, you will take the control in passed method (DownloadStringCallback2).

Also, is there a way to delay the call to RunWorkerAsync until the background worker isn't busy, without blocking the thread?

At first glance - yes, you can pass in worker method CancellationToken. When the user inputs a new value, you simply cancel the token and start a new task. When the cancelled request takes the control in DownloadStringCallback2 it should check the token and return. If it wasn't cancelled, it may continue work. On each user request you should simply cancel the old token and create a new one, then start a new request.

Or should I just create a new background worker every time?

I think it's OK at first glance. On each user request create a new BackgroundWorker and pass the cancellation token. The BackgroundWorker will use ThreadPool behind the scenes anyway.