BackgroundWorker acting bizarrely...

Question

Hi guys

I'm working on some code that calls a service. This service call could fail and if it does I want the system to try again until it works or too much time has passed.

I am wondering where I am going wrong as the following code doesn't seem to be working correctly... It randomly only does one to four loops...

protected virtual void ProcessAsync(object data, int count)
{
    var worker = new BackgroundWorker();
    worker.DoWork += (sender, e) =>
    {
        throw new InvalidOperationException("oh shiznit!");
    };
    worker.RunWorkerCompleted += (sender, e) =>
    {
        //If an error occurs we need to tell the data about it
        if (e.Error != null)
        {
            count++;
            System.Threading.Thread.Sleep(count * 5000);
            if (count <= 10)
            {
                if (count % 5 == 0)
                    this.Logger.Fatal("LOAD ERROR - The system can't load any data", e.Error);
                else
                    this.Logger.Error("LOAD ERROR - The system can't load any data", e.Error);
                this.ProcessAsync(data, count);
            }
        }
    };
    worker.RunWorkerAsync();
}

Cheers Anthony

UPDATE:

I've switched my code over to use ThreadPool.QueueUserWorkItem instead... Since doing this my problems have gone away and semantically I can do the same thing. Thanks for all your help guys.

Answer 1

This must be the most bizarre retry mechanism I saw. Can you make a cleaner one? Avoid recursive calls unless they are simple and easy to maintain, because it can easily lead to mistakes.

Answer 2

I've modified your code slightly and don't have any issues running through 10 iterations (VS 2008 Express) which leads me to this: Is this the actual code and if not, are you sure you've sent enough to reproduce the issue?

If I were to venture a guess, I would say that the count you're sending it varies such that count % 5 > 0 and that there's an exception getting thrown in Logger.Fatal.

private void button1_Click(object sender, EventArgs e)
{
    ProcessAsync("beer", 1);
}

protected virtual void ProcessAsync(object data, int count)
{
    var worker = new BackgroundWorker();
    worker.DoWork += (sender, e) =>
    {
        throw new InvalidOperationException("oh shiznit!");
    };
    worker.RunWorkerCompleted += (sender, e) =>
    {
        //If an error occurs we need to tell the data about it
        if (e.Error != null)
        {
            count++;
            //System.Threading.Thread.Sleep(count * 5000);
            if (count <= 10)
            {
                if (count % 5 == 0)
                    this.Logger.Fatal("LOAD ERROR - The system can't load any data - " + count.ToString(), e.Error);
                else
                    this.Logger.Error("LOAD ERROR - The system can't load any data - " + count.ToString(), e.Error);
                this.ProcessAsync(data, count);
            }
        }
    };
    worker.RunWorkerAsync();
}

SomeLogger Logger = new SomeLogger();

class SomeLogger
{
    public void Fatal(string s, Exception e)
    {
        System.Diagnostics.Debug.WriteLine(s);
    }

    public void Error(string s, Exception e)
    {
        System.Diagnostics.Debug.WriteLine(s);
    }
}

EDIT: A Suggestion
Put a try-catch around the call to Logger.Fatal and see what happens.

EDIT: Another suggestion
I suspect you aren't sharing enough code for us to help. The key to success here would be to isolate the problem in a dummy project that only has enough code to show the failure. I'd be willing to bet that if you can do that, you most likely wouldn't need to post this as question here...

You can start with my assumptions and should see that this works just fine. Then start changing the generalized code into what you're actually using (I'd start with the real implementation of Logger.Fatal). The error will likely become pretty obvious in short order.

Answer 3

I don't see an obvious reason. However, your RunWorkerCompleted event would normally run on the UI thread. And hang it for as long as 55 seconds. That cannot be desirable.

There isn't any reason I can think of why you'd not just loop in the DoWork method with a try/catch block.

Answer 4

There is one thing that is really bad!

Within your RunWorkerCompleted() you call a Thread.Sleep(). Due to the fact, that this function will be processed within the GUI thread your application is going to freeze!

Please don't call Thread.Sleep() within any event of the BackgroundWorker cause all of them will be processed within the GUI thread.

So this is maybe not a real solution to your current problem, but definitely something you should care about.

Update
To start something after a given time period you should take a look into the various timer classes. Each of them has it's own pros and cons. For a more insight view you should take a look into this article.