views:

249

answers:

6

As a training exercise, I am creating a counter that runs in a background thread, in a .NET 3.5 WPF app. The idea is that the counter gets incremented and every few seconds the total is output to the screen.

As it is a training exercise, the foreground thread doesn't really do anything, but imagine that it does!

There is a start and stop button to start and stop the counter.

The problem is that I want the foreground thread to poll the total every few seconds and update it in the text box on the form. The entire app goes 'Not Responding' when I try this. Currently I have only got it to update the count a fixed time after it starts it.

I'll dump all the code in here, so that you get the whole picture:

 public partial class Window1 : Window
    {
        public delegate void FinishIncrementing(int currentCount);
        private int reportedCount;
        private Thread workThread;

        public Window1()
        {
            InitializeComponent();
        }

        #region events

        private void Window_Loaded(object sender, RoutedEventArgs e)
        {
            txtCurrentCount.Text = "0";
            reportedCount = 0;
            InitialiseThread();
        }

        private void btnStart_Click(object sender, RoutedEventArgs e)
        {
            workThread.Start();

            // Do some foreground thread stuff

            Thread.Sleep(200); // I want to put this in a loop to report periodically
            txtCurrentCount.Text = reportedCount.ToString();
        }

        private void btnStop_Click(object sender, RoutedEventArgs e)
        {
            workThread.Abort();
            txtCurrentCount.Text = reportedCount.ToString();
            InitialiseThread();
        }

        #endregion

        private void InitialiseThread()
        {
            WorkObject obj = new WorkObject(ReportIncrement, reportedCount);
            workThread = new Thread(obj.StartProcessing);
        }

        private void ReportIncrement(int currentCount)
        {
            reportedCount = currentCount;
        } 
    }

    public class WorkObject
    {
        private Window1.FinishIncrementing _callback;
        private int _currentCount;

        public WorkObject(Window1.FinishIncrementing callback, int count)
        {
            _callback = callback;
            _currentCount = count;
        }

        public void StartProcessing(object ThreadState)
        {
            for (int i = 0; i < 100000; i++)
            {
                _currentCount++;

                if (_callback != null)
                {
                    _callback(_currentCount);
                    Thread.Sleep(100);
                }
            }
        } 
    }

My question is: how do I get it to report periodically, and why does it fail when I try to put the reporting in a for loop?

Edit: Should have mentioned that my training exercise was to familiarise myself with the old-school method of multithreading, so I cannot use BackgroundWorker!

+1  A: 

You could use a BackgroundWorker object. It supports reporting progress through an event that you subscribe to. That way you don't have to a thread polling for the status of the background task. Its really easy to use and should be trivial to implement in your application. It also plays nice with the WPF dispatcher.

As for why you get the "Not Responding" message in your app. Its the Thread.Sleep() you have in your button press handler. That is telling the UI thread to stop, which makes the application stop processing messages, which triggers the "Not Responding" message.

Navaar
+1 - when you're doing threading in WinForms, BackgroundWorker is the first place to start.
Ian Kemp
A: 

I would try something a bit different. Instead of the calling program polling the worker thread, have the worker thread raise an event periodically to report back to the calling thread. For example, in your counting loop you can do a check to see if the current count is evenly divisible by 1000 (or whatever) and if it is, raise an event. Your calling program can catch the event and update the display.

TLiebe
+2  A: 

It goes to "no responding" because you're using a loop.

Substitute the loop for a timer and all will work again :P

The thing is you're using a loop in the main thread so it never exits the click event and keep the main thread busy.

As the main thread is busy in the loop it cannot handle windows messages (which are used among other thing to handle input and drawing of the window). Whenever Windows detects the application is not handling messages it reports it as "No responding".

Jorge Córdoba
Why does the loop cause this to happen? The loop has a sleep in it, so it wasn't constantly updating.
Fiona Holder
Thread.Sleep puts the thread to sleep so the windows message handler does not executes and therefore you get a Not Responding
Jorge Córdoba
+2  A: 

What's happening when you put your polling in a loop is that you're never letting control get back to the Dispatcher which does all of the Windows message handling (messages for drawing, move, minimize / maximize, keyboard / mouse input, etc...) for the application. This means that your application is not responding to any of the messages that Windows sends it so it is labeld as "Not Responding".

There are a few ways you could restructure your code to get the behaviour you want. As others have mentioned, you could create a timer on the GUI thread that updates the text box. You would do this by creating a DispatcherTimer and setting the Tick and Interval properties.

You could also make the GUI update every time ReportIncrement is called by calling Control.Invoke and passing it a function that updates the text box. For example:

    private void ReportIncrement(int currentCount) 
    { 
        reportedCount = currentCount; 
        txtCurrentCount.Invoke(
            new Action(()=>txtCurrentCount.Text = reportedCount.ToString())
        );
    }
Jon Norton
+1 this is probably the most generally useful approach. A timer would tick at regular intervals even if nothing had changed, whereas `Invoke` as used here would only update the GUI when you specifically ask it to.
Daniel Earwicker
+3  A: 

Every form in a windows application has a message loop that looks something like this:

while (NotTimeToCloseForm)
{
    Message msg = GetMessageFromWindows();
    WndProc(msg);
}

This applies to all windows applications not just .Net ones. WPF provides a default implementation of WinProc that goes something like this:

void WndProc(Message msg)
{
    switch (msg.Type)
    {
        case WM_LBUTTONDOWN:
            RaiseButtonClickEvent(new ButtonClickEventArgs(msg));
            break;

        case WM_PAINT:
            UpdateTheFormsDisplay(new PaintEventArgs(msg));
            break;

        case WM_xxx
            //...
            break;

        default:
            MakeWindowsDealWithMessage(msg);
    }
}

As you can see this means that a window can only process one message/event at a time. As you are sleeping in your Button Click event the message loop is being held up and the application stops responding (getting and processing messages).

If you use a DispatcherTimer it asks windows to periodically stick a WM_TIMER (Tick) message in the windows message queue. This allows your application to process other messages while still being able to do something at regular intervals without using another thread.

For more detail see MSDN "About Messages and Message Queues"

Martin Brown
Well you learn something new every day, thanks
Fiona Holder
+1  A: 

While Martin Brown's example does work, your application still receives it's WM_TICK message in the same queue as all other windows messages, therefore making it synchronous.

As an alternitive, here is an example of "true multi threading", where your process spawns another thread, and the thread calls an update delegate to update the value of a label.

public partial class Window1 : Window
{
    Thread _worker;

    public Window1()
    {
        InitializeComponent();
        _worker = new Thread(new ThreadStart(this.DoWork));
        _worker.Start();
    }

    private void DoWork()
    {
        WorkObject tWorker = new WorkObject((MyUpdateDelegate)delegate (int count) { this.UpdateCount(count); } , 0);
        tWorker.StartProcessing();
    }

    public delegate void MyUpdateDelegate (int count);

    private void UpdateCount(int currentCount)
    {
        if (Dispatcher.CheckAccess())
        {
            lblcounter.Content = currentCount;
        }
        else
        {
            lblcounter.Dispatcher.Invoke((MyUpdateDelegate)delegate (int count)
            {
                this.UpdateCount(count); 
            }, currentCount);
        }
    }

    protected override void OnClosing(System.ComponentModel.CancelEventArgs e)
    {
        if (_worker != null)
        {
            _worker.Abort();
        }
        base.OnClosing(e);
    }
}

public class WorkObject
{
    private Window1.MyUpdateDelegate _callback;
    private int _currentCount;

    public WorkObject(Window1.MyUpdateDelegate callback, int count)
    {
        _callback = callback;
        _currentCount = count;
    }

    public void StartProcessing()
    {
        for (int i = 0; i < 100000; i++)
        {
            _currentCount++;

            if (_callback != null)
            {
                _callback(_currentCount);
                Thread.Sleep(100);
            }
        }
    }
}

This is what I would consider more "old school" threading, of course that depends on your definition of "old school".

EDIT: I might point out that the Martin's and my example functionally are the same.

Darien Ford