Multi-Threading pauses form

Question

I am doing some MultiThreading but the form pauses on load.

I am trying to display the form, and then in the background it should populate the combobox without pausing the form.

On my Form_Load event, I have this:

private void frmIni_Load(object sender, EventArgs e)
{
      Application.DoEvents();
      Thread threadOne = new Thread(GetServers);
      threadOne.Start();
}

In my GetServers() method:

private void GetServers()
{
       cboServer.BeginInvoke(
          (Action)(() => {
              servers = SmoApplication.EnumAvailableSqlServers(false);
              Thread.Sleep(1);
              foreach (DataRow server in servers.Rows)
              {
                  cboServer.Properties.Items.Add(server["Name"]);
                  Thread.Sleep(1);
              }
        }));
}

What am I missing here? The form should not pause, it should work and then eventually when the thread completes, it should just populate the combobox.

Because of `BeginInvoke`, all of your code is executed on the UI thread. — laszlokiss88, Dec 05 '13 at 12:00
BTW If you are using `Application.DoEvents();` then you are doing it wrong. — Matthew Watson, Dec 05 '13 at 12:02
you really shouldn't be using Application.DoEvents. its bad practice and can cause unexpected side effects. Also doing Thread.Sleep(1) wont even put the thread to sleep because that is not enough time to put the thread to sleep. Also you shouldn't really put anything to sleep here, or anywhere for that matter unless you are simulating something perhaps like network lag etc... I think for your particular scenario, you should put a thread on the threadpool and let it do the thread management for you — Ahmed ilyas, Dec 05 '13 at 12:02

score 6 · Accepted Answer · edited May 23 '17 at 12:03

6

Yeah so the reason it blocks the UI is simply because no real code is running in the new thread. In your call to GetServers you call back into the UI thread and then do the busy stuff (you may as well not use a thread at all here...).

You want to put any long-running work in the thread and only callback into the UI thread when you want to update it e.g.

private void frmIni_Load(object sender, EventArgs e)
{
    Task.Factory.StartNew(() => {
        return SmoApplication.EnumAvailableServers(false);
    }).ContinueWith((task) => {
        foreach (var server in task.Result)
        {
            cboServer.Properties.Items.Add(server["Name"]);
        }
    }, TaskScheduler.FromCurrentSynchronizationContext());
}

Points to note

Do not use DoEvents when trying to multi-thread, especially when you don't understand it's usage.
Do not spin up new threads unless you are sure you need to (e.g. long running I/O task)
Do not use Thread.Sleep to simulate "pausing" (which you appear to be doing)
Do use the thread-pool for short-lived work (like this).

edited May 23 '17 at 12:03

Community

1
1

answered Dec 05 '13 at 12:01

James

80,725
18
167
237

Thank you James. Perfect :-) I see my problem now. – Fred Dec 05 '13 at 12:08
Using BackgroundWorker is a better solution – Franck Dec 05 '13 at 12:44
@Franck and what evidence do you have to support that statement? If anything, the TPL is far superior to a background worker given it's support for cancellation tokens, continuations & nested tasks etc. – James Dec 05 '13 at 13:14
i was basing myself on performance only. The guy play with server stuff. I base myself on our thermodynamic simulator that we made switch from TPL to Background worker and went from 50-55 min operations down to 18-20 minutes. Seems to us that TPL vs Background worker performance is like comparing for loop vs LINQ – Franck Dec 05 '13 at 13:29

score 1 · Answer 2 · answered Dec 05 '13 at 12:03

Because of the BeginInvoke, all of your code is executed on the UI thread.

private void GetServers()
{
    servers = SmoApplication.EnumAvailableSqlServers(false);
    Thread.Sleep(1); // Why?
    cboServer.BeginInvoke(
            (Action)(() => {
                foreach (DataRow server in servers.Rows)
                {
                    cboServer.Properties.Items.Add(server["Name"]);
                    Thread.Sleep(1); // Why?
                }
            })
        );
}

This is what I wanted. This works as expected. But I am going to go with James answer. Thank you :-). — Fred, Dec 05 '13 at 12:08

score 1 · Answer 3 · answered Dec 05 '13 at 12:03

First: creating a Thread will consume a small amount of time. Also, you are dispatching your actual work back to the UI with your BeginInvoke

cboServer.BeginInvoke(
    (Action)(() => {
        servers = SmoApplication.EnumAvailableSqlServers(false);
        Thread.Sleep(1);
        foreach (DataRow server in servers.Rows)
        {
            cboServer.Properties.Items.Add(server["Name"]);
            Thread.Sleep(1);
        }
    })
);

You should try to move everything besides UI updates out of the BeginInvoke delegate, somehow like:

private void GetServers()
{
       servers = SmoApplication.EnumAvailableSqlServers(false);

       cboServer.BeginInvoke(
          (Action)(() => {
              foreach (DataRow server in servers.Rows)
              {
                  cboServer.Properties.Items.Add(server["Name"]);
              }
        }));
}

score 0 · Answer 4 · answered Dec 05 '13 at 12:06

here is the right way to do it for your scenario/situation.

you should create a thread on the threadpool and let it do thread management. furthermore, when your thread gets kicked/invoked by the pool, and once you have get all your SQL Servers, you then add to the combobox by doing begin invoke so you can correctly switch between the UI thread and current thread to update your comboboxes.

Threadpool:

http://msdn.microsoft.com/en-us/library/vstudio/system.threading.threadpool.queueuserworkitem(v=vs.100).aspx

http://msdn.microsoft.com/en-us/library/vstudio/kbf0f1ct(v=vs.100).aspx

And in your method that gets invoked by the pool, add items to the combobox by doing your begininvoke.

UPDATE - I see @James posted pretty much what I am proposing. Great minds think alike.

score 0 · Answer 5 · answered Dec 05 '13 at 12:22

Despite serious answers, your code needs very little to work as intended:

private void GetServers()
{
    servers = SmoApplication.EnumAvailableSqlServers(false);
    // Thread.Sleep(1);
    foreach (DataRow server in servers.Rows)
    {
          cboServer.BeginInvoke((Action)(() => { cboServer.Properties.Items.Add(server["Name"]);}));
        // Thread.Sleep(1);
    }
}

Multi-Threading pauses form

5 Answers5