1

I have Web Api which gets CancellationToken from users, the method inside it (DoWork) also get CancellationToken:

[HttpPost]
public async Task<long> GetInfo(CancellationToken cancellationToken)
{
   long result = 0;
   bool notDone = true;
   Task<long> t = Task.Run(async () =>
   {

      if (cancellationToken.IsCancellationRequested)
         cancellationToken.ThrowIfCancellationRequested();

      while (notDone && !cancellationToken.IsCancellationRequested)
      {
         result = await DoWork(cancellationToken);
         notDone = false;
      }

      return result;
   }, cancellationToken);
   try
   {
      return await t;
   }
   catch (AggregateException e)
   {
      Debug.WriteLine("Exception messages:");
      foreach (var ie in e.InnerExceptions)
         Debug.WriteLine("   {0}: {1}", ie.GetType().Name, ie.Message);

      Debug.WriteLine("\nTask status: {0}", t.Status);
      throw;
   }
   catch (Exception ex)
   {
      throw;
   }
}

private Task<long> DoWork(CancellationToken token)
{
   long result = 0;
   bool notDone = true;

   Task<long> task = Task.Run(() =>
   {

      if (token.IsCancellationRequested)
         token.ThrowIfCancellationRequested();

      while (notDone && !token.IsCancellationRequested)
      {
         Thread.Sleep(8000);
         result = 2;
         notDone = false;
      }

      return result;
   }, token);

   return task;
}

I expect when the user cancels the request it aborts the DoWork method and not continue the function, but after sending an Exception, when "Thread.Sleep" complete, the DoWork method continue. the user can call the API service like this method "cc" as you can see it cancel after 5 seconds and in the DoWork method "Thread.Sleep" is 9 seconds. the user gets an Exception but the method still running.

private async Task<bool> cc()
{
   UriBuilder builder = new UriBuilder("http://localhost:12458/api/Test/GetInfo");

   ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12;
   HttpClient client = new HttpClient();
   System.Threading.CancellationTokenSource s = new System.Threading.CancellationTokenSource();
   s.CancelAfter(5000);
   try
   {
      var result = client.PostAsJsonAsync<model1>(builder.ToString(), new model1 { }, s.Token).Result;
      string tmp = result.Content.ReadAsStringAsync().Result;
      long ApiResult = JsonConvert.DeserializeObject<long>(tmp);
   }
   catch (TaskCanceledException ex)
   {
   }
   catch (OperationCanceledException ex)
   {
   }
   catch (Exception ex)
   {
   }
   finally
   {
      s.Dispose();
   }
   return false;
}
TheGeneral
  • 79,002
  • 9
  • 103
  • 141
Mishaa
  • 97
  • 1
  • 13
  • 3
    This code is a mess, its hard to help because there is a lot of suspect stuff going on. I mean, what is the point of `notDone`. This loop should finish after one iteration, so i am guessing its not your real code. Also you are running fake async (see stephen cleary). And lastly the fact that you are trying to catch `AggregateException` points to the fact you have other suspect code going on which you arnt showing. – TheGeneral Jan 10 '21 at 08:12
  • Lets take a step back. What are you trying to achieve? if we know that we might be able to giving a simple working solution. – TheGeneral Jan 10 '21 at 08:14
  • Dear @00110001 I want when the user cancels the request, the web API service and the method inside it cancel at the same time, in my code "DoWork" when the user cancels a request, the method continues after Thread.Sleep(8000); – Mishaa Jan 10 '21 at 08:22
  • So i am guessing in your real code you are not using Thread.Sleep and there is some long running process you need to abort? What is the code to this long running task ? Did you write it? I feel the answer to what this presumed long running task is, will draw out the right solution – TheGeneral Jan 10 '21 at 08:29
  • What is the point of `Thread.Sleep(8000)`? – Theodor Zoulias Jan 10 '21 at 08:30
  • @00110001 exactly, I don't use Thread. Sleep in my code, it's just a demonstration of how long my real code takes to execute. – Mishaa Jan 10 '21 at 08:33
  • @Mishaa yup, now we are getting somewhere. So is the long running code you have a loop ? or is it some IO work (meaning, is it File / DB, or network / internet calls)? is it something that can take an CancellationToken, is it something you await, did you write it ? – TheGeneral Jan 10 '21 at 08:35
  • The reason i ask, is because if its a loop we will need to check a CancellationToken. If its a well written IO call, it should be able to take a CancellationToken, if its nothing you have control over we will have to abandon the call somehow. – TheGeneral Jan 10 '21 at 08:38
  • @00110001 I don't have control over the code, and I don't know if it has a loop or not. I can add ` t = Thread.CurrentThread;` then `token.Register(() => { t.Abort();});` but I am not sure it is the right thing to do – Mishaa Jan 10 '21 at 08:44
  • 1
    Ok if you have no control over that code, there is no reliable way to stop it easily. You have 2 options from here, you can abandon it... Meaning that the code will still run and if it has side affects then you cant do much about it. 2nd approach is load this code up in a new app domain and tear it down forcefully (this is not optimal, but it can be done a lot more safely than trying to abort a thread in your current domain). – TheGeneral Jan 10 '21 at 08:49
  • 1
    No, you should not use `Thread.Abort` in your current domain, the only relative safe way to use it, is to load up a new AppDomain, run the thread there, and abort where it has less chance of corrupting something – TheGeneral Jan 10 '21 at 08:50
  • @00110001 I don't get how can I use the 2nd approach. could please help me more. and could you please write your help as an answer so I can check it as solution? – Mishaa Jan 10 '21 at 08:54
  • [What's wrong with using Thread.Abort()](https://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort). Starting from .NET Core and .NET 5 the `Thread.Abort` is no longer supported in .NET, so it's now more difficult to shot yourself in the foot. But if you passionately want to do it, I am sure you'll find a way (like calling the Windows API directly). – Theodor Zoulias Jan 10 '21 at 09:05
  • @TheodorZoulias thanks for the link, but I still don't get how I can stop my method from executing after service being canceled. – Mishaa Jan 10 '21 at 09:12
  • Before we go down the other route... Is there any reason you just cant abandon the long running task ? – TheGeneral Jan 10 '21 at 09:17
  • @00110001 because it might save or delete something from the database. – Mishaa Jan 10 '21 at 09:19
  • Mishaa you could ask whoever implemented this service, to offer a cancelable version of the API. Cancellation is either cooperative, or doesn't exist. Fighting to cancel an API that was not designed to be cancelable, is a lost cause. – Theodor Zoulias Jan 10 '21 at 09:57
  • @TheodorZoulias the API I have in the DoWork method (I put Thread.Sleep(8000) instead ), I can't change it. but the rest of the code can be change. – Mishaa Jan 10 '21 at 10:16
  • Mishaa if you can't change the API, then my advice is to treat cancellation as a non-available feature, and forget about it. – Theodor Zoulias Jan 10 '21 at 11:11

3 Answers3

4

When use Thread.Sleep(8000) actually hold the main thread for 8 seconds and It can't check the cancellation token. you should use Task.Delay with cancellation token like this:

while (notDone && !token.IsCancellationRequested)
  {
     await Task.Delay(8000, token);
     result = 2;
     notDone = false;
  }

Task.Delay check the cancellation token itself.

sa-es-ir
  • 3,722
  • 2
  • 13
  • 31
  • The `Task.Delay` is intended to be `await`ed. Otherwise it becomes a fire-and-forget task with zero effect to the program (other than adding work for the garbage collector). – Theodor Zoulias Jan 10 '21 at 08:28
  • Thread.Sleep(8000) is not my real code, I just put it here to show how long my code takes to execute completely. – Mishaa Jan 10 '21 at 08:39
  • 1
    @TheodorZoulias Yes you are right. I edited the answer – sa-es-ir Jan 10 '21 at 08:40
  • @Mishaa with ``Task.Delay`` should be worked – sa-es-ir Jan 10 '21 at 08:42
  • You should check IsCancellationRequested in your code, it doesn’t cancel your work automatically, you should do it. If you want to be sure you also can check it before return. – Lana Jan 10 '21 at 09:45
  • 1
    @Lana you should rarely, if ever, check the `IsCancellationRequested` property. The correct API to call for consistent cancellation is the `ThrowIfCancellationRequested` method. The standard cancellation pattern in .NET communicates cancellation by throwing and handling an `OperationCanceledException`. – Theodor Zoulias Jan 10 '21 at 10:02
  • 1
    @TheodorZoulias you are absolutely right I've changed my answer. – Lana Jan 10 '21 at 10:08
1

Ok, in your code you should process CancellationToken.IsCancellationRequested too. It's not kinda of magic, you should do this work.

    public void DoWork(CancellationToken ctsToken)
    {
        ctsToken.ThrowIfCancellationRequested();
        DoSomething();

        ctsToken.ThrowIfCancellationRequested();
        DoSomethingElse();

        // end so on with checking CancellationToken before every part of work
    }

And your Task should look like this

    Task<long> t = Task.Run(async () =>
    {
        cancellationToken.ThrowIfCancellationRequested();

        result = await DoWork(cancellationToken);
        notDone = false;

        cancellationToken.ThrowIfCancellationRequested();

        return result;
    }, cancellationToken);
Lana
  • 1,024
  • 1
  • 7
  • 14
0

In my case it was because of fiddler. When I closed the fiddler app, it started working like a charm.