13

I'm a newbie with multithreading. I have a winform that have a label and a progress bar.

I wanna show the processing result. Firstly, I use Application.DoEvents() method. But, I find that the form is freezing.

Then I read some article about multithreading at MSDN.

Secondly, I use a BackgroundWorker to do it.

this.bwForm.DoWork += (o, arg) => { DoConvert(); };
this.bwForm.RunWorkerAsync();

The form doesn't freeze that I can drag/drog when processing. Unfortunately, it throws an InvalidOperationException. So I have to use this. Control.CheckForIllegalCrossThreadCalls = false; I ensure that's not a final solution.

Do you have some suggestion to do so, Experts?

Edit: When I call listview throw InvalidOperationException. This code is in DoWork().

          foreach (ListViewItem item in this.listView1.Items)
            {
                //........some operation
                 lbFilesCount.Text = string.Format("{0} files", listView1.Items.Count);
                 progressBar1.Value++;
            }

Edit2: I use delegate and invoke in DoWork() and it don't throw exeption. But the form is freezing again. How to do it so the form could be droppable/drappable while processing?

Greg
  • 15
  • 7
Justin
  • 676
  • 1
  • 8
  • 24
  • What are you doing in the DoConvert thread (e.g. is it manipulating any GUI?) – Yet Another Geek Jun 16 '11 at 09:40
  • 2
    The problems are inside DoConverrt(). Turning the checks off is only hiding the problem, not solving anything. – H H Jun 16 '11 at 09:40
  • You may be trying to access a control from a thread that hasn't created it. To do this you would need to check to see if the properpty of invoke required of the label is true if it is invoke the control and then change its value. http://www.ureader.com/msg/144644873.aspx – stuartmclark Jun 16 '11 at 09:45
  • @Yet Another Geek,@Henk Holterman, DoConvert() is to do some modifications to the label and progressbar. – Justin Jun 16 '11 at 09:54
  • You have not shown us the delegate, the `Invoke`, nor the `DoWork` code. You have to show us the code for us to diagnose it. – Dour High Arch Jun 24 '11 at 16:00

9 Answers9

11

You can only set progress bar user control properties from the UI thread (the WinForm one). The easiest way to do it with your BackgroundWorker is to use the ProgressChanged event:

private BackgroundWorker bwForm;
private ProgressBar progressBar;

In WinForm constructor:

this.progressBar = new ProgressBar();
this.progressBar.Maximum = 100;
this.bwForm = new BackgroundWorker();
this.bwForm.DoWork += new DoWorkEventHandler(this.BwForm_DoWork);
this.bwForm.ProgressChanged += new ProgressChangedEventHandler(this.BwForm_ProgressChanged);
this.bwForm.RunWorkerAsync();

...

void BwForm_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker bgw = sender as BackgroundWorker;
    // Your DoConvert code here
    // ...          
    int percent = 0;
    bgw.ReportProgress(percent);
    // ...
}

void BwForm_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    this.progressBar.Value = e.ProgressPercentage;
}

see here: http://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.aspx

EDIT

So I didn't understand your question, I thought it was about showing progress bar progression during work. If it's about result of work use e.Result in the BwForm_DoWork event. Add a new event handler for completed event and manage result:

this.bwForm.RunWorkerCompleted += new RunWorkerCompletedEventHandler(this.BwForm_RunWorkerCompleted);

...

private void BwForm_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    YourResultStruct result = e.Result as YourResultStruct;
    if (e.Error != null && result != null)
    {
        // Handle result here
    }
}
Vince
  • 1,036
  • 1
  • 10
  • 17
  • In which method is this code? DoWork, ProgressChanged, Completed or another one? You don't need to put lbFilesCount.Text = string.Format("{0} files", listView1.Items.Count); in the foreach. Verify than the Value is reset to 0 before adding 1 and set progress bar Max property properly. It would be better to just set Max to expected maximum and Value to list count. – Vince Jun 23 '11 at 10:19
  • Sorry for that, it's DoWork code. I want to show the processing. So I have to do this. The exeption do not throw in the foreach, but when foreach is starting. Thank you. – Justin Jun 23 '11 at 10:26
  • You can not do this in DoWork code. Do this in ProgressChanged (compute e.ProgressPercentage in DoWork) – Vince Jun 23 '11 at 10:29
  • I've know it. But........I have other operation in the DoWork. I did use ProgressChanged event, but didn't work to me. foreach(ListViewItem item in this.listView1.Items) cause exeption. `foreach (ListViewItem item in this.listView1.Items) {//... this.bwForm.ReportProgress(convertProgress); //lbFilesCount.Text = string.Format("{0} files", listView1.Items.Count); //progressBar1.Value++; //convertAmount++; }` – Justin Jun 23 '11 at 10:40
  • ReportProgress should be in the DoWork and WinForm User Control handling in the ProgressChanged... If it does not work i don't know. You can not handle your listview in the foreach in DoWork, DoWork must have no reference to WinForm, if you need parameter pass them through the background worker Run and get them back in Result. If you need a more complex structure than percent for progress (which i doubt) you need to use delegate and cross thread invoke. – Vince Jun 23 '11 at 12:27
  • 1
    @Justin, there's a property on BackgroundWorker called something like `WorkerReportsProgress` that you have to set to true, or it will never fire the `ProgressChanged` event. – Don Kirkby Jun 23 '11 at 20:21
8

Invoke the UI thread. For example:

void SetControlText(Control control, string text)
{
    if (control.InvokeRequired)
        control.Invoke(SetControlText(control, text));
    else
        control.Text = text;
}
foxy
  • 7,599
  • 2
  • 30
  • 34
  • As you suggested, I did that and worked fine. But...the form freezing again. What I want is when processing I can drag/drog the form fluency. – Justin Jun 17 '11 at 02:34
  • @Justin, are there any other blocking operations on your UI thread? – foxy Jun 17 '11 at 05:34
  • @freedompeace there are a label, a progress bar and a listview. I have to modify them. – Justin Jun 17 '11 at 06:54
  • 1
    @Justin - it seems to me,based on a comment you made to the original question, that you are doing things backwards. You should be performing the long-running non-UI operations in the DoWork method and update the UI progressbar / label / etc in the ProgressChanged callback. DoWork executes on the background thread and the ProgressChanged callback executes on the UI thread. – ScottTx Jun 22 '11 at 20:15
  • @ScottTx, you make sense. But I have to call a ListView to modify the progressBar and label, which caused InvalidOperationException. You seems to make me much closer to final solution.+1 – Justin Jun 23 '11 at 02:09
6

Avoid calling Application.DoEvents. Often times this leads to more problems than it solves. It is generally considered bad practice because there exists better alternatives which keep the UI pumping messages.

Avoid changing setting CheckForIllegalCrossThreadCalls = false. This does not fix anything. It only masks the problem. The problem is that you are attempting to access a UI element from a thread other than the main UI thread. If you disable CheckForIllegalCrossThreadCalls then you will no longer get the exception, but instead your application will fail unpredictably and spectacularly.

From within the DoWork event handler you will want to periodically call ReportProgress. From your Form you will want to subscribe to the ProgressChanged event. It will be safe to access UI elements from within the ProgressChanged event handler because it is automatically marshaled onto the UI thread.

Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
5

You should use Control.Invoke(). Also see this question for details.

Community
  • 1
  • 1
Zruty
  • 8,377
  • 1
  • 25
  • 31
5

Cleanest way is using BackGroundWorker as you did.

you just missed some points:

  1. you cannot access your form elements in DoWork event handler because that makes a cross thread method invokation, this should be done in ProgressChanged event handler.
  2. by default a BackGroundWorker will not allow to report progress, as well as in won't allow Cancelling the operation. When you add a BackGroundWorker to your code, you have to set the WorkerReportsProgress property of that BackGroundWorker to true if you want to call the ReportProgress method of your BackGroundWorker.
  3. In case you need to allow the user to cancel the operation as well, set WorkerSupportsCancellation to true and in your loop in the DoWork event handler check the property named CancellationPending in your BackGroundWorker

I hope I helped

Maziar Taheri
  • 2,288
  • 22
  • 27
4

There are some fundamental problems with the code you have shown. As other have mentioned, Application.DoEvents() will not do what you want from a background thread. Separate your slow background processing into a BackgroundWorker and update your progress bar in the UI thread. You are calling progressBar1.Value++ from a background thread, which is wrong.

Never call Control.CheckForIllegalCrossThreadCalls = false, that will only hide errors.

If you want to update a progress bar from a background thread, you will have to implement a ProgressChanged handler; you are not doing that.

If you need an example of implementing a BackgroundWorker, consult the article BackgroundWorker Threads from Code Project.

Dour High Arch
  • 21,513
  • 29
  • 75
  • 90
  • Thanks. But not fit my problems. I doubt that maybe I should use delegate and cross thread invoke to call listview. +1 – Justin Jun 24 '11 at 06:51
  • I use delegate and invoke. It works. But the form freezing again. Do you have any suggestion? Thank you. – Justin Jun 24 '11 at 08:05
  • The example is that of a WinForm with a label and a progress bar that shows results and that doesn't "freeze". How does that "not fit my problem"? – Dour High Arch Jun 24 '11 at 16:02
3

I would say writing thread safe applications is one of the more difficult things to understand and do properly. You really need to read up on it. If you learned C# from a book go back and see if there isn't a chapter on Multithreading. I learned from Andrew Troelsen's books (the latest was Pro C# 2005 and the .NET 2.0 Platform), he doesn't touch this topic until Chapter 14 (so a lot of people have quit reading by then). I come from an embedded programming background where concurrency and atomicity are also concerns so this is not a .NET or Windows specific issue.

A lot of those posts here are detailing the mechanics of dealing with threads via the facilities supplied in .NET. All valuable advice and things you have to learn but it will really help if you become more familiar with the "theory" first. Take the cross-threading issue. What's really going on is the UI thread has some built-in sophistication that checks to see if you are modifying a control from a different thread. The designers at MS realized this was an easy mistake to make so the built in protection against it. Why is it dangerous? That requires you understand what an atomic operation is. Since the control's "state" can't be changed in an atomic operation, one thread could start to change the control and then another thread could become active, thus leaving the control in a partially modified state. Now if you tell the UI I don't give a crap just let my thread modify the control, it might work. However, when it doesn't work, you'll have a very hard to find bug. You'll have made your code much less maintainable and the programmers that follow you will curse your name.

So the UI is sophisticated and checks for you, the classes and threads you are writing don't. You have to understand what is atomic in your classes and threads. You might be causing the "freeze" in your threads. Most commonly an app freezing is the result of a deadlock condition. In this case "freeze" means the UI never becomes responsive again. At any rate this is getting too long but I think at a minimum you should be familiar with the use of "lock" and probably also Monitor, interlocked, semaphore and mutex.

Just to give you an idea: (from Troelsen's book)

intVal++; //This is not thread safe 
int newVal = Interlocked.Increment(ref intVal); //This is thread safe

.NET also provides a [Synchronization] attribute. This can make writing a thread safe class easy, but you do pay a price in efficiency for using it. Well I'm just hoping to give you some idea of the complexities involved and motivate you to go do some further reading.

Tod
  • 8,192
  • 5
  • 52
  • 93
2

Do like this, create a new thread

         Thread loginThread = new Thread(new ThreadStart(DoWork));

         loginThread.Start();

Inside the ThreadStart(), pass the method you want to execute. If inside this method you want to change somes controls properties then create a delegate and point it to a method inside which you will be writing the controls changed properties,

         public delegate void DoWorkDelegate(ChangeControlsProperties);         

and invoke the controls properties do like this, declare a method and inside it define the controls new porperties

         public void UpdateForm()
         {
             // change controls properties over here
         }

then point a delegate to the method, in this way,

         InvokeUIControlDelegate invokeDelegate = new InvokeUIControlDelegate(UpdateForm);

then when you want to change the properties at any place just call this,

         this.Invoke(invokeDelegate);

Hope this code snippet helps you ! :)

Bibhu
  • 4,053
  • 4
  • 33
  • 63
1

To add onto freedompeace's solution, he was right. This solution is thread-safe and that's exactly what you want. You just need to use BeginInvoke instead of Invoke.

void SetControlText(Control control, string text)
{
    control.BeginInvoke(
        new MethodInvoker(() =>
        {
            control.Text = text;
        })
    );
}

The above fix is the simplest and cleanest.

Hope this helps. :)

Nahydrin
  • 13,197
  • 12
  • 59
  • 101
  • The form is still freezing. My program is much more complicated. – Justin Jun 24 '11 at 08:42
  • Thanks. I did do it. But the form is still freezing. – Justin Jun 24 '11 at 08:48
  • 1
    call `SetControlText(control, string)` from within a thread. Like so: `new Thread(() => { SetControlText(control, string); }) { IsBackground = true }.Start();`. Feel free to edit it so you can end the thread. This is still a safe way to call the method, and it **should** unfreeze your form. – Nahydrin Jun 24 '11 at 08:51