Search code examples
c#multithreadingthread-safetyprogress-barbackgroundworker

cancelling a backgroundworker with while loop


i know the common ways of cancelling a backgroundworker using eventwaithandles... but i wanna know is that right to use a while loop to trap and pause working of a backgroundworker ? i coded like this :

    Bool stop = false;

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        progressBar1.Minimum = 0;
        progressBar1.Maximum = 100000;
        progressBar1.Value = 0;

        for (int i = 0; i < 100000; i++)
        {
          progressBar1.Value++;

            if (i == 50000)
                stop = true;

            while (stop)
            { }
        }
    }
    private void button1_Click(object sender, EventArgs e)
    {
        stop = !stop;
    }

Solution

  • Did you try it? What happened? Was it what you wanted to happen? Did you notice your computer's fans speeding up, to handle all the heat from your CPU in a tight, "do-nothing" loop?

    Fact is, you should not "pause" a background task in the first place; if you don't it to keep running, interrupt it. If you want to be able to resume later, provide a mechanism to allow that. Even having your thread blocked efficiently waiting on a WaitHandle object would be the wrong thing to do, because it wastes a thread pool thread.

    The code you've posted here is about the worst way to implement "pausing". Instead of waiting on some synchronization object such as a WaitHandle, you have the current thread just loop without interrupting, constantly checking the value of a flag. Even ignoring the question of whether you're using volatile (the code example doesn't show that, but then it also wouldn't compile, so…), it's terrible to force a CPU core to do so much work and yet get nowhere.

    Don't pause your BackgroundWorker.DoWork handler in the first place. Really. Just don't do that. But if you insist, then at least use some kind of waitable object instead of a "spin-wait" loop as in the example you've posted here.


    Here's an example of how your code might work if you wanted to avoid altogether tying up a thread while "paused". First, don't use BackgroundWorker, because it doesn't have a graceful way to do this. Second, do use await…that does specifically what you want: it allows the current method to return, but without losing track of its progress. The method will resume executing when the thing it waited on indicates completion.

    In the example below, I've tried to guess at what the code that calls RunWorkerAsync() looks like. Or rather, I just assumed you've got a button2, which when clicked you call that method to start your worker task. If this is not enough to get you pointed in the right direction, please improve your question by including a good, minimal, complete code example showing what you're actually doing.

    // These fields will work together to provide a way for the thread to interrupt
    // itself temporarily without actually using a thread at all.
    private TaskCompletionSource<object> _pause;
    private readonly object _pauseLock = new object();
    
    private void button2_Click(object sender, DoWorkEventArgs e)
    {
        // Initialize ProgressBar. Note: in your version of the code, this was
        // done in the DoWork event handler, but that handler isn't executed in
        // the UI thread, and so accessing a UI object like progressBar1 is not
        // a good idea. If you got away with it, you were lucky.
        progressBar1.Minimum = 0;
        progressBar1.Maximum = 100000;
        progressBar1.Value = 0;
    
        // This object will perform the duty of the BackgroundWorker's
        // ProgressChanged event and ReportProgress() method.
        Progress<int> progress = new Progress<int>(i => progressBar1.Value++);
    
        // We do want the code to run in the background. Use Task.Run() to accomplish that
        Task.Run(async () =>
        {
            for (int i = 0; i < 100000; i++)
            {
                progress.Report(i);
    
                Task task = null;
    
                // Locking ensures that the two threads which may be interacting
                // with the _pause object do not interfere with each other.
                lock (_pauseLock)
                {
                    if (i == 50000)
                    {
                        // We want to pause. But it's possible we lost the race with
                        // the user, who also just pressed the pause button. So
                        // only allocate a new TCS if there isn't already one
                        if (_pause == null)
                        {
                            _pause = new TaskCompletionSource<object>();
                        }
                    }
    
                    // If by the time we get here, there's a TCS to wait on, then
                    // set our local variable for the Task to wait on. In this way
                    // we resolve any other race that might occur between the time
                    // we checked the _pause object and then later tried to wait on it
                    if (_pause != null)
                    {
                        task = _pause.Task;
                    }
                }
    
                if (task != null)
                {
                    // This is the most important part: using "await" tells the method to
                    // return, but in a way that will allow execution to resume later.
                    // That is, when the TCS's Task transitions to the completed state,
                    // this method will resume executing, using any available thread
                    // in the thread pool.
                    await task;
    
                    // Once we resume execution here, reset the TCS, to allow the pause
                    // to go back to pausing again.
                    lock (_pauseLock)
                    {
                        _pause.Dispose();
                        _pause = null;
                    }
                }
            }
        });
    }
    
    private void button1_Click(object sender, EventArgs e)
    {
        lock (_pauseLock)
        {
            // A bit more complicated than toggling a flag, granted. But it achieves
            // the desirable goal.
            if (_pause == null)
            {
                // Creates the object to wait on. The worker thread will look for
                // this and wait if it exists.
                _pause = new TaskCompletionSource<object>();
            }
            else if (!_pause.Task.IsCompleted)
            {
                // Giving the TCS a result causes its corresponding Task to transition
                // to the completed state, releasing any code that might be waiting
                // on it.
                _pause.SetResult(null);
            }
        }
    }
    

    Note that the above is just as contrived as your original example. If all you had really was a simple single loop variable iterating from 0 to 100,000 and stopping halfway through, nothing nearly so complicated as the above would be required. You'd just store the loop variable in a data structure somewhere, exit the running task thread, and then when you want to resume, pass in the current loop variable value so the method can resume at the right index.

    But I'm assuming your real-world example is not so simple. And the above strategy will work for any stateful processing, with the compiler doing all the heavy-lifting of storing away intermediate state for you.