ShowDialog in BackgroundWorker RunWorkerCompleted

Question

I'm working on a WinForms application that is launching several background workers. When one background worker is complete, if the result is a failure, it will show a dialog box via the ShowDialog(this) method. The problem is when multiple background worker results are fail, it will show multiple dialogs at the same time. I didn't think this was possible, but it apparently is. I was reading some stuff about the the Message Loop and it seems that even though a dialog is open, the message loop is still processing messages, which means that the runworkercompleted will be called even if a dialog is open already. I though that I could use a "lock (myObject)" on the dialog, but it appears that it does not, I'm guessing since that same thread is calling the lock each time.

So what is the appropriate way to fix this? I'm half tempted to just use a flag and a loop like this:

public bool dialogOpen = false;
public bool cancelMessages = false;
public void x_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    while (dialogOpen) {}
    if (cancelMessages) return;
    dialogOpen = true;
    MyDialog dlg = new MyDialog("Something went wrong.");
    if (dlg.ShowDialog(this) == DialogResult.Cancel) cancelMessages = true;
    dialogOpen = false;
}

Would this even work? Would this cause other bad things to happen? (Would this block the message loop?)

Answer 1

I ended up doing the bool check with a message queue. This appears to be working. If there is a race condition like Sinatr was suggesting, then I'm not sure why, as from my understanding, it is all processing on the UI thread.

Here is a chopped version of what I did to get it working.

public List<BGWOProcess> messageQueue = new List<BGWOProcess>();
public static bool DialogOpen = false;
public static bool CancelPending = false;

public void loginProcess_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    BGWOProcess result = (BGWOProcess)e.Result;

    if (!result.Result)
    {
        if (CancelPending) return;
        if (!DialogOpen) DialogOpen = true;
        else
        {
            messageQueue.Add(result);
            return;
        }

        try
        {
            processFailedMessage(result);
        }
        catch (Exception) { }
        DialogOpen = false;
    }
    else
    {
        //...
    }
}

public void processFailedMessage(BGWOProcess result)
{
    MyMessage msg = new MyMessage("The process " + result.Label + " failed: " + result.Error + " Retry?");
    if (msg.ShowDialog(this) == System.Windows.Forms.DialogResult.Yes)
    {
        // Retry request.
        Queue(result.func, result.Label, result.progressIncrement);

        if (messageQueue.Count > 0)
        {
            BGWOProcess nextMessage = messageQueue[0];
            messageQueue.Remove(nextMessage);
            processFailedMessage(nextMessage);
        }
    }
    else
    {
        r = false;
        CancelPending = true;

        // Fail.
        DialogResult = System.Windows.Forms.DialogResult.Abort;
    }
}

Answer 2

You'd have to ask the user from inside the BackgroundWorker() worker threads, the DoWork() methods themselves. A simple lock statement will prevent them from attempting to display more than one dialog. You can use Invoke() to properly display the dialog on the main UI thread.

Here's a simplified example:

    private void button1_Click(object sender, EventArgs e)
    {
        for(int i = 1; i <=5; i++)
        {
            BackgroundWorker x = new BackgroundWorker();
            x.DoWork += x_DoWork;
            x.RunWorkerCompleted += x_RunWorkerCompleted;
            x.RunWorkerAsync();
        }
    }

    private bool cancelMessages = false;
    private Object dialogLock = new object();

    void x_DoWork(object sender, DoWorkEventArgs e)
    {

        // ... some work ...
        System.Threading.Thread.Sleep(5000); // five seconds worth of "work"

        if (true) // some error occurred
        {
            lock (dialogLock) // only one worker thread can enter here at a time
            {
                if (!cancelMessages) // if error messages haven't been turned off, ask the user
                {
                    // ask the user on the main UI thread:
                    // *Invoke() is SYNCHRONOUS, so code won't go continue until after "dlg" is dismissed
                    this.Invoke((MethodInvoker)delegate() {
                        MyDialog dlg = new MyDialog("Something went wrong.");
                        if (dlg.ShowDialog(this) == DialogResult.Cancel) 
                            cancelMessages = true;
                    });
                }
            }
        }
    }

    public void x_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        Console.WriteLine("RunWorkerCompleted");
    }

Answer 3

ShowDialog is not blocking anything (so all your events will keep firing), only code in that method, where you show modal form, will wait for it to close.

Your idea with variables is almost good. If you want to display just one dialog for all workers, then (not so perfect solution)

public bool dialogOpen = false;
public void x_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    // check
    if(dialogOpen)
        return;
    dialogOpen = true;
    (new MyDialog("Something went wrong.")).ShowDialog(this);
    dialogOpen = false;
}

Problem here is race condition, which occurs if several workers will check dialogOpen when it is not yet set to true by either. If you want it to be perfect, then use ManualResetEvent instead.

However, you seems to want what all workers will display error, but only one error at time. This is harder, your solution is wrong, because you are blocking UI thread itself. The easiest would be to prevent (block) workers themselves from completing, if one of them is using dialog (same as before, use ManualResetEvent).

If you have difficulties with the code, I'll help you tomorrow.