6

I'm trying to use threads and prevent the program from freezing while the thread is busy. It should show the progress (writing of 0's / 1's) and not just show the result after its done, freezing the form in the meanwhile.

In the current program I'm trying to write to a textbox, and actually see constant progress, and the form can't be affected by the tasks of the other thread.

What I have now is I can write to a textbox with a thread using invoke, but It only shows the result (Form freezes while thread is busy), and the form freezes.

Form image:

enter image description here

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Threading;

namespace MultiThreading
{
public partial class MultiThreading : Form
{
    public MultiThreading()
    {
        InitializeComponent();
    }

    Thread writeOne, writeTwo;

    private void writeText(TextBox textBox, string text)
    {
        if (textBox.InvokeRequired)
        {
            textBox.BeginInvoke((MethodInvoker)delegate()
            {
                for (int i = 0; i < 500; i++)
                {
                    textBox.Text += text;
                }
            });
        }
        else
        {
            for (int i = 0; i < 500; i++)
            {
                textBox.Text += text;
            }
        }
    }
    private void btnWrite1_Click(object sender, EventArgs e)
    {
        writeOne = new Thread(() => writeText(txtOutput1, "0"));
        writeOne.Start();
    }

    private void btnWrite2_Click(object sender, EventArgs e)
    {
        writeTwo = new Thread(() => writeText(txtOutput2, "1"));
        writeTwo.Start();
    }

    private void btnClear1_Click(object sender, EventArgs e)
    {
        txtOutput1.Clear();
    }

    private void btnClear2_Click(object sender, EventArgs e)
    {
        txtOutput2.Clear();
    }

    private void btnWriteBoth_Click(object sender, EventArgs e)
    {
        writeOne = new Thread(() => writeText(txtOutput1, "0"));
        writeTwo = new Thread(() => writeText(txtOutput2, "1"));

        writeOne.Start();
        writeTwo.Start();
    }

    private void btnClearBoth_Click(object sender, EventArgs e)
    {
        txtOutput1.Clear();
        txtOutput2.Clear();
    }
}

}

EDIT:

Btw for anyone wondering, I'm new to multithreading and I'm just trying to write a small program to understand the best way to do this.

I understand that my previous invoke didn't realy help because I still wasn't giving the form a chance to update, so its getting there.

Ok so running 1 thread like this works, but still running multiple threads together, won't update the form till after the thread is done.
I've added a thread.sleep() so I can try and clear while writing, to see if I can still use the form.

When writing to 1 textbox I can still clear the screen while writing.
But once I use 2 threads, I can't use the form anymore till the thread completes, and gives the output.

private void writeText(TextBox textBox, string text)
    {
        for (int i = 0; i < 500; i++)
        {
            Invoke(new MethodInvoker(() =>
            {
                textBox.Text += text;
                Thread.Sleep(2);
            }));
        }

    }

(If I'm totally wrong on this I don't mind having to read through some examples/threads, I'm still trying to see what is the best way to do this, besides a backgroundworker)

EDIT 2:

I've reduced the number of invokes by reducing the amount to write, but to increase delay giving the same effect of constant writing, just reducing the load.

private void writeText(TextBox textBox, string text)
    {
        for (int i = 0; i < 500; i++)
        {
            Invoke(new MethodInvoker(() =>
            {
                textBox.Text += text;
                Thread.Sleep(2);
            }));
        }

    }

EDIT 3:

Sumeet's example works using

Application.DoEvents();

(notice the s, .DoEvent doesn't work, typo probably :P), writing multiple strings simultaneously & having them show the progress and not just the result.

So Code update again :)

*Using a new button to create 5 threads that write a random number to both textboxes

private void writeText(TextBox textBox, string text)
    {
        for (int i = 0; i < 57; i++)
        {
            Invoke(new MethodInvoker(() =>
            {
                textBox.Text += text;
                Thread.Sleep(5);
                Application.DoEvents();
            }));
        }

    }
private void btnNewThread_Click(object sender, EventArgs e)
    {
        Random random = new Random();
        int[] randomNumber = new int[5];
        for (int i = 0; i < 5; i++)
        {
            randomNumber[i] = random.Next(2, 9);
            new Thread(() => writeText(txtOutput1, randomNumber[i-1].ToString())).Start();
            new Thread(() => writeText(txtOutput2, randomNumber[i-1].ToString())).Start();
        }
    }
Random IT Guy
  • 625
  • 1
  • 8
  • 16
  • 3
    Consider using [`BackgroundWorker`](http://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.aspx) if you have a long-running process running in the background, and the UI needs to stay responsive. – Robert Harvey Jan 28 '13 at 19:20
  • @RobertHarvey That wouldn't really help this particular example. – Servy Jan 28 '13 at 19:22
  • 1
    @Servy This particular example seems... broken. And contrived. There's a well-worn path for this kind of thing. – Robert Harvey Jan 28 '13 at 19:23
  • @RobertHarvey I disagree. It's representative of a different type of problem; the long running *UI* operation, in which you need to perform a significant amount of UI updates, rather than a long running non-UI task that just happens to update the UI either to update progress or when completed. It's a valid use case that the backgroundworker class isn't designed to handle. – Servy Jan 28 '13 at 19:27
  • 1
    I believe you're both correct. `BackgroundWorker` isn't suitable for this task, as Servy points out. The quickest fix may be to move the `BeginInvoke` call within the `for` loop. However, this example is contrived; I can't think of a single real-world situation where I would consider this a "valid" use case. It's almost surely indicative of a need to rethink your design. – JosephHirn Jan 28 '13 at 19:49
  • Add a Application.DoEvents() ! Its so simple isn't it ? – sumeet Jan 28 '13 at 20:06
  • there are bunch of valid use cases for that. you just haven't seen them – Boppity Bop Jan 31 '13 at 03:57

6 Answers6

7

This solution works ! Have checked it.

The problem is you keep telling the UI thread to change the Text, but never letting it have time to show you the updated text. To make your UI show the changed text, add the Application.DoEvents line like this :

textBox.Text += text;
Application.DoEvents();

p.s. : Remove the else block of your If / Else loop, it is redundant, and also as pointed by others there is not any use of creating those 2 Threads as all they are doing is post the message on the UI Thread itself.

guyzyl
  • 387
  • 5
  • 18
sumeet
  • 318
  • 4
  • 11
  • This does indeed work, now I've got no delay from the threads, even running multiple threads ;) – Random IT Guy Jan 28 '13 at 20:34
  • 2
    I would be cautious with `DoEvents()`. Use it sparingly; I've seen folks deadlock programs using it indiscriminately. If you're threading properly, you should seldom have the need for `DoEvents()`. – Robert Harvey Jan 29 '13 at 00:07
  • 1
    If you ask me, you should **never** use `Application.DoEvents()`. It's... manageable in this situation, but using it in place of proper multithreading is not a habit you should get into. – JosephHirn Jan 29 '13 at 19:06
  • This does NOT make the app multi-threaded, it just pushes an UI update . Overall UX will still be choppy at best - try moving the form while the process is going on – Sten Petrov Jan 31 '13 at 14:42
  • IMHO use Application.DoEvents() as freely as you like, not only its fully safe (If you are having a deadlock blame your threading not Application.DoEvents and you would have deadlocked anyways.), but also its much easier for a quick safe windows form capable of pseudo threading without actually spawning threads, and hence without worrying about getting caught in a deadlock. – sumeet Feb 23 '13 at 17:54
5

You're still performing a single-threaded task, just re-launching it on the UI thread if needed.

for (int i = 0; i < 500; i++){
    string text = ""+i;
    textBox.BeginInvoke((MethodInvoker)delegate()
            {
                textBox.Text += text;
            });
}
Sten Petrov
  • 10,943
  • 1
  • 41
  • 61
4

The problem is that you're starting a new thread, and then that new thread is doing nothing except adding one new task for the UI thread to process that does a lot of work. To keep your form responsive you need to have time where the UI thread is doing nothing, or at least not spending a significant amount of time doing any one task.

To keep the form responsive we need to have lots of little BeginInvoke (or Invoke) calls.

private void writeText(TextBox textBox, string text)
{
    for (int i = 0; i < 500; i++)
    {
        Invoke(new MethodInvoker(() =>
        {
            textBox.Text += text;
        }));
    }
}

By having lots of little invoke calls it allows things like paint events, mouse move/click events, etc. to be handled in the middle of your operations. Also note that I removed the InvokeRequired call. We know that this method will be called from a non-UI thread, so there's no need for it.

Servy
  • 202,030
  • 26
  • 332
  • 449
2

You're defeating the purpose of using threads.

All your thread does is tell the UI thread to execute some code using BeginInvoke().

All of the actual work happens on the UI thread.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
0

Either you're doing data processing or you're just trying to animate the UI.

For data processing you should do all the heavy lifting on a background thread and only update the UI occasionally. In your example a TextBox is particularly troublesome in this regard, as you're adding data to the underlying data model several hundred times and the UI element (a TextBox) takes longer to render each time. You must be careful about how often to update the UI so that processing for UI updates does not overwhelm data model updates. TextBoxes are nasty like that.

In the example below, a flag set during the paint event ensures that additional UI updates aren't queued until the TextBox has finished painting the last update:

string str = string.Empty;
public void DoStuff()
{
    System.Threading.ThreadPool.QueueUserWorkItem(WorkerThread);
}

void WorkerThread(object unused)
{
    for (int i = 0; i < 1000; i++)
    {
        str += "0";
        if (updatedUI)
        {
            updatedUI = false;
            BeginInvoke(new Action<string>(UpdateUI), str);
        }
    }
    BeginInvoke(new Action<string>(UpdateUI), str);
}

private volatile bool updatedUI = true;
void textbox1_Paint(object sender, PaintEventArgs e) // event hooked up in Form constructor
{
    updatedUI = true;
}

void UpdateUI(string str)
{
    textBox1.Text = str;
}

On the other hand if UI animation is your goal then you probably ought to be using something other than a TextBox. It's just not designed to handle updates so frequently. There might be some optimizations to text rendering you could make for your specific use case.

RogerN
  • 3,761
  • 11
  • 18
  • Right now I'm just trying to have a good overview of multithreading. I'm using the textbox because its easy to see that you're constantly writing to the textbox, and that I can still use the form. Right now I'm just trying to have constant writing to the textbox, where you can see the progress, and also do other tasks at the same time, and not starting a thread, seeing only the result and not beeing able to use the form. – Random IT Guy Jan 28 '13 at 20:26
0

You must never use a string in high volume applications. UI or not. Multi-threading or not.

You should use StringBuilder to accumulate the string. and then assign

tb.Text = sb.ToString();
Boppity Bop
  • 9,613
  • 13
  • 72
  • 151