Exception when closing Form (thread + invoke)

Question

I have just started to learn about threads and methodinvoking in c#, but I have come across a problem which I couldn't find the solution of.

I made a basic C# form program which keeps updating and displaying a number, by starting a thread and invoke delegate.

Starting new thread on Form1_load:

private void Form1_Load(object sender, EventArgs e)
  {
        t = new System.Threading.Thread(DoThisAllTheTime);
        t.Start();
  }

Public void DoThisAllTheTime (which keeps updating the number) :

public void DoThisAllTheTime()
  {
     while(true)
      {
        if (!this.IsDisposed)
         {
           number += 1;
           MethodInvoker yolo = delegate() { label1.Text = number.ToString(); };
           this.Invoke(yolo);
         }
      }
  }

Now when I click the X button of the form, I get the following exception:

'An unhandled exception of type 'System.ObjectDisposedException' occurred in System.Windows.Forms.dll

Can't update a deleted object'

While I actually did check if the form was disposed or not.

EDIT: I added catch (ObjectDisposedException ex) to the code which fixed the problem. Working code:

  public void DoThisAllTheTime()
  {
     while(true)
      {
         number += 1;

         try {  
              MethodInvoker yolo = delegate() { label1.Text = number.ToString(); };
              this.Invoke(yolo);
             }
         catch (ObjectDisposedException ex)
             {
              t.Abort();
             }
      }
 }

Well, the form got disposed after `(!this.IsDisposed)` was calculated, but before `this.Invoke(yolo);` was called. Welcome to the world of races. — Joker_vD, Oct 27 '13 at 15:48
I have edited your title. Please see, "[Should questions include “tags” in their titles?](http://meta.stackexchange.com/questions/19190/)", where the consensus is "no, they should not". — John Saunders, Oct 27 '13 at 15:48
@Joker_vD How would I fix that? I assumed the check would be faster than the methodinvoke. It is also used on MSDN that way. ( http://msdn.microsoft.com/en-us/library/system.windows.forms.methodinvoker(v=vs.110).aspx?cs-save-lang=1&cs-lang=csharp#code-snippet-2 ) — John, Oct 27 '13 at 15:52
As others already said this is a race problem see: http://stackoverflow.com/questions/1874728/avoid-calling-invoke-when-the-control-is-disposed — Alireza, Oct 27 '13 at 15:53
@Bart don't count on MSDN sample code being correct. Usually, it isn't. — oefe, Oct 27 '13 at 15:57

Rob Lyndon · Accepted Answer · 2013-10-28T01:50:43.120

3

Your call to this.IsDisposed is always out of date. You need to intercept your form closing event and stop the thread explicitly. Then you won't have to do that IsDisposed test at all.

There are many ways you can do this. Personally, I would use the System.Threading.Tasks namespace, but if you want to keep your use of System.Threading, you should define a member variable _updateThread, and launch it in your load event:

_updateThread = new System.Threading.Thread(DoThisAllTheTime);
_updateThread.Start();

Then in your closing event:

private void Form1_Closing(object sender, CancelEventArgs e)
{
    _stopCounting = true;
    _updateThread.Join();
}

Finally, replace the IsDisposed test with a check on the value of your new _stopCounting member variable:

public void DoThisAllTheTime()
{
    MethodInvoker yolo = delegate() { label1.Text = number.ToString(); };
    while(!_stopCounting)
    {
        number += 1;
        this.Invoke(yolo);
    }
}

edited Oct 28 '13 at 01:50

answered Oct 27 '13 at 15:51

Rob Lyndon

12,089
5
49
74

I tried adding 't.abort();' to Form1_closing, but that didn't help either. – John Oct 27 '13 at 15:54
@Bart: Try setting a flag and then calling `t.Join();` to wait until the thread has recognized it. – Ry- Oct 27 '13 at 15:55
I added catch (ObjectDisposedException ex) to it. This fixed the problem. I will update my question. – John Oct 27 '13 at 16:01
@Bart: As oefe said, that’s heinous! :D – Ry- Oct 27 '13 at 16:02
And in fact, you don't need to define a new delegate at every stage of the loop -- that will make your garbage collector work a lot harder than it needs to. Instead, add a method to your class an invoke that. – Rob Lyndon Oct 27 '13 at 16:08
1

Since `Invoke` is blocking waiting for the main thread, there is a possibility for deadlock here if the `Form1_Closing` method is executed on the line `number += 1;`. The chance of deadlock goes up as more and more work is done in place of that line. I fixed this in my app by using `BeginInvoke` instead of `Invoke` since `BeginInvoke` will not block the thread from exiting and hence the `Join` will not block forever. – Wayne Uroda Jul 29 '16 at 01:03

score 2 · Answer 2 · answered Oct 27 '13 at 16:00

2

Just put this override in your form class:

protected override void OnClosing(CancelEventArgs e) {
    t.Abort();
    base.OnClosing(e);
}

answered Oct 27 '13 at 16:00

amiry jd

27,021
30
116
215

+1 For a nice solution. To get more info: see http://stackoverflow.com/questions/1874728/avoid-calling-invoke-when-the-control-is-disposed – Alireza Oct 27 '13 at 16:04
@Alireza Thanks buddy. The answer is compatible with starter's skills that I got -and thought- from the question. But thanks really. Cheers – amiry jd Oct 27 '13 at 16:13
2

Normal scenario shutdown MUST NOT include calls to `Thread.Abort()`, `Application.Exit()`, etc. I've seen a bug caused by this -- transactions weren't rolled back because of a spurios `Environment.Exit()` inside of an `OnClose()` event handler. – Joker_vD Oct 27 '13 at 20:19

score -3 · Answer 3 · answered May 17 '16 at 17:50

-3

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
Thread.CurrentThread.Abort();
}

answered May 17 '16 at 17:50

gunderhold

1

Exception when closing Form (thread + invoke)

3 Answers3

Linked