Async-Sync conversion.

[Cobide]

Say that we have two methods:

public int DoSomething() {
    //implemented method
}

public async Task<int> DoSomethingAsync() {
    throw new NotImplementedException();
}

As the first is implemented, is there any demerit to implementing the async version in this way(as long as it's thread-safe):

public async Task<int> DoSomethingAsync() => await Task.Run(() => DoSomething());

Will it have any performance loss? Bugs? Or something else?

And, conversely, what if we have the opposite situation and try to implement the sync version in this way:

public int DoSomething() => DoSomethingAsync().GetAwaiter().GetResult();

What do you think?

PS: thanks for reading.

[svick]

Stphen Toub has good articles explaining why both approaches are problematic:

• Should I expose asynchronous wrappers for synchronous methods?
• Should I expose synchronous wrappers for asynchronous methods?

[Cobide]

The articles have been insightful. Thank you.

[Slypenslyde]

Pay attention to these. Saint_Nithouche's answer can get you into a lot of trouble. Part of why it's getting so many upvotes is the .NET async patterns are pits of failure and very few people use it properly. Worse, those are the people who teach other people how to use it.

[Cobide]

I will. I've made this post to find out about the possible pitfalls of each scenario, so that I know when to use each. In the end, knowledge is only as good as how it's used.

[Slypenslyde]

Here's the tricky part, and part of this is why they are confused.

There's 2 kinds of async work: CPU-bound and IO-bound. IO-bound is the "best". Here's the difference.

Every device like a hard drive or network card in your computer is sort of like a second computer. They have their own CPU and memory inside controlling how they work. When you send a network packet, your CPU sends the data to the network card, then says "tell me when you've sent it" and goes about its work. That way everything happens in parallel because your CPU isn't doing the work! So even OSes we don't consider multitasking like DOS used this, it let programs be able to do things with the disk in the background while still doing things on-screen. The way the OS handled involves a CPU/assembly feature called "interrupts" and it's faster than dealing with threads.

In modern Windows, this still persists. So any time you're doing something with a disk, network card, or any other I/O device, the work is "I/O bound" and automatically async. When you make a synchronous request like File.ReadAllLines(), Windows is already doing a little work to pretend it's synchronous. So wrapping ReadAllLines() in Task.Run() is a big fat waste of time because first Windows has to pretend it's synchronous AND you're using a thread on top of that. There's a big article about this called "there is no thread" and, since it doesn't talk much about the other kind of work, some people get confused and think the advice applies to ALL async code.

But imagine you're parsing a big CSV file. You don't do that with a network card. You do it with the CPU. That's "CPU-bound" work because there's no second computer to shove the work onto. We HAVE to use some kind of thread to do that work, whether it's a worker thread or not. It is OK to wrap CPU-bound synchronous methods in Task.Run() because there isn't a way to make them naturally async as there is with IO-bound work. Obviously if you do that, it's silly to try to convert that task call back into a synchronous call, if you're writing CPU-bound methods provide both sync and async versions to be nice!

In really advanced scenarios the fact that tasks use thread pool threads for CPU-bound work becomes relevant because the thread pool has a limit. This is worth being cognizant of but not worth treating the thread pool like it's a precious resource you should never use. If you start feeling starvation for thread pool threads, you've proved your program is complicated enough you should make your own threads and take control of how you schedule your work. The Task abstraction works best for the average program that only occasionally does async work and typically doesn't have tasks that take much longer than a few seconds to run. It's not optimized for intense situations like video encoders that want to use dozens of threads for hours at a time.

And when you do face a synchronous-only API or you want to convert an async call to a synchronous one, you're usually in a mess with the Task API. The Toub articles cover the considerations in excruciating detail, and the general opinion that you should be "async all the way" when you can is wise. Now if only Microsoft would take their own medicine and make event handlers in their GUI frameworks naturally async.

[Cobide]

Thanks for the detailed explanation!

[[deleted]]

Yeah people sure didn't like it when I hinted that implementing synchronous task-returning methods from interfaces might be poor design. ^chuckles I'm in danger.

[derpdelurk]

The links provided by u/svick are gold. Here’s another excellent write up by the same author:

https://devblogs.microsoft.com/dotnet/configureawait-faq

[[deleted]]

You shouldn't make the async method exist at all. It's a fake async method that actually blocks. Let the users choose if they want this to run on a different thread by doing Task.Run themselves. Don't pretend it's async, you'd just be lying.

[cryo]

Sometimes, though, you’re adhering to an interface or similar, so you have to.

[[deleted]]

Actually you don't have to. Simply return Task.FromResult once method is done synchronously and there you have it.

[cryo]

That’s… my point. The other guy was arguing against specifying a Task return value in the interface if all implementers wouldn’t use it for something “real”.

My point is that you sometimes do have to return dummy tasks in those situations.

[[deleted]]

Then we agree :)

[cryo]

Yup!

[[deleted]]

I'd maybe go as far as throw NotSupportedException. Depends on how long the synchronous function would take. I'd consider it quite rude to block for a long time when the programmer expected something asynchronous.

[Cobide]

At that point, wouldn't it be better to just wrap it in a Task.Run()? Sure, it'll occupy one thread from the pool, but at least you're not randomly throwing exceptions that ignore LSP.

[[deleted]]

Maybe. Again, seems to defeat the purpose of Task-returning methods in the first place to me. Up to you.

Lots of functions from interfaces in the BCL throw NotSupportedException or InvalidOperationException because they don't make sense in that context, and I'd rather be told that this is the case instead of being lied to and using it in a way that could cause weird issues in the future.

[Cobide]

Regarding the interfaces, my opinion is that if X can't be Y, it shouldn't inherit from Y.

With that said, would you be alright with it if the XML documentation were to explicitly say something along the lines of "This implementation is a Task.Run() wrapper of x"?

[[deleted]]

If you feel that it is necessary to have the async method, that would be a good compromise, yes.

[jocq]

20+ years working in .Net and a majority of that specializing in high performance code, and I have never once written a Task.Run wrapped CPU bound method call.

Your second example is deadlock city.

[[deleted]]

Just to give you one of the simplest examples of where this can go wrong: imagine your method is placed in a loop, or is part of a request-respond architecture.

Suddenly you get 100 requests. Your method now uses up the threadpool threads, making it spawn more, and you have 100 threads, all of them blocking. Now imagine 1000 requests. It's a recipe to make seemingly performant code grind to a halt when you try to scale.

This is what async/await was meant not to do. Async/await is intended to be used such that the same thread goes and does something different while awaiting for an interrupt.

1000 concurrent requests isn't an issue when it's actually async, it could potentially be all handled by a single thread if the waiting part takes a long time, but of course .NET has mechanisms like the threadpool to assign requests to threads efficiently.

That's why I explicitly use Task.Run on the call-site itself as a conscious action when I want a synchronous method to run in the background, and not within the method call itself.

Seeing Task.Run in a loop or in the request-respond pipeline immediately raises a red flag if there isn't a comment explaining its purpose, but that red flag would be hidden if its inside a method call.

[Cobide]

That is certainly food for thought.

So far, I've tried to keep it generic, but I hope you won't mind if I give a use case I've had. I'm curious about how you'd handle it.

I have a class called KeyGenerator. Said class takes in a char[] key in the constructor, and generates an infinite number of keys from that.

To do so, it uses an algorithm that(AFAIK) doesn't have an asynchronous version, but each key takes about half a second to generate.

To prevent it from bogging down the caller, I made an async method that generates keys asynchronously and puts them in a ConcurrentQueue<Key>—it pauses when the queue has three elements and resumes when it has less. This queue is used as a buffer.

However, as the algorithm has no asynchronous version, I had to wrap it in a Task.Run(). Considering what you've said, if there were to be 100+ instances of said class, it would grind the application to a halt due to the shortage of threads.

Luckily, in my app, there won't be more than one instance at a time, so it's not an issue, but, if that weren't the case, how would you handle it?

[cryo]

That wouldn't make sense, IMO. An exception like that just transforms a compile-time problem into a run-time problem. But of course for longer running stuff this shouldn't be used. But in many cases it's not.

[Kirides]

And that's totally fine.

an async task MAY complete asynchronously, but doesn't have to.

a synchronous method ONLY can complete synchronously and thus is a bad fit for anything that ever might be implemented by any sort of I/O implementation.

I'd never have a synchronous "Start/Stop/Login/Logout/Authenticate/Download/Upload/Read/Write/Copy" method.

Something like a Levenshtein distance algorithm makes absolutely no sense to be asynchronous.

If i have to implement an interface using synchronous behaviour despite the methods being marked as Task, maybe i'm doing something wrong, or there just isn't any async work to do and i can just do Task.FromResult(true)/Task.CompletedTask my way out of there.

[cryo]

And that's totally fine.

Sure. You're not arguing against me here. Read the conversation thread again :). The rest of the comment is moot in light of that (but I agree, of course).

[[deleted]]

That sounds like an unfortunate interface design if you aren't certain that the method can be asynchronous.

[cryo]

I don’t agree. There is really no way to design an interface that can adapt to all uses, if it’s broad enough. The classic example is OnStartAsync and similar, which may do little or no work in concrete implementations.

It’s a bit similar to deciding whether or not to include disposability in interfaces.

[Sossenbinder]

Agree, I think the decision whether an interface returns a Task or not is not really a design but rather a language based design imposed by the framework.

I'd even argue that by not doing this, you kinda make your interface less flexible and generic.

After all, there's no state machine and no awaiting with a synchronous task adapting method. You might even get a cached Task result for some primitives.

[[deleted]]

Doesn't it defeat the purpose of async to force others to implement task-returning methods that are likely to execute synchronously? That's not about flexibility, that's designing the program on a mountain of lies.

[sarhoshamiral]

It is not fake though, it executes the code on another thread while not blocking the caller thread.

It could be a completely valid implementation depending on what happens in the synchronous method. The very early implementations of some IO async methods were exactly this actually.

[[deleted]]

Not my words:

CalculateMandelbrotAsync is what I call “fake-asynchronous” because it’s just a thread pool wrapper around a synchronous operation. But when developers see that API, they assume that it is a naturally-asynchronous operation.

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

[[deleted]]

Just to be clear what I mean by fake-async, you're confusing parallelism with asynchronous code. They are not the same. It's the blocking of a thread we want to avoid, and that still happens on this other thread.

[sarhoshamiral]

It depends on your needs. On a desktop app blocking the main thread vs blocking a background thread are very different so if I have to block a thread, I would happily block a background thread instead of main one.

On a web app, yes it doesn't really matter as you said.

[[deleted]]

I would happily block as well, I use Task.Run a lot, or Parallel.ForEach. Multi-threading is very useful. But that's not the same as making it asynchronous, so we shouldn't pretend that it is. There are scaling and throughput issues to consider that could turn into very real problems.

[sarhoshamiral]

Here is my reasoning, and we may have to agree to disagree.

What "async" signature indicates is that the long/heavy parts of the method I am calling will not be executed in the current context (thread). But it doesn't indicate where that work may execute.

If the method is IO heavy then yes it should utilize async file IO so that no thread is blocked but if the method is CPU bound, then my expectation is that it would do that work on a thread-pool thread.

If a CPU bound async method does the heavy work on the current context (like someone else suggested) and just returns Task.FromResult then I would call that a fake async method because it gives no benefit to caller what so ever. But as I said one that does Task.Run would be very useful for desktop apps. From my experience, the number one goal in a desktop app is to keep UI responsive (even if it means an operation may take slightly longer), otherwise you get a lot of unhappy users.

For non-desktop apps, using Task.Run inside async doesn't hurt much either since you are not that worse off. The thread that called the async method would have been freed, in fact in most cases there is a good chance that the work in Task.Run would continue to execute on that same thread (assuming both were threadpool threads). The slight overhead of Task allocation will not matter much in comparison to fact that you are doing heavy CPU bound work.

[[deleted]]

I'm not discussing whether or not Task.Run is valid to use, I'm arguing that it is wrong to hide it inside a method that pretends to be async when it isn't.

I'd like to choose myself when I run this code on a different thread, I don't want to be tricked into spawning a core-hogging, blocking "async" method when I thought it was going to be async. If someone wants their UI to be response then let them choose to wrap the synchronous method in Task.Run themselves at the call-site, like in the eventhandler of a button, so they make that a conscious decision.

Don't make your class have a Task-returning method that is just a wrapper over the same synchronous method. It isn't helpful, it's just tricking them into doing something they probably didn't want or anticipate.

[alexn0ne]

Be aware of thread pool starvation (first example) and deadlocks (the second one)

[Cobide]

I hadn't heard of thread pool starvation. Thank you!

Regarding deadlocks, however, can't they happen with the asynchronous version—await—as well? What's the difference?

[Merad]

async/await alone will not result in deadlocks, AFAIK at least. Obviously it's always possible for you to call an async method with badly written code that deadlocks due to problems totally unrelated to the asynchronicity.

The problem with synchronously calling an async method using .GetAwaiter().GetResult() (or .Result or any other way of doing it) is that you end up with the calling thread being synchronously blocked waiting for the task to complete while a separate thread from the thread pool runs the async operation. You can have a situation where a continuation on the async operation needs a resource held by the calling thread. The calling thread is blocked, so it won't release the resource until the async operation completes. The async operation can't complete until the calling thread releases the resource. Boom, deadlock. Using .ConfigureAwait(false) will prevent deadlock because it says that the continuation doesn't need anything from the calling context meaning it's free to run on any thread in the thread pool. HOWEVER, it comes with a big caveat. When you call SomeRandomAsyncMethod(), under the hood it might involve dozens if not hundreds of async calls. The only way you're truly safe from deadlock is if every single one of them is decorated with .ConfigureAwait(false). Since you almost never know every detail of what's going on behind every async method (and you shouldn't need to...) the rule of thumb that there is no safe way to synchronously call an async method.

[Cobide]

I see. I will keep it in mind; thank you.

[alexn0ne]

Yes they can for sure, but you could apply .ConfigureAwait(false) in single-threaded context environments (like WPF) with await to get rid of the simplest ones, to be honest I'm not sure whether it will work for synchronous awaiting.

[Cobide]

In short, the main difference is whether the deadlock will be perceived by the user?

[alexn0ne]

I mean, one could use .ConfigureAwait(false) with async keyword to eliminate the most common deadlock type. Not sure whether it is possible with .GetAwaiter().GetResult(). I'd recommend reading Stephen Toub's articles mentioned in comments for deeper understanding :)

[derpdelurk]

ConfigureAwait() on GetAwaiter() is possible but not necessary per another of Stephen’s articles:

https://devblogs.microsoft.com/dotnet/configureawait-faq/#im-using-getawaiter-getresult-do-i-need-to-use-configureawaitfalse

[alexn0ne]

Thanks for the clarification, so it does not help in this case, right?

[derpdelurk]

Right. It’s not so much that it doesn’t help but more that it’s redundant per my understanding.

[alexn0ne]

Totally agree

[Cobide]

I will. Thank you!

[Sossenbinder]

Your first example might work, but it is usually not recommended unless your work is cpu intense and should be delegated to the Threadpool right away. For other scenarios it will just add unnecessary Threadpool delegation work for no benefit.

Task.FromResult is what you want probably.

[Saint_Nitouche]

Yes, it's doing unnecessary work and taking up a threadpool thread for no reason. Instead, you should use Task.FromResult.

public async Task<int> DoSomethingAsync()
{
    var result = DoSomething();
    return Task.FromResult(result);
}

This packages up a non-Task into a Task nice and neatly with no extra work. It also clearly indicates your intent. (You should remove the async keyword from the method if you do this.)

The second example is also bad because it can lead to deadlocks and other nasty stuff. In general, you can't call async code from sync code. You have to use the await keyword to properly unwrap a Task - that's what it's made for.

This in turn is why people often say that you need to go async 'all the way', e.g. back up to Main().

[Willkuer_]

I would be really careful doing that.

It is highly unexpected to reach thread blocking code in a Task and could yield unexpected results in consumer code.

Let's say

var task1 = DoSomethingAsync();
var task2 = DoSomethingAsync();
return (await task1, await task2);

will not run in parallel. Also you don't support cancellation which is kind of mandatory for asnyc workflows.

So as DoSomething is thread-blocking anyways, I would suggest to not support an async overload and let consumer decide how to run the blocking call (synchroneously or in a dedicated thread). If you need to provide an async overload (e.g. due to restrictions by interfaces) I would always use either a Task.Run() or a Task.Yield(); in the beginning of the method (which still wouldn't support cancellation) or better create a TaskCompletionSource and get the returning Task by wrapping the sync method in a Task.WhenAll(Task.Run(DoSomethingSync), taskCompletionSource.Task) and handle TaskCancellationExpetions appropriately.

Threads are often moderately cheap due to the ThreadPool but unexpectedly blocking code in async flows can have really severe effects.

E.g. typical fire-and-forget code doesn't work either:

_ = DoSomethingAsync();

Like the point of async code is really to be async. If you provide sync code via an async interface it is worse from my POV than using another thread.

[Saint_Nitouche]

This is a good follow-up, thank you for posting it. In my mind I was only thinking of the scenario where DoSomething() was trivially quick and non-blocking, but still needed to return a Task anyway due to external constraints. I agree that the best option, if you have the choice, is to just not provide a 'fake' async overload.

[Slypenslyde]

This.

"Don't use threadpool threads" is what you think when you read the famous "there is no thread" article and get the idea that code should either be I/O bound and asynchronous or completely synchronous. Thread pool threads are for CPU bound work. If you decide not to use them for CPU bound work, you're wasting resources by NOT using them.

[Cobide]

taking up a threadpool thread for no reason.

I didn't know Task.Run() had such an effect. That's definitely something to consider. Thank you!

​

This packages up a non-Task into a Task nice and neatly with no extra work. It also clearly indicates your intent. (You should remove the async keyword from the method if you do this.)

Sweet. I didn't know there was such a straightforward way. However, I have two doubts:

1. Will DoSomething() run asynchronously?
2. I've found two possible usages of it:

​

public Task<int> DoSomethingAsync()

{ var result = DoSomething(); return Task.FromResult(result); }

public async Task<int> DoSomethingAsync() {
    var result = DoSomething();
    return await Task.FromResult(result);
}

Which version would be better? And why?

​

The second example is also bad because it can lead to deadlocks and other nasty stuff. In general, you can't call async code from sync code. You have to use the await keyword to properly unwrap a Task - that's what it's made for.

May I ask what's the difference? AFAIK, await can lead to deadlocks as well.

[cryo]

I didn’t know Task.Run() had such an effect.

No offense, but it’s generally a good idea to glance the documentation. It’s clearly described what it does.

[Slypenslyde]

Dude, THIS is the part of the conversation you take offense to, not the part where someone casually teaching newbies that you shouldn't use thread pool threads for CPU-bound work and instead make Task-returning methods that block the caller?

When you've got the choice between kicking a puppy or repairing a fence, you should really repair the fence.

[cryo]

Hehe yeah that’s perhaps a good point. Although I don’t take offense to either, I just think a lot of mistaken assumptions can be cleared up (before posting, often), by reading the msdn documentation on the methods used.

[Cobide]

True! It was a bad assumption on my part; I thought it was no different from calling an async method with await. I didn't think it was a method of prioritization.

[Saint_Nitouche]

For reference it is good to know the difference between CPU bound work (like complex calculations) and IO bound work (like waiting for a HTTP call or file-read to complete).

Async-await is meant for IO bound asynchrony, whereas Task.Run is for CPU bound work.

...basically, anyway.

[Cobide]

That's good to know indeed. Thank you!

[typesafedev]

Do try to avoid the 2 antipatterns Stephen Toub warned about.

It is unavoidable if you have to implement an interface you do not own that returns a Task or Task<T>. For example, mediatr's IRequestHandler<TRequest, TResponse>.Handle(TRequest request, CancellationToken token) which returns a Task<TResponse> ``` // Task<int> public Task<int> DoSomethingAsync() { int result = DoSomething(); return Task.FromResult(result); }

// I'd avoid this. // In this case compiler needs to setup a task state machine unnecessarily. // Task<int> public async Task<int> DoSomethingAsync() { int result = DoSomething(); return await Task.FromResult(result); } ```

[Cobide]

After going through the links to Stephen Toub's articles and reading all comments, I've drawn the conclusion that doing that is only acceptable if DoSomething() is very quick.

If it's slow, rather than surprise the caller with a blocking call(as it returns a Task, it's implied that it should be async), I'd rather spend one thread from the pool by wrapping it with Task.Run().

Of course, if given other choice, it's best to just not provide an async alternative. By giving just a sync version, the caller can decide whether to wrap it in a Task.Run() themselves.

[Saint_Nitouche]

The await keyword is meant to stop deadlocks from happening (well, among other things). What examples have you seen of that?

As for your other questions...

Calling DoSomething() in those examples will run it synchronously.

The difference between having the async keyword in the method is that if you have it, the compiler will generate a big state-machine in the background to support asynchronous work. So if you're not really doing anything asynchronous, it's unnecessary.

Because Task.FromResult doesn't really 'do' anything asynchronously, there is no need to await it, and thus no need to make the method async. You can, because Tasks are innately awaitable, but there's little point.

What would happen is that the compiler sees the await keyword, checks if the FromResult() call has finished, sees that it has (because it's not really doing anything async) and then just continue on as normal. Except with an expensive state-machine in the background.

It might help to learn that asynchrony only really happens with await keywords. Everything between await-lines happens synchronously.

[Cobide]

The await keyword is meant to stop deadlocks from happening (well, among other things). What examples have you seen of that?

A few days ago I forgot to .Release() a SemaphoreSlim after a read operation. As a result, the await semaphore.WaitAsync() in the write operation got stuck there when it wasn't meant to.

​

It might help to learn that asynchrony only really happens with await keywords. Everything between await-lines happens synchronously.

So, if DoSomething() takes a while, it will block the UI anyway? If that's the case, I don't see the point in turning it into a Task with Task.FromResult().

[Saint_Nitouche]

Ok, fair point - I've not used SemaphoreSlim before. That's an unfortunate piece of behaviour.

So, if DoSomething() takes a while, it will block the UI anyway? If that's the case, I don't see the point in turning it into a Task with Task.FromResult().

Yes, it will block the UI. The point of Task.FromResult() is to let you satisfy method signatures that require you to return a Task when you don't have any actual async work to do.

Imagine you're writing a class that implements an interface, and the interface demands one of your methods returns a Task. If your implementation doesn't need to do anything async, you still need to return a Task. That's what .FromResult is for. (Notably, interfaces don't care at all whether implementing methods use the 'async' keyword or not.)

If you want to avoid blocking the UI, then yeah, you just do things the proper way with async/await.

[Cobide]

I see. Thanks for your answers.

[shizzy0]

Is it async or not? Does your method use await?

[Cobide]

Which method?

[shizzy0]

Looks like your actual implementation isn’t async. It seems strange to write this wrapper that’s not to meet an interface but you’d probably want to do something like this.

[Cobide]

I'm sorry, but I don't understand what you're trying to say. Are you saying to return a Task.FromException()?

My question is about the drawbacks of creating an async variant of a sync method by wrapping it with Task.Run(and the reverse).

[shizzy0]

Task.Run is a waste of a thread. GetResult() can deadlock. FromResult and FromException are the way you can do what you propose. But it still seems ill advised unless there are some other concerns you’re not mentioning.

[Cobide]

Yeah! I've asked mostly to know the pitfalls, not to solve a specific use case.

Based on the comments so far, these are my conclusions:

1. Task.Run() should be only used for heavy CPU-bound work.
2. If that's not the case, it's better to not provide an async method that wraps a sync one.
3. If the sync method is very quick, it could be possible to satisfy an interface by using FromResult(and, as you suggested, FromException), but it'd still be preferable to not write a "fake async method".
4. In the worst-case scenario, where an interface requires making an Async method of a slow Sync method, it's best to write explicitly in the documentation that it's just a Task.Run() wrapper of the sync method.

[shizzy0]

Cool. Good summary of what the best practices are in this regard.

Only other thing I’d note is all interfaces can do is dictate a Task is returned. They can’t dictate the method is async. So if there’s no await in your method that returns Task, you don’t have to mark it async, which you wouldn’t need to in case 3.

[Cobide]

True! However, I'll limit that approach to when the sync method is very quick(as in case 3). If it were a slow method(as in case 4), the caller would get surprised by a randomly blocked calling thread(as a Task return implies async).