Stop long running events

Question

I have a following form Form3 that is opened by another form Form1, and when closed Form1 opens back up.

The problem is when I close Form3 DoSomething keeps running after form is closed.

I understand that I can make DoSomething into a thread and set IsBackground = true but is there another way to stop all processes when form closes.

This code is just example, For illustration.

   public partial class Form3 : Form
    {

    public Form3()
    {
        InitializeComponent();
    }
    private void DoSomething()
    {
        int i = 0;
        while(true)
        {
            if (!this.IsDisposed)
            {
                Application.DoEvents();
                i++;
                Thread.Sleep(10);
                label1.Text = i.ToString();
                dataGridView1.Rows.Add();
            }
        }
    }

    private void button1_Click(object sender, EventArgs e)
    {
        DoSomething();
    }

    private void Form3_FormClosed(object sender, FormClosedEventArgs e)
    {
        this.Dispose();
        Form1.Default.Show();
    }

}

You probably meant `while (!this.IsDisposed) {` instead of `while (true) { if (!this.IsDisposed) {` — cdhowie, Oct 03 '12 at 15:52
No I wanted while(true) to create endless loop to illustrate the problem. Once form is closed DoSomething should stop execution, but I am not sure how to accomplish that without using threads. — Andrew Rokicki, Oct 03 '12 at 15:56
`while (true) {` does not illustrate the problem, *it is the problem.* — cdhowie, Oct 03 '12 at 15:59
if I make while (!this.IsDisposed) I still have the same problem. Ofter close DoSomething is running the loop event though !this.IsDisposed shows false in the watch. I get error "No row can be added to a DataGridView control that does not have columns. Columns must be added first." — Andrew Rokicki, Oct 03 '12 at 16:09
That is because you don't synchronize termination of the method. If you dispose the form while `Thread.Sleep()` is executing, the thread will wake up and continue on its way trying to do stuff on disposed objects. The problem isn't that the loop doesn't terminate, it's that you're not *waiting for it to terminate* before disposing the form, which means that you need to use something other than `IsDisposed` as your "abort" flag. It would also help to move `DoEvents()` to be the last call in the loop, so that when control returns to this method, the "abort" flag is the first thing you check. — cdhowie, Oct 03 '12 at 16:13
Bingo. That did it if your want to post this as answer. I can check you as the answer used. Thank you cdhowie. — Andrew Rokicki, Oct 03 '12 at 16:35

score 2 · Answer 1 · answered Oct 03 '12 at 15:57

2

You never break out of the while(true). You should either break the loop when it's IsDisposed is true, change your while loop to while(!IsDisposed), or store use a class level variable that determines when to break the loop.

I would probably opt for the latter, as it gives you a little more control.

public partial class Form3 : Form
{
    volatile bool clDoSomething;

    public Form3()
    {
        InitializeComponent();
    }
    private void DoSomething()
    {
        int i = 0;
        clDoSomething = true;
        while(clDoSomething)
        {
            Application.DoEvents();
            ++i;
            Thread.Sleep(10);
            label1.Text = i.ToString();
            dataGridView1.Rows.Add();
        }
    }

    private void button1_Click(object sender, EventArgs e)
    {
        DoSomething();
    }

    private void Form3_FormClosed(object sender, FormClosedEventArgs e)
    {
        clDoSomething = false;
        Form1.Default.Show();
    }

}

answered Oct 03 '12 at 15:57

Sheridan Bulger

1,214
7
9

1

Do not forget to make `clDoSomething` volatile, or the JIT is free to emit code that caches the value of the variable in `DoSomething()` which will lead to another (more difficult to detect) infinite loop. – cdhowie Oct 03 '12 at 15:58
On second examination, `volatile` *may* not be necessary here, since there is actually no threading happening. But at least it won't hurt. And, as I mentioned in another comment, it would probably be good to move `DoEvents()` to be the last call in the while loop, so that when control returns to this method, the abort flag is checked right away, instead of executing the loop body once more. – cdhowie Oct 03 '12 at 16:16
I always operate under the assumption where if you're not sure, take the safer approach until research proves it's not necessary. I've only used `volatile` for thread-safety concerns in the past, but it certainly can't hurt. – Sheridan Bulger Oct 03 '12 at 16:25

score 2 · Answer 2 · edited May 23 '17 at 11:48

Your fundamental approach is flawed.

First off, Application.DoEvents should be avoided unless you are sure that you really need it, and that you are using it correctly. You do not need it here, and you are not using it correctly.

What you really need here is a Timer.

private Timer timer = new Timer();
private int count = 0;
public Form3()
{
    InitializeComponent();

    timer.Tick += timer_Tick;
    timer.Interval = 10;

    //when the form is closed stop the timer.
    FormClosed += (_, args) => timer.Stop();
}

private void button1_Click(object sender, EventArgs e)
{
    count = 0;
    timer.Start();
}

private void timer_Tick(object sender, EventArgs e)
{
    count++;
    label1.Text = count.ToString();
    dataGridView1.Rows.Add();
}

When the Form is create the Timer is configured. The tick event is set, along with the interval. The tick event will look similar to your DoSomething method; it will involve running some bit of code every 10 seconds, from the UI thread, while keeping the UI responsive. When the form is closed simply stop the timer and it will stop firing off these events.

Also note that in this example here pressing the button multiple times simply resets the timer and the count, it doesn't end up creating two loops that each fire every 10 milliseconds.

The OP asked why his loop doesn't break, which is what my answer was addressing. This answer is a far more sensible solution to the actual use-case, however. ++ — Sheridan Bulger, Oct 03 '12 at 16:35
That is a good solution but, the code I posted is just a sudo code for the real application for sake of simplicity and illustration and making a timer would not be applicable since the sleep(10) is used to simulate serial communication that is happening in the production app. I guess I should have posted that in original question. — Andrew Rokicki, Oct 03 '12 at 16:46
@AndrewRokicki Yes, you probably should have said that. In that case what you should be using is `BackgroundWorker`. The `timerTick` should go in the `ReportProgress` event handler, and the IO work should be in a loop inthe `DoWork` event. `BackgroundWorker` also supports cancellation, so you can have the loop checking if it's canceled, and then cancel the BGW when the form is closed. — Servy, Oct 03 '12 at 16:52

score 0 · Answer 3 · answered Oct 03 '12 at 15:54

0

Override this.Dispose() or this.Close() as appropriate and kill off DoSomething() manually.

answered Oct 03 '12 at 15:54

Dan Pichelman

2,312
2
31
42

score 0 · Accepted Answer · answered Oct 04 '12 at 19:45

Thanks to cdhowie suggestions and input of all others. Mowing DoEvents to the end and adding IsDipsosed solved my problem.

public partial class Form3 : Form
{
    public Form3()
    {
        InitializeComponent();
    }
    private void DoSomething()
    {
        int i = 0;
        while ((true) && !this.IsDisposed)
        {
                i++;
                Thread.Sleep(10);
                label1.Text = i.ToString();
                dataGridView1.Rows.Add();
                Application.DoEvents();
        }
    }

    private void button1_Click(object sender, EventArgs e)
    {
        DoSomething();
    }

    private void Form3_FormClosed(object sender, FormClosedEventArgs e)
    {
        Form1.Default.Show();
    }

}

score 0 · Answer 5 · edited Nov 28 '19 at 18:12

0

try to add this intsruction in the FormClosing event : System.Diagnostics.Process.GetCurrentProcess().Kill();

it's a little bit like this:

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
    System.Diagnostics.Process.GetCurrentProcess().Kill();
}

edited Nov 28 '19 at 18:12

ChrisMM

8,448
13
29
48

answered Nov 28 '19 at 16:50

haithem khdimallah

1

Stop long running events

5 Answers5