4

Imagine I Have such code:

 private bool flag = false;
 public SomeClass()
 {
     public void setup()
    {
        worker = new BackgroundWorker();
        worker.DoWork += worker_DoWork;

        if(!worker.IsBusy)
            worker.RunWorkerAsync();
    }

    void worker_DoWork(object sender, DoWorkEventArgs e)
    {
        if(flag == true)
          return;
        //whatever You want the background thread to do...
        SomeClass.SomeStaticMethodWhichHasLoopInside();
    }
}

When user clicks Exit button in my application I can set flag=true inside - is this the right way to stop the thread started by BackgroundWorker? I want to do this only when application wants to quit - so that I stop the thread. Or it will be automatically killed?

  • The best practice is written in [Sriram Sakthivel's answer](http://stackoverflow.com/a/30231268/2144232) hower I must mention, that your solution might work if you mark `flag` as `volatile`. Without the `volatile` keyword - which prevents the variable from being cached - the optimiser might store the `flag` of your thread in L2 or a register while the `flag` of your main thread is stored in the RAM, so your without `volatile` your thread might never get informed. – mg30rg May 14 '15 at 11:45

1 Answers1

7

No. Don't do that. It will not work as expected. Your flag field isn't even declared as volatile; it is possible that true value of the field isn't flushed to the memory yet. Other thread will still see the flag to be false because it caches its own copy of the value and your BackgroundWorker may never end.

Instead, just call BackgroundWorker.CancelAsync and check for BackgroundWorker.CancellationPending boolean inside the DoWork event handler.

private void OnExit(..)
{
    worker.CancelAsync();
}

So your DoWork method will become

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    while(!worker.CancellationPending)
    {
        //whatever You want the background thread to do...
    }
}

If you don't have a loop, you need to check CancellationPending property time to time.

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    if(worker.CancellationPending)
        return;
    //Do something..
    if(worker.CancellationPending)
        return;
    //Do some other thing..
    if(worker.CancellationPending)
        return;
    //Do something else..
}

I want to do this only when application wants to quit - so that I stop the thread. Or it will be automatically killed?

It will be killed automatically by the CLR since the thread is a Background Thread. But, don't rely on this fact because we don't know what your thread will be executing at the time CLR kills the thread. For example, it might have been writing a file in which case you'd be left with the corrupted file.

Consequences can be even worse, so it is always recommended to stop your threads yourself gracefully.

Question asked in comments: How can I cancel when the loop is in some other method?

It seems you're using .Net4.5. You can take advantage of CancellationTokenSource and CancellationToken. Also you may consider using TPL and async-await features which will make your life a lot easier.

private CancellationTokenSource tokenSource = new CancellationTokenSource();

private void OnExit(..)
{
    tokenSource.Cancel();
}

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    //whatever You want the background thread to do...
    SomeClass.SomeStaticMethodWhichHasLoopInside(tokenSource.Token);
}

class SomeClass
{
    public static void SomeStaticMethodWhichHasLoopInside(CancellationToken token)
    {
        while (!token.IsCancellationRequested)
        {
            //Do something
        }
    }
}
Sriram Sakthivel
  • 72,067
  • 7
  • 111
  • 189
  • There is a loop inside a method which is called inside the `worker_Dowork` method - but that method is from another class - how do I refer to worker from that method? make `worker` static? –  May 14 '15 at 07:07
  • do you mean this: http://postimg.org/image/efldufscd/? Or this http://postimg.org/image/gefw4bu9f/ Please provide full code for the solution - I am a little bit new to C# –  May 14 '15 at 07:21
  • I have updated question how the thread looks like using `SomeStaticMethodWhichHasLoopInside` - please show answer how would I check that property from that method –  May 14 '15 at 07:30
  • Why can't I declare `worker` as static and check for `SomeClass.Static_worker.CancellationPending` from `SomeStaticMethodWhichHasLoopInside` -- looks easier ?? –  May 14 '15 at 07:36
  • @user300455 That's not a good practice. I discourage the use of static. If you want, you may do it. But I recommend the right way which is right there in my answer. – Sriram Sakthivel May 14 '15 at 07:38
  • So your solution will work even if I click exit *after* the `SomeStaticMethodWhichHasLoopInside` has already been called and the `tokenSource` was already passed to it?. Why do you discourage with static? From other method I could check: `Class.static_worker.CancellationPending` –  May 14 '15 at 07:41
  • @user300455 It will work. Try it out. There are several reasons for discouraging static members. Refer [this question](http://stackoverflow.com/questions/7026507/why-are-static-variables-considered-evil) also there are many discussions about static. Just do a web search in this topic. – Sriram Sakthivel May 14 '15 at 07:46