views:

1106

answers:

7

I've been struggling with this for quite a while: I have a function designed to add control to a panel with cross-thread handling, the problem is that though the panel and the control are in "InvokeRequired=false" - I get an exception telling me that one of the controls inner controls are accessed from a thread other than the thread it was created on, the snippet goes like this:

public delegate void AddControlToPanelDlgt(Panel panel, Control ctrl);
    public void AddControlToPanel(Panel panel, Control ctrl)
    {
        if (panel.InvokeRequired)
        {
            panel.Invoke(new AddControlToPanelDlgt(AddControlToPanel),panel,ctrl);
            return;
        }
        if (ctrl.InvokeRequired)
        {
            ctrl.Invoke(new AddControlToPanelDlgt(AddControlToPanel),panel,ctrl);
            return;
        }
        panel.Controls.Add(ctrl); //<-- here is where the exception is raised
    }

the exception message goes like this:

"Cross-thread operation not valid: Control 'pnlFoo' accessed from a thread other than the thread it was created on"

('pnlFoo' is under ctrl.Controls)

How can I add ctrl to panel?!


When the code reaches the "panel.Controls.Add(ctrl);" line - both panel and ctrl "InvokeRequired" property is set to false, the problem is the the controls inside ctrl has "InvokeRequired" set to true. To clarify things: "panel" is created on the base thread and "ctrl" on the new thread, therefore, "panel" has to be invoked (causing "ctrl" to need invoke again). Once both of the invokes are done, it reaches the panel.Controls.Add(ctrl) command (both "panel" and "ctrl" doesn't need invocation in this state)

Here is a small snippet of the full program:

public class ucFoo : UserControl
{
    private Panel pnlFoo = new Panel();

    public ucFoo()
    {
        this.Controls.Add(pnlFoo);
    }
}

public class ucFoo2 : UserControl
{
    private Panel pnlFooContainer = new Panel();

    public ucFoo2()
    {
         this.Controls.Add(pnlFooContainer);
         Thread t = new Thread(new ThreadStart(AddFooControlToFooConatiner());
         t.Start()
    }

    private AddFooControlToFooConatiner()
    {
         ucFoo foo = new ucFoo();
         this.pnlFooContainer.Controls.Add(ucFoo); //<-- this is where the exception is raised
    }
}
+3  A: 

Where is pnlFoo being created, and in which thread? Do you know when its handle is being created? If it's being created in the original (non-UI) thread, that's the problem.

All control handles in the same window should be created and accessed on the same thread. At that point, you shouldn't need two checks for whether Invoke is required, because ctrl and panel should be using the same thread.

If this doesn't help, please provide a short but complete program to demonstrate the problem.

Jon Skeet
That is not actually true. The control *object* can be created on any thread, its *handle* cannot be created on a different thread. Since the OP is actually dealing with this very detail, the difference is more than relevant.
Remus Rusanu
Thanks Remus - will edit.
Jon Skeet
(If you could check that the edit is correct, that would help a lot.)
Jon Skeet
I totally believe OP shouldn't be doing this the way it does. It is obviously too obscure and complicated to rely on the threading difference between the managed C# wrapper and the underlying GDI handle, and everybody will fall into the same traps when working with that code.
Remus Rusanu
+1  A: 

In your own answer you state:

To clarify things: "panel" is created on the base thread and "ctrl" on the new thread

I think this might be the cause of your problem. All UI elements should be created on the same thread (the base one). If you need to create "ctrl" as a consequence of some action in the new thread, then fire an event back to the base thread and do the creation there.

ChrisF
+3  A: 

As an aside - to save yourself having to create countless delegate types:

if (panel.InvokeRequired)
{
    panel.Invoke((MethodInvoker) delegate { AddControlToPanel(panel,ctrl); } );
    return;
}

Additionally, this now does regular static checks on the inner call to AddControlToPanel, so you can't get it wrong.

Marc Gravell
Tried that but still the same error comes upThe problem is that once panel is invoked, ctrl needs to be invoked too
Nissim
Oh, I realise that- I agree 100% with Jon's analysis (that you are somehow creating the control(s) on a worker thread) - I was just trying to show a trick to save some typing and improve static checking.
Marc Gravell
+2  A: 

'panel' and 'ctrl' must be created on the same thread, ie. you cannot have panel.InvokeRequired return different value than ctrl.InvokeRequired. That is if both panel and ctrl have the handles created or belong to a container with the handle created. From MSDN:

If the control's handle does not yet exist, InvokeRequired searches up the control's parent chain until it finds a control or form that does have a window handle. If no appropriate handle can be found, the InvokeRequired method returns false.

As it is right now your code is open to race conditions because the panel.InvokeNeeded can return false because the panel is not yet created, then ctrl.InvokeNeeded will certainly return false because most likely ctrl is not yet added to any container and then by the time you reach panel.Controls.Add the panel was created in the main thread, so the call will fail.

Remus Rusanu
So what is the best way for doing that?
Nissim
It depends on many factors. You can also have problems if panel is added and removed from its container during its lifetime (so its handle gets destroyed). So I'd say that a safe approach would be to have an extra parameter, probably the main form, that is guaranteed to be created, and that should be used for cross thread check and invoke. Your background thread should not be started until this form is created.
Remus Rusanu
BTW I also agree with the other posts that your probably shouldn't be doing what you're doing, trying to create the control objects in background threads and realize the handle in the main thread. Rather have the background threads pass *data* to the main thread and always create the controls on the main thread.
Remus Rusanu
+1  A: 

Here is a working piece of code :

public delegate void AddControlToPanelDlg(Panel p, Control c);

        private void AddControlToPanel(Panel p, Control c)
        {
            p.Controls.Add(c);
        }

        private void AddNewContol(object state)
        {
            object[] param = (object[])state;
            Panel p = (Panel)param[0];
            Control c = (Control)param[1]
            if (p.InvokeRequired)
            {
                p.Invoke(new AddControlToPanelDlg(AddControlToPanel), p, c);
            }
            else
            {
                AddControlToPanel(p, c);
            }
        }

And here is how I tested it. You need to have a form with 2 buttons and one flowLayoutPanel (I chose this so I didn't have to care about location hwhen dinamically adding controls in the panel)

private void button1_Click(object sender, EventArgs e)
        {
            AddNewContol(new object[]{flowLayoutPanel1, CreateButton(DateTime.Now.Ticks.ToString())});
        }

        private void button2_Click(object sender, EventArgs e)
        {
            ThreadPool.QueueUserWorkItem(new WaitCallback(AddNewContol), new object[] { flowLayoutPanel1, CreateButton(DateTime.Now.Ticks.ToString()) });
        }

I that he probem with your exaple is that when you get in the InvokeRequired branch you invoke the same function wou are in, resulting in a strange case of recurssion.

AlexDrenea
Tried that... it's actually a more advanced synonym of the normal invoke... but as I mentioned, once "panel" is invoked - "ctrl" InvokeRequired=true
Nissim
not sure what to say, it works fine for me. was your code exacly as what I pasted above? sometimes the smallest detail can lead to an error
AlexDrenea
A: 

Here is a small snippet of the full program:

public class ucFoo : UserControl
{
    private Panel pnlFoo = new Panel();

    public ucFoo()
    {
        this.Controls.Add(pnlFoo);
    }
}

public class ucFoo2 : UserControl
{
    private Panel pnlFooContainer = new Panel();

    public ucFoo2()
    {
         this.Controls.Add(pnlFooContainer);
         Thread t = new Thread(new ThreadStart(AddFooControlToFooConatiner());
         t.Start()
    }

    private AddFooControlToFooConatiner()
    {
         ucFoo foo = new ucFoo();
         this.pnlFooContainer.Controls.Add(ucFoo); //<-- this is where the exception is raised
    }
}
Nissim
+1  A: 

Lots of interesting answers here, but one key item for any multithreading in a Winform app is using the BackgroundWorker to initiate threads, and communicate back to the main Winform thread.

Cylon Cat