Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Closing or Hiding forms causes a cross thread error

Tags:

c#

.net

winforms

I am baffled by this simple task i do over and over again.

I have an array of child forms. The array is initiated in another form's constructor:

frmChildren = new ChildGUI[20];

When the user requests to see a child form, i do this:

if (frmChildren[nb] == null)
{
    frmChildren[nb] = new ChildGUI();
    frmChildren[nb].MdiParent = this.MdiParent;
}
frmChildren[nb].Show();

So far this works. In the background i can download new content for these forms. When a download is finished i fire a ChildChange event. Here is where it stops working. I simply want to close/hide any forms open then regenerate a new set of -frmChildren = new ChildGUI[20];- here is one of many trials:

        for (int i = 0; i < frmChildren.Length;i++ )
        {
            if (frmChildren[i] != null)
            {
                //frmChildren[i].BeginInvoke(new EventHandler(delegate
                //{
                    frmChildren[i].Close();
                //}));
            }
        }             
        frmChildren= new ChildGUI[20];

I get a Cross Thread exception on the .Close(). Notice i've already tried doing an invoke, but doing so bypasses the !=null for some reason. I think it may have something to do with the garbage collector. Anybody have an input?

like image 387
Roast Avatar asked Jan 23 '23 13:01

Roast


1 Answers

The problem is that your anonymous method is capturing i - so by the time it's actually invoked in the UI thread, you've got a different value of i, which may be null. Try this:

for (int i = 0; i < frmChildren.Length; i++)
{
    ChildGUI control = frmChildren[i];
    if (control != null)
    {
        control.BeginInvoke(new EventHandler(delegate
        {
            control.Close();
        }));
    }
}             
frmChildren = new ChildGUI[20];

See Eric Lippert's blog post for why introducing a new variable within the loop fixes the problem.

EDIT: If you want to use a foreach loop, it would look like this:

foreach (ChildGUI control in frmChildren)
{
    // Create a "new" variable to be captured
    ChildGUI copy = control;
    if (copy != null)
    {
        copy.BeginInvoke(new EventHandler(delegate
        {
            copy.Close();
        }));
    }
}             
frmChildren = new ChildGUI[20];

Just as an aside, you can use the fact that you just want to call a void method to make the code slightly simpler. As this no longer uses an anonymous method, you can make do away with the "inner" variable:

foreach (ChildGUI control in frmChildren)
{
    if (control != null)
    {
        control.BeginInvoke(new MethodInvoker(control.Close));
    }
}             
frmChildren = new ChildGUI[20];
like image 179
Jon Skeet Avatar answered Jan 25 '23 23:01

Jon Skeet