Resolved Detecting the end of a thread

[cbreemer]

I am struggling with something that I feel should be simple... once you know how.
I have a Windows Form that implements a rudimentary print queue monitor. It starts a thread which goes into an endless loop, each second checking the local printer queue and calling Invoke(new MethodInvoker(delegate ()... to update the UI. It works beautifully, but now my extra requirement is that as soon the number of jobs reaches zero, the form terminates itself. A fairly normal thing to want, I would hope ?

Trying to close the form from within the delegate function in the thread body does not seem like a good idea. No matter what I try, this hangs the application.
I guess it would have to be the main thread terminating the form. But how does the main thread know that the worker thread has ended ? I don't want to be polling the thread's isAlive flag as that would make the form unresponsive. I read about Thread.Abort() being unreliable, and I would not know how the main thread could catch the ThreadAbortException anyway. Is there some event that the thread can raise which then calls an event handler in the form ? I googled around but the dozens of different ideas and suggestions only serve to confound me I hope someone here has been there and done that.

 

[cbreemer]

+1 to @JohnH 's point about just letting the other thread close the form. Here's some quick and dirty code that shows the feasibility of doing it.

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

namespace WinForms
{
    class MainForm : Form
    {
        MainForm()
        {
            Text = "Main";
            Size = new Size(800, 600);

            var button = new Button() { Text = "Start Countdown" };
            button.Click += (o, e) => { new OtherForm().Show(this); };
            Controls.Add(button);

            CenterToScreen();
        }

        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(new MainForm());
        }
    }

    class OtherForm : Form
    {
        Label _lblCountdown = new Label() { Text = "Starting...", Dock = DockStyle.Fill };

        public OtherForm()
        {
            Text = "Other";
            Size = new Size(400, 300);
            Controls.Add(_lblCountdown);
            Shown += (o, e) => new Thread(Countdown).Start();
            CenterToParent();
        }

        void Countdown()
        {
            for (int i = 10; i >= 0; i--)
            {
                Thread.Sleep(1000);
                _lblCountdown.Invoke((MethodInvoker) delegate
                {
                    _lblCountdown.Text = $"{i}: {DateTime.Now}";
                });
            }
            Invoke((MethodInvoker) delegate { Close(); });
        }
    }
}

Well that's interesting. It certainly did not work for me, just froze the application with both forms becoming unresponsive. But I was doing quite a lot of stuff inside a delegate function, whereas you use a delegate only for the statements that need it. Perhaps that makes the difference. I'll pay around with your code, as it has some other aspects that are useful to me. Thanks again ! I'll let you know how it worked out in my case.

 

[jmcilhinney]

You could use a BackgroundWorker for this. Call RunWorkerAsync and then do your polling in an infinite loop in the DoWork event handler. When your queue is empty, break out of the loop and that event handler will complete. You can then call Close in the RunWorkerCompleted event handler. You can also use the in-built cancellation mechanism to enable you to stop polling and close the form.

 

[cbreemer]

You could use a BackgroundWorker for this. Call RunWorkerAsync and then do your polling in an infinite loop in the DoWork event handler. When your queue is empty, break out of the loop and that event handler will complete. You can then call Close in the RunWorkerCompleted event handler. You can also use the in-built cancellation mechanism to enable you to stop polling and close the form.

Thanks !
I tried BackgroundWorker but IIRC either could not get it to work or found it too complicated As I did not see any advantage over simply using a thread, I ditched it. But I could well be wrong, and really need to give it another try.
Hm, there seem to be any number of ways to solve this particular "problem"

 

[jmcilhinney]

I tried BackgroundWorker but IIRC either could not get it to work or found it too complicated As I did not see any advantage over simply using a thread, I ditched it. But I could well be wrong, and really need to give it another try.

A BackgroundWorker would be simpler than most other options so I suspect that you did do something wrong. If you want to give it a go and post back if you have an issue, I'm sure we can straighten it out.

 

[Skydiver]

But I was doing quite a lot of stuff inside a delegate function, whereas you use a delegate only for the statements that need it. Perhaps that makes the difference.

It does. All the work within the delegate is done back on the UI thread. That negates all your attempts to try to do work on another thread so as not to burden the UI thread.

 

[Skydiver]

And here's the equivalent of post #15 using a BackgroundWorker:

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

namespace WinFormsCore
{
    class MainForm : Form
    {
        MainForm()
        {
            Text = "Main";
            Size = new Size(800, 600);

            var button = new Button() { Text = "Start Countdown" };
            button.Click += (o, e) => { new OtherForm().Show(this); };
            Controls.Add(button);

            CenterToScreen();
        }

        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(new MainForm());
        }
    }

    class OtherForm : Form
    {
        Label _lblCountdown = new Label() { Text = "Starting...", Dock = DockStyle.Fill };

        public OtherForm()
        {
            Text = "Other";
            Size = new Size(400, 300);
            Controls.Add(_lblCountdown);

            var backgroundWorker = new BackgroundWorker() { WorkerReportsProgress = true };
            backgroundWorker.DoWork += BackgroundWorker_DoWork;
            backgroundWorker.ProgressChanged += BackgroundWorker_ProgressChanged;
            backgroundWorker.RunWorkerCompleted += (o, e) => Close();
            backgroundWorker.RunWorkerAsync();

            CenterToParent();
        }

        private void BackgroundWorker_DoWork(object sender, DoWorkEventArgs e)
        {
            var backgroundWorker = (BackgroundWorker)sender;
            for (int i = 10; i >= 0; i--)
            {
                Thread.Sleep(1000);
                backgroundWorker.ReportProgress(0, i);
            }
        }

        private void BackgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
        {
            _lblCountdown.Text = $"{e.UserState}: {DateTime.Now}";
        }
    }
}

 

[cbreemer]

It does. All the work within the delegate is done back on the UI thread. That negates all your attempts to try to do work on another thread so as not to burden the UI thread.

Yes ! I guess I had not really thought that through well enough.

 

[cbreemer]

And here's the equivalent of post #15 using a BackgroundWorker:

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

namespace WinFormsCore
{
    class MainForm : Form
    {
        MainForm()
        {
            Text = "Main";
            Size = new Size(800, 600);

            var button = new Button() { Text = "Start Countdown" };
            button.Click += (o, e) => { new OtherForm().Show(this); };
            Controls.Add(button);

            CenterToScreen();
        }

        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(new MainForm());
        }
    }

    class OtherForm : Form
    {
        Label _lblCountdown = new Label() { Text = "Starting...", Dock = DockStyle.Fill };

        public OtherForm()
        {
            Text = "Other";
            Size = new Size(400, 300);
            Controls.Add(_lblCountdown);

            var backgroundWorker = new BackgroundWorker() { WorkerReportsProgress = true };
            backgroundWorker.DoWork += BackgroundWorker_DoWork;
            backgroundWorker.ProgressChanged += BackgroundWorker_ProgressChanged;
            backgroundWorker.RunWorkerCompleted += (o, e) => Close();
            backgroundWorker.RunWorkerAsync();

            CenterToParent();
        }

        private void BackgroundWorker_DoWork(object sender, DoWorkEventArgs e)
        {
            var backgroundWorker = (BackgroundWorker)sender;
            for (int i = 10; i >= 0; i--)
            {
                Thread.Sleep(1000);
                backgroundWorker.ReportProgress(0, i);
            }
        }

        private void BackgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
        {
            _lblCountdown.Text = $"{e.UserState}: {DateTime.Now}";
        }
    }
}

Wow, thanks ! That should get me started on BackgroundWorker. It does indeed look easier and cleaner than the basic thread with the Invoke calls.

I really appreciate you guys' continuous support and suggestions It's been a great help to me.

 

[cbreemer]

And here's the equivalent of post #15 using a BackgroundWorker:

Your code works beautifully, as expected, and it is clear and easy to understand. Am I right in assuming that any updates to the UI can only be done without using Invoke in the ProgressChanged handler ? Because when I change your code to set the label text in the DoWork handler, it says "Cross-thread operation not valid".
I don't have any progress to report, my thread continuously updates some fields and a grid based on information from the printer job queue. Moving those calls to ProgressChanged seems strange to me, plus that I don't see how I can pass several parameters to that function. I can see 3 ways forward :

1. Do the UI updates in DoWork, using Invoke, and forget about ProgressChanged
2. Do the UI updates in ProgressChanged, without Invoke, and use global variables
3. Do the UI updates in ProgressChanged, without Invoke, and somehow pass all needed variables in the userState object parameter

Which one would you recommend, or perhaps there is yet another possibility ?

 

[Skydiver]

All UI updates can be done in the ProgressChanged without using Invoke(). Globals bad. Pass things using UserState like I did above, or better yet, derive from BackgroundWorker and use class variables.

 

[Skydiver]

I don't have any progress to report, my thread continuously updates some fields and a grid based on information from the printer job queue.

Updating the UI is reporting the progress.

I've got a feeling that you are using your UI both as the View as well as the Model. Yes, that used to be the way to do things back in the 80's and 90's when memory and CPU constraints forced the developers hand, but modern systems have lots of CPU and memory to burn. You need to let the UI just be a reflection of the underlying data. Work on the data. Let the UI be updated when needed (or periodically) to give the user feedback that some work is happening.

Think of it this way: if you were writing a console app, you wouldn't be storing the information on the screen and be trying to go back to particular coordinates on the screen to read back the product name. You would have that product name somewhere in one of your internal data structures. The console output was just a reflection of what your internal data structures contains.

 

[jmcilhinney]

Note that, when using a BackgroundWorker, the DoWork event handler is executed on a secondary thread while the ProgressChanged and RunWorkerCompleted event handlers are executed on the UI thread. The steps are as follows:

1. Call RunWorkerAsync on the UI thread and pass any data required for the background work as a single object to the Argument parameter.
2. Handle the DoWork event and do the background work, getting the data from the e.Argument property.
3. If you need to update the UI during the background work, call ReportProgress and pass any data required by the UI as arguments. You can pass one int, which generally represents the progress, and one object containing arbitrary data. Handle the ProgressChanged event and update the UI as required.
4. If you need to update the UI after the background work, handle the RunWorkerCompleted event. Assign any data required to the e.Result property and get it back from the same property in the event handler and update the UI as required.

Here's one I prepared earlier:

private void Form1_Load(object sender, EventArgs e)
{
    // Raise the DoWork event in a worker thread.
    this.backgroundWorker1.RunWorkerAsync();
}
 
private void backgroundWorker1_DoWork(object sender,
                                      DoWorkEventArgs e)
{
    BackgroundWorker worker = (BackgroundWorker)sender;
 
    for (int i = 1; i <= 100; i++)
    {
        if (worker.CancellationPending)
        {
            // The user has cancelled the background operation.
            e.Cancel = true;
            break;
        }
 
        // Raise the ProgressChanged event in the UI thread.
        worker.ReportProgress(i, i + " iterations complete");
 
        // Perform some time-consuming operation here.
        System.Threading.Thread.Sleep(250);
    }
}
 
// This method is executed in the UI thread.
private void backgroundWorker1_ProgressChanged(object sender,
                                               ProgressChangedEventArgs e)
{
    this.progressBar1.Value = e.ProgressPercentage;
    this.label1.Text = e.UserState as string;
}
 
// This method is executed in the UI thread.
private void backgroundWorker1_RunWorkerCompleted(object sender,
                                                  RunWorkerCompletedEventArgs e)
{
    if (e.Cancelled)
    {
        // The background operation was cancelled.
        this.label1.Text = "Operation cancelled";
    }
    else
    {
        // The background operation completed normally.
        this.label1.Text = "Operation complete";
    }
}
 
private void button1_Click(object sender, EventArgs e)
{
    // Only cancel the background operation if there is a background operation in progress.
    if (this.backgroundWorker1.IsBusy)
    {
        this.backgroundWorker1.CancelAsync();
    }
}

 

[cbreemer]

Thanks guys, that makes things a lot clearer. I don't like to have to use Invoke so I'll handle the UI in ProgressChanged as advised, passing data either as class variables or an aggregate object parameter.
Yes, I am rather an old-fashioned programmer... I well know I should be using a data-bound grid for example, but I guess I'm too used to do things manually Every now and then I learn something new here though !

 

[cbreemer]

Not quite there yet... I have now implemented the BackgroundWorker as per SkyDiver's example. It is working vaguely but I hit upon something that totally stumps me.
DoWork has two objects which I pass to ReportProgress as an object of type object[]. That works well, I see that ReportProgress correctly picks up the two objects from the userState parameter. But it seems picky about which things it can access, and which it cannot because they are in a different thread. See screenshot:

pq is the PrintQueue object. The member pq.Name is referenced without problems, and contains the name of my printer as expected. However the member pq.NumberOfJobs raises an exception ! Interestingly, if I use jobs.Count instead of pq.NumberOfJobs , that does work, and gives me the expected number 16.
Argh.. this is killing me. Why one member of the same object, but not the other ? What is this voodoo ?

For good measure here's the relevant code. I'd like to understand this first before tacking the other hurdles which I know to be on the way (but have commented out for now).

private void PrintMonitor_Load(object sender, EventArgs e)
{
    var backgroundWorker = new BackgroundWorker() { WorkerReportsProgress = true };
    backgroundWorker.DoWork += BackgroundWorker_DoWork;
    backgroundWorker.ProgressChanged += BackgroundWorker_ProgressChanged;
    backgroundWorker.RunWorkerCompleted += (oo, ee) => Close();
    backgroundWorker.RunWorkerAsync();
}

private void BackgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    var prms = (object[])e.UserState;
    var pq   = (PrintQueue)prms[0];
    var jobs = (List<PrintSystemJobInfo>)prms[1];

    lblPrinter.Text    = pq.Name;
    lblDocuments.Text  = "" + pq.NumberOfJobs;
    //lblDocuments.Text  = "" + jobs.Count;
}

private void BackgroundWorker_DoWork(object sender, DoWorkEventArgs e)
{
    PrintQueue pq;
    List<PrintSystemJobInfo> jobs;
    var backgroundWorker = (BackgroundWorker)sender;

    while ( true )
    {
        pq   = LocalPrintServer.GetDefaultPrintQueue();
        jobs = new List<PrintSystemJobInfo>(pq.GetPrintJobInfoCollection());

        object[] prms = new object[2];
        prms[0] = pq;
        prms[1] = jobs;
        backgroundWorker.ReportProgress(0, prms);

        Thread.Sleep(1000);
    }
}

 

[JohnH]

Get those property values (the string and the integer) from the background thread and pass them to progress.