27

I thought I understood async/await and Task.Run() quite well until I came upon this issue:

I'm programming a Xamarin.Android app using a RecyclerView with a ViewAdapter. In my OnBindViewHolder Method, I tried to async load some images

public override void OnBindViewHolder(RecyclerView.ViewHolder holder, int position)
{
    // Some logic here

    Task.Run(() => LoadImage(postInfo, holder, imageView).ConfigureAwait(false)); 
}

Then, in my LoadImage function I did something like:

private async Task LoadImage(PostInfo postInfo, RecyclerView.ViewHolder holder, ImageView imageView)
{                
    var image = await loadImageAsync((Guid)postInfo.User.AvatarID, EImageSize.Small).ConfigureAwait(false);
    var byteArray = await image.ReadAsByteArrayAsync().ConfigureAwait(false);

    if(byteArray.Length == 0)
    {
        return;
    }

    var bitmap = await GetBitmapAsync(byteArray).ConfigureAwait(false);

    imageView.SetImageBitmap(bitmap);
    postInfo.User.AvatarImage = bitmap;
}

That pieces of code worked. But why?

What I've learned, after configure await is set to false, the code doesn't run in the SynchronizationContext (which is the UI thread).

If I make the OnBindViewHolder method async and use await instead of Task.Run, the code crashes on

imageView.SetImageBitmap(bitmap);

Saying that it's not in the UI thread, which makes totally sense to me.

So why does the async/await code crash while the Task.Run() doesn't?

Update: Answer

Since the Task.Run was not awaited, the thrown exception was not shown. If I awaitet the Task.Run, there was the error i expected. Further explanations are found in the answers below.

Tobias von Falkenhayn
  • 1,355
  • 5
  • 26
  • 59
  • 2
    your code does have a crash you're just ignoring it. Async-> Void is Fire and forget which is bad you should be returning a task – johnny 5 Apr 26 '18 at 18:04
  • 1
    @johnny5 no it's not. its called on an UI event, where async void is totally "legal". – Tobias von Falkenhayn Apr 26 '18 at 18:15
  • Oops, didn't realize you were calling an event, I was expecting EventArgs, void is legal, but it's still fire and forget, nothing will catch the exceptions – johnny 5 Apr 26 '18 at 18:23
  • 1
    johnny 5 is right, wrap `imageView.SetImageBitmap(bitmap);` in a try/catch block and you'll find the same `ex.Message`. This [post](https://stackoverflow.com/a/5383408) might be of interest (note the quote). – Funk Apr 27 '18 at 19:10
  • Add `Debug.WriteLine($"thread: {System.Threading.Thread.Currentthread.Managedthreadid}")` before and after each `await`. What's the output? – noseratio May 01 '18 at 01:43

4 Answers4

14

It's as simple as you not awaiting the Task.Run, so the exception gets eaten and not returned to the call site of Task.Run.

Add "await" in front of the Task.Run, and you'll get the exception.

This will not crash your application:

private void button1_Click(object sender, EventArgs e)
{
    Task.Run(() => { throw new Exception("Hello");});
}

This however will crash your application:

private async void button1_Click(object sender, EventArgs e)
{
   await Task.Run(() => { throw new Exception("Hello");});
}
Robin
  • 616
  • 3
  • 9
13

Task.Run() and the UI thread should be used for a different purpose:

  • Task.Run() should be used for CPU-bound methods.
  • UI-Thread should be used for UI related methods.

By moving your code into Task.Run(), you avoid the UI thread from being blocked. This may solve your issue, but it's not best practice because it's bad for your performance. Task.Run() blocks a thread in the thread pool.

What you should do instead is to call your UI related method on the UI thread. In Xamarin, you can run stuff on the UI thread by using Device.BeginInvokeOnMainThread():

// async is only needed if you need to run asynchronous code on the UI thread
Device.BeginInvokeOnMainThread(async () =>
{
    await LoadImage(postInfo, holder, imageView).ConfigureAwait(false)
});

The reason why it's working even if you don't explicitly call it on the UI thread is probably because Xamarin somehow detects that it's something that should run on the UI thread and shifts this work to the UI thread.

Here are some useful articles by Stephen Cleary which helped me to write this answer and which will help you to further understand asynchronous code:

https://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-dont-use.html https://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-using.html

Dennis Schröer
  • 2,392
  • 16
  • 46
  • Thanks for your answer, but like you said when Task.Run forces a function to run on the Thread Pool - not the UI Thread - shouldn't be calling any UI-Element on this function result in an error? Like i did: Task.Run(***update Image here***) <= why does this not fail? – Tobias von Falkenhayn Apr 26 '18 at 12:49
  • 1
    It may as well crash, but it seems like Xamarin is shifting some work to the UI thread. But it's important to understand that 1. it **could crash** the way you implemented it and 2. you are running code in a thread pool which should be called on the UI thread, which means performance losses for you. In the first link I provided in my answer, you can read more about the reasons why you lose performance by using `await Task.Run()`. – Dennis Schröer Apr 26 '18 at 13:11
  • So maybe the UI Thread is also part of the "Thread Pool" in Xamarin? I know that it's bad code but i was just wondering why it didn't crash when it should. – Tobias von Falkenhayn Apr 26 '18 at 13:34
  • Maybe, I don't know how exactly it's working. It's more important to understand that it could as well crash **and** that it's much better performance if you run it directly on the UI Thread. – Dennis Schröer Apr 27 '18 at 05:50
  • 1
    @DennisSchröer `Task.Run() blocks a whole thread pool.` It blocks a thread in the ThreadPool, not the entire pool of threads – SushiHangover Apr 30 '18 at 05:38
  • What makes you think that Xamarin delegates some action subroutine execution to a UI thread? – Leonid Vasilev May 03 '18 at 16:40
  • @LeonidVasilyev OP said that the code "worked" which would indicate it updated the UI as expected, which means it didn't crash. Like I said in my (downvoted) answer is my best guess as to why it didn't crash. It probably isn't guaranteed not to crash though, and definitely isn't a "good" way to do it. But it does explain the behavior of it not bombing out. – Matti Price May 03 '18 at 17:16
3

Probably UI access still throws UIKitThreadAccessException. You do not observe it because you do not use await keyword or Task.Wait() on a marker that Task.Run() returns. See Catch an exception thrown by an async method discussion on StackOverflow, MSDN documentation on the topic is a bit dated.

You can attach continuation to the marker that Task.Run() returns and inspect exceptions thrown inside an action passed:

Task marker = Task.Run(() => ...);
marker.ContinueWith(m =>
{
    if (!m.IsFaulted)
        return;

    // Marker Faulted status indicates unhandled exceptions. Observe them.
    AggregateException e = m.Exception;
});

In general, UI access from non UI thread may make an application unstable or crash it, but it isn't guaranteed.

For more information check How to handle Task.Run Exception, Android - Issue with async tasks discussions on StackOverflow, The meaning of TaskStatus article by Stephen Toub and Working with the UI Thread article on Microsoft Docs.

Leonid Vasilev
  • 11,910
  • 4
  • 36
  • 50
-2

Task.Run is queuing LoadImage to execute the async process on the thread pool with ConfigureAwait(false). The task that LoadImage is returning is NOT awaited though and I believe that is the important part here.

So the results of Task.Run is that it immediately returns a Task<Task>, but the outer task does not have ConfigureAwait(false) set, so the whole thing resolves on the main thread instead.

If you change your code to

Task.Run(async () => await LoadImage(postInfo, holder, imageView).ConfigureAwait(false)); 

I'm expecting you to hit the error that the thread isn't running on the UI thread.

Matti Price
  • 3,351
  • 15
  • 28
  • Sorry that was a mistake, the OnBindViewHolder is a Non-Synchronous method. It must be void cause its an Event. Doesn't Task.Run() marshall the delegate to a thread within the threadpool? – Tobias von Falkenhayn Apr 24 '18 at 14:48
  • 4
    Using Task.Run doesn't make an async operation synchronous. It just runs a delegate in a thread pool thread. Nothing here explains why the use of a UI element doesn't fail when using `Task.Run`. Additionally the event handler can't return a task, because it's an event handler. – Servy Apr 24 '18 at 14:50
  • @Servy is right, I misspoke, I need to edit that in a bit, typing to quick before a meeting – Matti Price Apr 24 '18 at 15:08
  • 1
    Everything after the first sentence is wrong. Adding that await in there changes literally nothing about how the code behaves, and the only code that the OP has shown that ever runs on the UI thread is the code that schedules an operation to run in a thread pool thread.. – Servy Apr 24 '18 at 15:51
  • And yet, the OP clearly stated that it was working with Task.Run so obviously that code is in fact running on the UI thread. Feel free to provide another answer as to why though @Servy – Matti Price Apr 24 '18 at 15:58
  • 1
    Not necessarily. It's possible that the operation doesn't always successfully fail when it's not run on the UI thread, or the type they're using could, under certain circumstances, marshal to the UI thread when performing that operation, or other possibilities. I'm not familiar enough with xamarin to know which of those things is going on. What I can say is that adding the `async` and `await` as you have changes nothing about how the code runs, nor does it run it on the UI thread. – Servy Apr 24 '18 at 16:00
  • https://stackoverflow.com/questions/33026865/non-awaited-async-methods-run-on-the-ui-thread – Matti Price Apr 24 '18 at 16:01
  • @Servy adding async/await wasn't intended to run on the UI thread which is why I said I would expect it to actually throw the error the OP is expecting. What is likely happening is that since the async call inside the Task.Run is not being awaited, the ConfigureAwait is not being respected. Because no other work is being done in the the main UI thread, it is considered free, and picks up the Task.Run queued work to execute. The result is that the code is executed ont he main thread. Adding async/await as I showed I believe would change that behavior and throw the error that the OP expected – Matti Price Apr 24 '18 at 16:30
  • `Task.Run` specifically queues work in the thread pool. It doesn't just randomly choose to run the code in the UI thread, even if the UI thread is free. You're correct that the `ConfigureAwait` in the Task.Run call doesn't do anything and can be removed. Adding `async` and `await` to the lambda just means that the code that runs *after* `LoadImage` (i.e. nothing at all) will actually care about how the await is configured, so you're just stacking more superfluous code on top of existing superfluous code. – Servy Apr 24 '18 at 16:42
  • I believe adding the await inside the lambda will trigger the exception that the OP expects (about it not running on the UI thread) and explain the behavior they are seeing. If you have another explanation for what the OP is seeing, I'm all ears. – Matti Price Apr 24 '18 at 16:45
  • So the real question fot me is: Why does imageView.SetBitmapInage NOT crash when called inside the Task.Run? Since it marshalls the code execution to a thread in the threadpool, and the imageview sits on the UI thread, it should crash. – Tobias von Falkenhayn Apr 24 '18 at 17:31
  • That's what I was getting at in my answer. It is being assigned to run on the thread pool, but since the UI thread isn't busy, the UI thread is the one picking up the work. That's what I'm pretty sure is happening.. I think it is more an oddity than correct code. Try that async/await Task.Run I posted though. I'm curious if that will error or not. – Matti Price Apr 24 '18 at 17:41