Timers are cool (was: C# Multithread conundrum)

This is the first time I'm trying concurrency in c#. I've got several UserControls, each of which has a public method called animate(int value).
The control is basically an animated progress bar which, given a new value, creates a new thread in which it animates the progress bar  (slides to the right or left) until it reaches the given value.
Now, I have 4 or 5 instances of this control on my form, and I animate each of them through a for loop, with a random value. The problem is the application hangs, and it seems to be random, somethimes immediately after animation starts or even after half an hour of the progress bars animating  continuously. No exceptions are thrown, just the usual End Program dialog followed by the Error Report dialog when I try and exit.
How do I start debugging the program when I'm not given any info on where it breaks?

What you need to do is have the code that executes the progress bar animation calculations (like how much to step next) in the other thread.  Have that code throw an event when the value needs to be updated.  At this point, in the event handler have a conditional if statement that determines if it is on the UI thread or not.  If it is - updated the progress bar directly.  If it isn't, then you need to call Control.BeginInvoke() instead as this is a thread safe version.

Let me know if futher explanation is needed.  But basically in Windows it is dangerous to update the UI from multiple threads.

EDIT - or you could skip the whole event handler thing and just call Control.BeginInvoke() directly from your animation code.

hmm, I'm not sure I understand, right now I have the animate(value) method cread a new thread that runs another private method called animateControl().
So this is basically what's in my user control:

protected override void  OnPaint (PaintEventArgs e)
{
    //draw progress bar
}

public void animate (int value)
{
    ...
    // Check if value is valid and stop the thread if it is already running
    this.newval=value;
    ...
     t1 = new Thread(new ThreadStart(animateControl));
    ...
}

public void animateControl()
{
    ...
    // Calculate next step in animation
    // repaint the control, in a very crude manner, I have no idea if this is the proper way to do it, but it seems to work!
    repaint();
    // pause for several milliseconds
    // repeat until newval is reached
    ...
}

private void repaint ()
{
    GC.Collect();
    try
    {
        drawing=true;
        e = new PaintEventArgs(this.CreateGraphics(), this.ClientRectangle);
        OnPaint(e);
    }
    catch(Exception edp)
    {
        Console.WriteLine("In the repaint method: \n"+ edp.ToString());
    }
    finally
    {
        e.Graphics.Dispose();
        drawing=false;
    }
}

Where should I call BeginInvoke, and what delegate should I give it?

What does your

animate(int value)

function look like?

I apologize, it is hard to explain this in a forum post.

Here is a great article on the subject -

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnforms/html/winforms06112002.asp

For some technical stuff -

http://weblogs.asp.net/justin_rogers/articles/126345.aspx

For more info you might want to check out -

http://msdn.microsoft.com/msdnmag/issues/03/02/Multithreading/default.aspx

Specifically the part on Control.Invoke/Control.BeginInvoke

nightski, Thanks for the articles, I shall read them over, hopefully they will help, most other articles are way over my head.
Minh, here it is in all its glory, very messy.
public void animate (int newval)
{
    if (newval<=max &&newval>=min)
    {
        if(boolAnimate)
        {    
           
            try
            {
                if(t1.ThreadState==ThreadState.Stopped|t1.ThreadState==ThreadState.Unstarted)
                {
                    this.newval=newval;
                    t1 = new Thread(new ThreadStart(animateControl));
                    t1.Name="ProgressBar";
                    t1.Start();
                    while(!t1.IsAlive);
                    if (debugout!=null) debugout.Items.Insert(0,t1.Name+": THREAD START");
                }
                else if (t1.ThreadState==ThreadState.Running)
                {
                    while(drawing);
                    if(t1.IsAlive)
                    {
                        t1.Abort();
                        t1.Join();
                    }
                    if (debugout!=null) debugout.Items.Insert(0,t1.Name+": THREAD ABORT");
                    this.newval=newval;
                    t1 = new Thread(new ThreadStart(animateControl));
                    t1.Name="ProgressBar";
                    t1.Start();
                    while(!t1.IsAlive);
                    if (debugout!=null) debugout.Items.Insert(0,t1.Name+": THEARD START");
                }
                else
                {
                    //    MessageBox.Show(t1.ThreadState.ToString());
                }
            }
            catch(Exception ex)
            {
                if (debugout!=null) debugout.Items.Insert(0,t1.Name+": In the setMeterValue method: \n"+ex.ToString());
            }
        }
        else
        {
            this.val=newval;
            repaint();
        }
    }
}

You cannot update any UI control unless you are in the UI thread.  How do you get into the UI thread?

Like this:

public delegate void FunctionNameDelgate(paramtype1 paramname1, paramtype2 paramname2, etc...);

public void FunctionName(paramtype1 paramname1, paramtype2 paramname2, etc...
{
   if(this.InvokeRequired==true)
   {
      FunctionNameDelegate fnd = new FunctionNameDelegate(FunctionName);
      this.Invoke(fnd,new object[]{paramname1, paramname2, etc.});
   }
   else
   {
      // do UI based stuff here
   }
}

not entirely true.
value based types like strings and numbers can be updated. things like .text = blah.
when you need to use invoke is when you have to creare an object such as a treenode

Er...kinda.  As long as the thing you are updating has nothing to do with the UI, you are safe.  The problem is...given his description... is that he's actually trying to update the UI with multiple threads.

On the plus side, .Net 2.0 will throw an exception if you even attempt to do this.  1.1 will let you try in most cases and you'll end up with a locked up UI...like the original poster.

trolane wrote:

not entirely true.
value based types like strings and numbers can be updated. things like .text = blah.
when you need to use invoke is when you have to creare an object such as a treenode

1) Strings are not value types!!

http://odetocode.com/Blogs/scott/archive/2005/07/21/1961.aspx

2) It's well documented you should never touch a UI control from a thread other than the thread that created the control. It might work for specific properties on specific controls, but that is only because you got lucky. The implementation could change in a future release and the program go boom. Controls are inherently tied to a specific thread.

Alright people thanks for all your help, here's the new code, please review it and tell me what I'm doing wrong this time:


delegate void animateControlDelegate (double newval);
animateControlDelegate animc;
PaintEventArgs e = null;

public AnimatedProgressBar()
{
    ...
    animc = new animateControlDelegate(animateControl);
    ...
}

protected override void  OnPaint (PaintEventArgs e)
{
    //draw progress bar
}

public void animate (int newval)
{
    if (newval<=max &&newval>=min)
    {
        if(boolAnimate)
        {
            animc.BeginInvoke(newval,null,null);
        }
    }
}

public void animateControl()
{
    ...
    // Calculate next step in animation
    // repaint the control, I think this is better than my last attempt, again; it seems to work!
    repaint();
    // pause for several milliseconds
    // repeat until newval is reached
    ...
}

private void repaint ()
{
    GC.Collect();
    try
    {
        e = new PaintEventArgs(this.CreateGraphics(), this.ClientRectangle);
        this.InvokePaint(this,e);
    }
    catch(Exception edp)
    {
        Console.WriteLine("In the repaint method: \n"+ edp.ToString());
    }
    finally
    {
        e.Graphics.Dispose();
    }
}



Also, I need a way to stop the animation thread if the animate(int newval) method is called in the middle of the previous animation. Is that possible?

Delete the repaint method and just call Invalidate. If you only need animation, don't use threads -- that is not a good use. Instead use a Windows.Forms.Timer. Threads are better for background tasks.

If you are using threads, you can always communicate between threads with instance fields. I don't think you meant that you wanted to "stop" the thread -- instead you want to avoid another BeginInvoke? If that is the case, have a field, lets say called painting,
and set that field at the beginning of the OnPaint, clearing it at the end:

painting = true;
try
{
// ... rest of code
}
finally { painting = false; }

This way your animation thread can tell if you are in OnPaint, mostly. There is no need for a lock since you are only modifying from one thread.

A property is just a pair of get/set methods in MSIL.  If you call a method from multiple threads then won't it be runnning on all of those threads and hence require a lock?

Or is there magic somewhere with properties?

Hmm, Don't timers run in the main thread?, I think I'll continues using threads anyway.
No, I don't want to avoid another begin invoke, say I call the animate method with a value of 97, the progress bar will start animating (starting from zero) towards 97, but in the middle of the animation, say when the progressbar shows 54, I call the animate method again , with a value of 34, I want the animation to stop where it is and go backwards.
So how do I stop the animation, i.e the thread created with BeginInvoke in the animate(int newval) method?
I guess I shouldn't call the control a progress bar, since it can go backwards, it's more like a guage.

BrianPeiris wrote:

Hmm, Don't timers run in the main thread?

There's nothing wrong with that. When the timer fires determine whether you need to redraw and invalidate if you do.

Also, why do you have GC.Collect() in your code above? The general rule on calling GC.Collect() is "Don't."

Hmmmm...this is an interesting idea, I like it.

One way to do this is to have your progress bar control itself decide what to do.  You'd have a public accessor that you can set to whatever value.  Then, you'll have a private thread (or timer) that will periodically read this value and decide if it should be increasing or decreasing.

If you use a timer or thread, you'll need to keep a private handle to them so that when you are destroyed, you can be sure to get rid of them, but other than that, it should be easy.

Well! many thanks to Frank Hileman for suggesting it, I have switched my code to timers-only. This has made things much easier, I should have done this from the start
I would have, if I didn't think it would prevent me from animating serveral controls simultaneously. But now I have learned my lesson, animation == timers !
Now I have another question.
I have my gauges on a my main form and I am using a Timer to send them random values every 2 to 3 seconds, just as a demo for show what the animation looks like.
However, if the form looses focus the gauges finish their last animation cycle and the program stops, I.E. the Timer on the main form stops sending random values to the gauges.
I'd like the timer to keep going even when the app is out of focus, can anyone help?

P.S. I might be posting the full code in a while, I might even put it in the sandbox If I figure out how.

I have never seen that behavior. When my forms lose focus Timers keep going fine.