3

I have the following piece of code:

private void btnAction_Click(object sender, RoutedEventArgs e)
{

    /** Clear the results field */
    txtResult.Text = "";

    /** Disable the button and show waiting status */
    btnAction.IsEnabled = false;
    lblStatus.Text = "Wait...";

    /** Get input from the query field */
    string input = query.Text;

    /** Run a new task */
    Task.Run(() => {

        // calling a method that takes a long time (>3s) to finish and return
        var attempt = someLibrary.doSomethingWith(input);

        // return the result to the GUI thred
        this.Dispatcher.Invoke(() =>
        {

            if (attempt.ContainsKey("success"))
            {
                if (attempt["success"] == true)
                {
                    txtResult.Text = "Success! The door is: " + (attempt["is_open"] ? "open" : "closed");
                    lblStatus.Text = "";
                }
                else
                {
                    lblStatus.Text = "Error! The service says: " + attempt["errorMessage"];
                }
            }

            else
            {
                MessageBox.Show("There was a problem getting results from web service.");
                lblStatus.Text = "";
            }

            /** Re-enable the button */
            btnAction.IsEnabled = true;

        });
    });

}

Now, I would like to:

  • Write the same code in a way that uses a callback instead of using Dispatcher.Invoke().
  • Be able to cancel a running task that calls doSomething()
  • Be able to chain multiple calls, i.e. await doSomething() and after it finished, doAnotherThing() with the results from the previous call

Hence why I want to write this using the async model.

What do I do?

ConfusedGuy
  • 121
  • 9
  • 1
    define "*Be able to chain multiple calls*" – TheGeneral Feb 19 '20 at 01:14
  • You can move all the UI stuff outside. There's no need to `Invoke()` anything here. You just using the result of the method (`attempt`) to set the text of some controls. `Task.Run()` your method and await it. Make the handler `async`. – Jimi Feb 19 '20 at 01:20
  • There's nothing here that would benefit from being async. If you made `someLibrary.doSomethingWith` async, then you'd have a reason to make the event handler async and eliminate the call to `Task.Run`. – madreflection Feb 19 '20 at 01:22
  • @madreflection the main benefit here would be getting a long running task off the message pump and the continuations so you could update the UI – TheGeneral Feb 19 '20 at 01:24
  • 2
    @MichaelRandall: That's happening with `Task.Run`, but the question is about making the event handler async and *not* using `Task.Run`. The impetus for that change is probably because `Dispatcher.Invoke` has to be used to get back to the UI thread. – madreflection Feb 19 '20 at 01:25
  • 1
    @madreflection `Dispatcher.Invoke()` does not need to be used at all. If you `var attempt = await TaskRun(() => someLibrary.doSomethingWith(input));`, you can use the result to set the UI. This doesn't take time. Well just repeating what I wrote in my previous comment. – Jimi Feb 19 '20 at 01:29
  • @Jimi: that's also part of the point I'm making. "IF" you make that change. Yes. – madreflection Feb 19 '20 at 01:30
  • 1
    @madreflection *There's nothing here that would benefit from being async*. Well, apparently there is. The *Be able to chain multiple calls* part is what needs to be *discussed*, eventually. What do you chain if you're updating the same Controls? Or, there's some other use of it? Maybe this procedure must be repeated ad libitum? Then, the MessageBox...? – Jimi Feb 19 '20 at 01:32
  • 1
    What I was saying is that nothing in that code is awaitable, except for the `Task.Run` that the OP is looking to remove. – madreflection Feb 19 '20 at 01:33
  • @Jimi Your approach is actually what I want here, as you put it `Dispatcher.Invoke()` is not needed here at all, so thanks! – ConfusedGuy Feb 19 '20 at 01:46

2 Answers2

5

You would mark your method as async, await the Task.Run so the continuations run on the UI, also leaving only the long running (seemingly CPU bound) job within it

private async void btnAction_Click(object sender, RoutedEventArgs e)
{
   btnAction.IsEnabled = false;
   txtResult.Text = "";       
   lblStatus.Text = "Wait...";

   string input = query.Text;

   // calling a method that takes a long time (>3s) to finish and return
   var attempt =  await Task.Run(() => someLibrary.doSomethingWith(input));

   if (attempt.ContainsKey("success"))
   {
      if (attempt["success"] == true)
      {
         txtResult.Text = "Success! The door is: " + (attempt["is_open"] ? "open" : "closed");
         lblStatus.Text = "";
      }
      else
      {
         lblStatus.Text = "Error! The service says: " + attempt["errorMessage"];
      }
   }  
   else
   {
      MessageBox.Show("There was a problem getting results from web service.");
      lblStatus.Text = "";
   }

   btnAction.IsEnabled = true;

}

Update

To cancel the task, you would use a CancellationToken from am instance of CancellationTokenSource and pass that into Task.Run and also your long running method to check for IsCancellationRequested (if you can). You cancel by calling CancellationTokenSource.Cancel

Note you would likely want to wrap this in a try catch finally and catch on OperationCanceledException and place your button enabling code in the finally

TheGeneral
  • 79,002
  • 9
  • 103
  • 141
  • 1
    Perfect. 2 things I would add: **1. use `Task.Factory.StartNew` instead of `Task.Run`.** (`await Task.Factory.StartNew(...); UpdateUi(...); await Task.Factory.StartNew(...); UpdateUi(...); ...`) **2. disabling the button is 1st line! Always!** Trust me I can double click if not. ;) – Alb Feb 19 '20 at 01:37
  • @Alb why are we using `StartNew`? – TheGeneral Feb 19 '20 at 01:39
  • This is perfect in the sense that it removes the unnecessary call to `Dispatcher.Invoke()`! It also solves the issue of chaining calls since now I can `await` the `Task.Run()`. For the sake of completeness, is there a way to Cancel the running task (possibly throw an exception)? – ConfusedGuy Feb 19 '20 at 01:40
  • @MichaelRandall [look at this](https://stackoverflow.com/questions/38423472/what-is-the-difference-between-task-run-and-task-factory-startnew) – Alb Feb 19 '20 at 01:42
  • @Alb yeah ok to get it off the threadpool, i personally wouldn't worry unless it was needed, however thanks for the note – TheGeneral Feb 19 '20 at 01:46
  • @Alb 1. Looking into `Task.Factory.StartNew()` not sure what the difference is, also 2. Wow, that's interesting! I guess that makes sense. I'll make sure to always set the button to disabled on the first line from now on – ConfusedGuy Feb 19 '20 at 01:49
  • @MichaelRandall This is great -- although since I can't really check for `IsCancellationRequested` since `doSomethingWith` is being handled by `someLibrary`. Is there a way to force stop the task and just get an exception thrown? – ConfusedGuy Feb 19 '20 at 01:51
  • 1
    @ConfusedGuy you would use StartNew, it if you wanted to get the long running task off the thread-pool, its an older method with a lot more fine-grained hints to control its behaviour, i would personally just stick with the newer `Task.Run` for now, unless you having issues with thread-pool resources. To cancel the task, you pass the token in to `Task.Run` make sure you catch the exceptions though – TheGeneral Feb 19 '20 at 01:54
  • 1
    @ConfusedGuy yes, a `CancellationTokenSource` should be declared, then take its `.Token`. I would not let the task handle it but would pass directly to `doSomethingWith()` as 2nd parameter. Inside, I would write `token.ThrowIfCancellationRequested();` (the check necessary especially in loops). And since we there: you should know that **all await should be wrapped with try-finally** (or try-catch) because they don't bubble up to the topmost caller! Example: `try { await Task.Run(async () => await Task.Run(() => throw new Exception())); } catch {}` not caught in release mode and crashes your app – Alb Feb 19 '20 at 02:00
  • @ConfusedGuy if you have access to *SomeLibrary*, you should refactor the code to have a 2nd, `CancellationToken` parameter, since whenever you don't need it, you just pass in `CancellationToken.None`. – Alb Feb 19 '20 at 02:05
  • @Alb I see, but in this case, I can't check for cancellation within a loop in `doSomethingWith()`, as it's mostly a network operation that I don't have control over. Also, I can not pass the cancellation token to `doSomethingWith()` since I can't modify the source code of it. – ConfusedGuy Feb 19 '20 at 02:05
  • @ConfusedGuy Well, I know for sure, there is no `Task.Abort` implemented for Tasks, probably because `Thread.Abort` was way unstable to rely on (not Abort itself but what you put on that thread). – Alb Feb 19 '20 at 02:27
  • @ConfusedGuy You can make a 2nd (fake) task (that returns only when cancelled) as a rival for the main task and use `Task.WhenAny()`. But this just enables you to execute further on the main thread, does not stops the other Task. This only acceptable if you are closing the app (threads are shutting down anyway) or you can forget a network call. Because 1.: you may allow the user to re-launch something which still runs in the background. 2.: you lose robustness by missing `try { await ...; } catch {}` – Alb Feb 19 '20 at 02:28
  • @Alb: `Task.Run` is superior to `StartNew` for this scenario. – Stephen Cleary Feb 19 '20 at 03:12
2

The async modifier requires that the function return Task<T> (or void, in which case any await statements will be ignored). This means that using async and using Task.Run() are one and the same, the premise of your question doesn't make sense.

However, what I think you want to do is just use the async await syntax to avoid an explicit call to Task.Run().

Callbacks

Just create a function which returns a Task

async Task<T> Foo()

And then assign a variable var bar=await Foo();

Cancel a running task

Just use CancellationToken

CancellationTokenSource tokenSource = new CancellationTokenSource();

If you construct a task with two arguments, the second argument is a cancellation token:

var bar= new Task(action, tokenSource.Token)

This lets you use tokenSource.Cancel();

Relevant link: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken?view=netframework-4.8

Chaining Calls

If you don't require a defined order of execution, you can use Task.WhenAll(), otherwise you can either execute the next task inside the previous one or in the awaiting code.

Task.WhenAll(): https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.whenall?view=netframework-4.8#System_Threading_Tasks_Task_WhenAll_System_Collections_Generic_IEnumerable_System_Threading_Tasks_Task__

Ben
  • 2,200
  • 20
  • 30
  • Thank you for pointing me to the right direction. So, in this case, calling `tokenSource.Cancel()` will cancel `bar`? What will happen then, will an exception be thrown? Also, in this context I really do need to chain calls, as the results from the first call is an argument for the second. So I can't call both methods asynchronously here – ConfusedGuy Feb 19 '20 at 01:44