0

I am using an MVVM model in my WPF application. I have an command binding to the cancel button. I have a start button which starts a few background workers. When i click on the cancel button, i want all the background workers to stop/quit. With my current code when i click on cancel button, the background worker does not stop and the "StartEngineeringOperation" finishes. Can anyone please help me out with what i am doing wrong here?

Current code:

For EngineeringViewModel.cs:

public class EngineeringViewModel{

public EngineeringViewModel()
{
            StartEngineering= new DelegateCommand(o =>
            {
                worker = new BackgroundWorker
                {
                    WorkerReportsProgress = true,
                    WorkerSupportsCancellation = true
                };
                worker.ProgressChanged += Worker_ProgressChanged;
                worker.RunWorkerCompleted += worker_RunWorkerCompleted;
                if (worker.IsBusy != true) worker.RunWorkerAsync();
                worker.DoWork += (s, e) =>
                {
                    StartEngineeringOperation();
                    if (worker.CancellationPending)
                    {
                        e.Cancel = true;
                        return;
                    }
                };
            },
                k => true);
            Cancel = new DelegateCommand(CancelEngineeringOperation);
}

private void StartEngineeringOperation()
   {
      startAlarmService();
      startTrendQualityCheck();
   }

private void CancelEngineeringOperation(object param)
    {           
        worker.DoWork += (s, e) =>
        {
            if (worker.IsBusy)
            {
                worker.CancelAsync();
                e.Cancel = true;
                return;
            }
           
        };
       
    }
}

I tried this : but doesn't seem to work:

private void StartEngineeringOperation()
   {
      startAlarmService();                                                                                                      
           if (worker.CancellationPending)
            {
                e.Cancel = true;
                return;
            }
      startTrendQualityCheck();
   }
Atom99
  • 29
  • 7
  • 3
    You can't just "cancel" arbitrary code. Your `StartEngineeringOperation` has to manually check if cancellation was requested and (gracefully) stop what it was doing. – Evk Nov 11 '21 at 13:34
  • 1
    Consider using the [async await Asynchronous programming](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/) instead of background workers. There is also [CancellationToken](https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken?view=net-5.0) used for cancelling multithreaded operations. – Felix Castor Nov 11 '21 at 13:36
  • 1
    as Felix already pointed out `BackgroundWorkers` are outdated , you should use `Task.Run` with `async` and `await` – Stefano Cavion Nov 11 '21 at 13:42
  • 1
    Even though Background workers might be outdated - using tasks with cancellation tokens, or anything else, will not solve the general problem. – Evk Nov 11 '21 at 13:59
  • @Evk but how to i constantly check for the cancellation request in the StartEngineeringOperation? – Atom99 Nov 11 '21 at 14:22
  • Same way you do already: check worker.CancellationPending – Evk Nov 11 '21 at 14:30
  • i tried this: but doesn't seem to work: private void StartEngineeringOperation() { startAlarmService(); if (worker.CancellationPending) { e.Cancel = true; return; } startTrendQualityCheck(); } – Atom99 Nov 11 '21 at 15:00
  • 2
    Related: [How to stop BackgroundWorker correctly](https://stackoverflow.com/questions/4732737/how-to-stop-backgroundworker-correctly) – Theodor Zoulias Nov 11 '21 at 15:29
  • What is the actual work being done? A background task that does some work and then returns the result might be handled differently than something that is running continuously until stopped. – JonasH Nov 11 '21 at 15:46
  • @JonasH The actual work is to start a couple of services and fetch the status of the services and display in the status field in the UI, what i'm trying to achieve is to have a cancel button that, in case one wants to stop the work any time after it was started, can click on the cancel button and the work being done immediately terminates. – Atom99 Nov 11 '21 at 16:04

2 Answers2

1

As you may have learned from te comments, you need to poll the state of the BackgroundWorker in your operations that you want to support cancellation. Then take measures to cancel the ongoing operation gracefully.

The example shows how to cancel a background thread on button click. The first example uses the old BackgroundWorker and the second the modern and cleaner Task library.

BackgroundWorker

private BackgroundWorker Worker { get; set; }

private void StartWorker()
{
  this.Worker = new BackgroundWorker
  {
    WorkerReportsProgress = true,
    WorkerSupportsCancellation = true
  };
   
  this.Worker.DoWork += BackgroundWorker_DoWork;
}

private void BackgroundWorker_DoWork(object sender, DoWorkEventArgs e)
{
  BackgroundWorker worker = sender as BackgroundWorker;

  DoCancellableWork();  

  // Stop BackgroundWorker from executing
  if (worker.CancellationPending)
  {
    e.Cancel = true;
  }     
}

private void DoCancellableWork()
{      
  // Check for cancellation before executing the cancellable operation and allocating resources etc..
  if (this.Worker.CancellationPending)
  {
    return;
  }

  // Periodically/regularly check for the cancellation flag
  for (int i = 0; i <= 10000000000; i++)
  {
    if (this.Worker.CancellationPending)
    {
      // Cancel operation gracefully e.g., do some cleanup, free resources etc.

      return;
    }

    // Do some work
  }
}

// Alternatively use a command e.g., in a view model class
private void CancelBackgroundWorker_Click(object sender, EventArgs e)
{
  if (this.Worker.WorkerSupportsCancellation)
  {
    this.Worker.CancelAsync();
  }
}

Task library

The example uses Progress<T> to report progress from the background thread to the UI thread.

private CancellationTokenSource CancellationTokenSource { get; set; }

private async Task StartWorker()
{
  this.CancellationTokenSource = new CancellationTokenSource();

  // Prepare callback to update UI from the background thread.
  // The Progress<T> instance MUST be created on the UI thread
  IProgress<int> progressReporter = new Progress<int>(progress => this.ProgressBar.Value = progress);

  await Task.Run(
    () => DoWork(progressReporter, this.CancellationTokenSource.Token), 
    this.CancellationTokenSource.Token);

  this.CancellationTokenSource.Dispose();
}

private void DoWork(IProgress<int> progressReporter, CancellationToken cancellationToken)
{
  DoCancellableWork(progressReporter, cancellationToken);
}

private void DoCancellableWork(IProgress<int> progressReporter, CancellationToken cancellationToken)
{
  // Check for cancellation before executing the operation and allocating resources etc..
  if (cancellationToken.IsCancellationRequested)
  {
    return;
  }

  // Periodically/regularly check for the cancellation flag
  for (int i = 0; i <= 10000000000; i++)
  {
    if (cancellationToken.IsCancellationRequested)
    {
      // Cancel operation gracefully e.g., do some cleanup, free resources etc.

      return;
    }

    // Do some work

    // Report progress
    progressReporter.Report(20);
  }
}

// Alternatively use a command e.g., in a view model class
private void CancelBackgroundThread_Click(object sender, EventArgs e)
{
  this.CancellationtokenSource?.Cancel();
}
BionicCode
  • 1
  • 4
  • 28
  • 44
  • Thank you for the answer @BionicCode. But my CancellableWork does not need to be done periodically. So using a for loop does not work. I need the Work to be cancelled at any time between the start and completion of the work depending on the cancel button click. The work to be done is not cyclic. How can i poll for the "IsCancellationRequested" without using a loop? – Atom99 Nov 12 '21 at 03:56
  • The loop is just an example to mimic a long running operation. As you can see in the example, the stae is polled before/outside the loop too. You can check the state anytime between the instruction calls. If you call other operations that need cancellation, then pass in the CancellationToken. – BionicCode Nov 12 '21 at 05:08
  • 1
    It's not about the loop. That's why I wrote in the code comment above the loop "periodically/regularly check...". – BionicCode Nov 12 '21 at 05:10
  • ok, got it! Just a question though, how do I poll/check for the CancellationPending property regularly? The cancellation request can come at any time, so that would mean that I would have to check for the CancellationPending before/after and during almost every statement/operation. That sounds absurd. Is there an easier way to do it? Or did I get this completely wrong? – Atom99 Nov 12 '21 at 15:15
  • No, that is completely right. But you must consider that the code is executed by a machine that processes billions statements in a second. It's difficult to tell how fast your `startAlarmService()` method executes. If it does nothing intensive then assume it to return after a few miliseconds. Probably faster than you can click the cancel button. Taking this into account you don't want to poll the CancellationPending property after each statement. – BionicCode Nov 12 '21 at 15:38
  • Usually you check it 1) **before** you allocate resources like threads (e.g. before starting the `BackgroundWorker`) or managed that implement IDisposable or unmanaged in general as allocation can be expensive as well as freeing those resources as this involves the garbage collector which impacts perfomance. You would also check the cancellation state in expensive loops. – BionicCode Nov 12 '21 at 15:38
  • 1
    2) **before** calling another long running operation (while the long running operation would execute useful checks itself, so that it can cancel itself once the long running operations got kicked off). This means you must decide when to check CancellationPending. – BionicCode Nov 12 '21 at 15:38
  • 1
    3) Probably before persisting data (write to a file, database etc.). – BionicCode Nov 12 '21 at 15:38
  • 1
    4) before creating a result or updating UI. All this considerations apply for every method that is called (call tree). – BionicCode Nov 12 '21 at 15:39
  • Thank you @BionicCode. That clears it up! I have implemented it in my code. It finally seems to work! – Atom99 Nov 12 '21 at 17:10
  • It seems to almost work as I've stumbled upon another issue. I have a for loop doing a couple of jobs: – Atom99 Nov 12 '21 at 17:12
  • It seems to almost work as I've stumbled upon another issue. I have a for loop doing a couple of jobs: foreach (var job in jobs){ if(worker.CancellationPending) {e.cancel=true; DoJob(job);} It seems like at least the first job will get executed completely before the "CancellationPending" property is set to true. So is there any way I can stop the "job1" while it is executing as soon as the cancel button is clicked or do I wait for "job1" to be finished and in the next iteration the loop breaks thereby job2,job3 etc.. doesn't execute? – Atom99 Nov 12 '21 at 17:18
  • You must iterate the above considerations for each call to a method. Then inside each method you have to analize it again. So inside the called method e.g., DoJob() you must think if and where it makes sense to abort the method and insert a CancellationPending check there. – BionicCode Nov 12 '21 at 17:22
0

Since the OP describes the task being done as "checking services", I would assume the work done looks something like this:

while(true){
    // check service
    // Post result back to UI thread
    Thread.Sleep(...);
}

This is not the best way to write such such a check. As in most cases where Thread.Sleep is used, a timer would be a better alternative:

var myTimer  = new System.Timers.Timer(...);
myTimer .Elapsed += OnTimedEvent;
myTimer .AutoReset = true;
myTimer .Enabled = true;

...

private void OnTimedEvent(Object source, ElapsedEventArgs e)
{
    // check service
    // Post result back to UI thread
}

This makes the problem of stopping/starting the task being done a simple matter of changing the Enabled-flag of the timer. It is also possible to use a timer, or a synchronization context to run the event directly on the UI thread, this is probably the best solution if "checking services" only takes a few ms.

JonasH
  • 28,608
  • 2
  • 10
  • 23