In this situation, does Task.Run is useful at all?
Knowing that FireAndForgetAsync is not too important (it might raise exception, but we don't really care)

public async Task<IActionResult> DoSomething()
{
      await repo.SaveDataAsync();
      _ = Task.Run(FireAndForgetAsync);
}

public static async Task FireAndForgetAsync()
{
     await repo.SaveOtherDataAsync();
}

Or

public async Task<IActionResult> DoSomething()
{
      await repo.SaveDataAsync();
      _ = FireAndForgetAsync();
}

public static async Task FireAndForgetAsync()
{
     await repo.SaveOtherDataAsync();
}

You seems to use Task.Run everywhere in examples, but is it really needed?

Anyone aware of a Roslyn-based analyzer for the guidance “Prefer async/await over directly returning Task”?

/cc @p-schuler

Avoid using Task.Result and Task.Wait:

ASP.NET Core Blazor does appear to use a synchronization context, being the RendererSynchronizationContext](https://source.dot.net/#Microsoft.AspNetCore.Components/Rendering/RendererSynchronizationContext.cs,11).

Hi, what's currently the recommended way to run async application initialization in asp.net core and .net core apps based on IGenericHost?
Wouldn't it be a nice idea to integrate async initialization into WebHost and GenericHost themselves? As an example we could use https://github.com/thomaslevesque/AspNetCore.AsyncInitialization/
What I am trying to achieve with that is:

• to get access to DI container in async initializer
• to run initialization logic (bootstrapping) before executing the Host itself

@davidfowl, Constructors section explains getting around by using static factory pattern.

But, what happens when I cannot call this static method to create an instance because the instance is created by container when IService is dependency injected via constructor into other class?

Basically, what is the best practice for calling Async method in within constructor for these 2 different scenarios?

• Need to wait for the Async method in ctor to finish before moving forward
• Fire and forget to loading some collections asyncronously
  • I guess this will be similar to Async void i.e. Task.Run(BackgroundOperationAsync);?

As I understand the last line always should fail because of context.Response.WriteAsync("Hello "); nothing to do with next() call here, right?

There are legacy classes that use events and that need to use new async calls in their handlers. What is the suggested way of running async calls synchronously there?

Hi @davidfowl,

Trying to get rid of Task.Result in our code and follow your approach in AsyncGuidance.md#concurrentdictionarygetoradd by storing the Task instead of the actual value.
In our tests, it causes issue when the task fails (e.g. HTTP Exception) but the task obviously still gets added to the cache...

Any recommendations here?

Thanks,
Patrick

There's a note regarding avoidance of TaskCreationOptions.LongRunning:

💡 NOTE:Task.Factory.StartNew has an option TaskCreationOptions.LongRunning that under the covers creates a new thread and returns a Task that represents the execution. Using this properly requires several non-obvious parameters to be passed in to get the right behavior on all platforms.

The code sample then proceeds to use:

var thread = new Thread(ProcessQueue) 
    {
        // This is important as it allows the process to exit while this thread is running
        IsBackground = true
    };
    thread.Start();

The bit I'm missing is how we go from "API is awkward" to "look, you're better off with Thread.Start"; is it for reasons of cumbersome syntax or is it because it's outright better and/or TaskCreationOptions.LongRunning should all-but be deprecated that we go to the lengths of actually creating a thread explicitly? I propose to use the answer to either
a) make a PR to switch the example to use Task.Factory.StartNew, or
b) clarify the note to explicitly indicate that Task.Factory.StartNew should be avoided, and (Thread {IsBackground = true}).Start() is the recommended alternative

For instance in Serilog.Sinks.Async, we create the thread using TaskCreationOptions.LongRunning

I would be interested in some guidance how to handle events (e.g. WinForms) where you need to process something and put a value back to the event args (e.g. Cancel = true).

Hi, maybe this is worth mentioning. Basically, the FileOptions.Asynchronous flag should be passed when creating a FileStream (e.g. with File.Create), otherwise the actual I/O operations are performed synchronously. More info.

I'm sorry, I don't have the answer, I'd just like to note that this is a common problem.

I wouldn't agree on the statement that async void is evil. There are definitely exceptions where you're fine to use async void.
Moreover I'd really appreciate more details on how async void can cause process crashes when exceptions occur.

To summarize this first guideline, you should prefer async Task to async void. Async Task methods enable easier error-handling, composability and testability. The exception to this guideline is asynchronous event handlers, which must return void. This exception includes methods that are logically event handlers even if they’re not literally event handlers (for example, ICommand.Execute implementations).

from
https://docs.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming

Not sure what a better header text would be, but
"❌ BAD This example uses Task.Result to get the connection in the constructor. This could lead to thread-pool starvation and deadlocks." is copy pasted in this section:
https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#windowsidentityrunimpersonated

from this section:
https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#constructors

and the context doesn't make sense.

Hi David,

Hope you are doing well!

Following your (awesome) async guidance, I'm having a strange behavior while testing a long-running message consumer with Task.Run vs Background Thread.

Code sample:

        public async Task MainAsync(string[] args)
        {
            var backgroundThread = new Thread(Consume)
            {
                IsBackground = true
            };

            backgroundThread.Start(); //CPU - NOK


            await Task.Run(Consume); //CPU - OK
        }

        private void Consume()
        {
            //while loop (sync) consuming messages from Kafka
        }

Test conditions:

• K8s cluster
• 3 pods
• Kafka cluster
• 3 partitions by topic

Background thread

Task.Run

As we can see, pods running with background thread are struggling the CPU.

Do you have any thoughts why it happens?

Code: https://github.com/bmwhite20/cpu-workload

Best,
Bruno

IT might be less bad, but good???

The Guidance.md doc mentions using AsyncLazy<T>. I know Roslyn defines this internally, and the vs-threading library offers one publicly. Which one (or a new one?) are you proposing customers should use?

Could be possible to include the links in Guidance.md file into the README.md file so when visiting the repository navigation to each section becomes easier?

Reading the WindowsIdentity.RunImpersonated examples in the AsyncGuidance, there appear to be two errors:

1. Each of the delegates are defined as taking a context parameter. However, RunImpersonated does not take any delegate with parameters. It only takes Action or Func<T> (Func<Task> or Func<Task<T>> for RunImpersonatedAsync), so why is this context parameter in the examples?
2. The first BAD example appears to be incorrect. It states: "This example tries to execute the query asynchronously, and then wait for it outside of the call to RunImpersonated. This will throw because the query might be executing outside of the impersonation context." However, that example code doesn't actually have any functional problem. In fact, it is semantically equivalent to the first GOOD example, which we can confirm by seeing that the supposedly BAD example matches the runtime implementation of the RunImpersonated method that is used in the first GOOD example.

There is no Contributors section in readme file .
As we know Contributions are what make the open-source community such an amazing place to learn, inspire, and create.
The Contributors section in a README.md file is important as it acknowledges and gives credit to those who have contributed to a project, fosters community and collaboration, adds transparency and accountability, and helps document the project's history for current and future maintainers. It also serves as a form of recognition, motivating contributors to continue their efforts.

cc @pakrym

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/d319a43874ca5d25c5e12dcbba8f8bc71a81e102/AsyncGuidance.md#windowsidentityrunimpersonated

https://github.com/dotnet/corefx/issues/24977

Arrived to this great repository after watching your great talk on scalability on youtube.

The situation here:
https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#avoid-using-taskrun-for-long-running-work-that-blocks-the-thread
is relevant for us (we have ~10 such workers which keep synchronizing some database tables, each worker for a different table, into memory cache so that when http requests come we can handle them quickly using the data from the memory cache instead of querying the database) however we run some async calls which don't have alternative sync calls from that background worker and the example shown is sync so it is unclear how to correctly start async code in that new background thread without going back to using .Wait or .Result?

Maybe in such cases we need to use Task.Factory.StartNew with TaskCreationOptions.LongRunning which allows to smoothly run async task? and if so can you please provide an example of how to correctly do it because I saw that you wrote that its error prone if to pass wrong parameters

Thanks!

If we use async/await in the A note about WebClient section of code then it will aligned with Prefer async/await over directly returning Task.

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blame/d319a43874ca5d25c5e12dcbba8f8bc71a81e102/AspNetCoreGuidance.md#L187-L200

When buffering you should still check Response.HasStarted before setting response headers (like the prior example. Buffers can be Flushed to start the response early.

We often see code like that

public async Task<IActionResult> Get()
{
    await Task.Delay(1000);
    return View();
}

Is it a good practice to add the CancellationToken parameter to actions when it goes async and pass this parameter to async methods ?

public async Task<IActionResult> Get(int id, CancellationToken cancellationToken)
{
    await Task.Delay(1000, cancellationToken);
    return View();
}

I have to disagree on your statement:

Use of async void in ASP.NET Core applications is ALWAYS bad. Avoid it, never do it.

I know what the motivation is but the expression is wrong. async void has use case(s?) and should be used only if needed be. Such, maybe only, example is async event handler.

As discussed on twitter, I'm opening this issue about covering the usage of EF Query Async operations.

Is it wise to say that we should always use Async methods for query when it comes to EntityFramework Core (according to the ef core docs I would say it is but since you don't mention it) https://docs.microsoft.com/en-us/ef/core/querying/async

For something like the following:

public class QueueProcessor
{
    private readonly BlockingCollection _messageQueue = new BlockingCollection();
    
    public void StartProcessing()
    {
        //Start background processing here. What's the best approach?
    }

    public void Enqueue(Message message)
    {
        _messageQueue.Add(message);
    }
    
    private async Task ProcessQueue() //notice the async Task here
    {
        foreach (var item in _messageQueue.GetConsumingEnumerable())
        {
             await ProcessItem(item).ConfigureAwait(false);
        }
    }
    
    private async Task ProcessItem(Message message) { }
}

What is the best approach of executing the long running work in the background? I have looked everywhere and I've found a lot of contradicting advice on this.

I think at minimum, this advice should be clarified in two ways: example improved to actually show how a deadlock or other adversity can happen (example just shows when it'll run on the same thread, not how one gets in trouble when that happens; I don't have one in C# of the top of my head but see footnote²); and highlight performance trade-offs when this flag is used, as it does come at a penalty(!).

In fact, I spent couple of days thinking about it, and the more I'm thinking about it, the more I'm starting to lean toward that it may actually be a counter-productive (ok, bad 😄, forgive me 😹 ) advice. Here's my rationale.

There are two parts to my argument:

1. There's a performance cost associated with the TaskCreationOptions.RunContinuationsAsynchronously, and using it indiscriminately bears that cost.
2. I believe it doesn't solve all the cases, and still demands that care be taken in the continuation tasks; and is not a magic solution to all the problems. (Afterall, I think there's a reason that it's not the default option). In fact, it may be exacerbating them by virtue of masking (hiding) them, rather than fixing them

So, first, onto the performance part.

When a task is run in-line - that is, non-asynchronous continuation - it's run immediately as a direct function call. The cost of invoking that function is simply building the stack call frame for the function and making the execution jump to the first instruction of that function. And that has to happen regardless of inline or async continuation, and so inline execution is pretty much done at zero net cost.

Asynchronous continuation, however, is more involved. Instead of making that direct call, it is placed into a work queue. Another pool thread gets notified that there's available work, which then dequeues the work and only then calls the continuation function. So this trip through the queuing is the extra cost that is incurred with the TaskCreationOptions.RunContinuationsAsynchronously.

Below example is from a part of TPL code - not sure if that's the exact part that handles this or something similar for some particular variation, but I think that's illustrative:

https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/Threading/Tasks/RendezvousAwaitable.cs#L101-L118

The above makes it pretty clear: if no RunContinuationsAsynchronously is demanded, it's a direct call to the continuing action when it does c(). But if it does demand asynchronous continuation, it sends the action down the Task.Run's tunnels which "do a few things" (in fact, I was under impression that TPL used lock-less work-stealing queues, but I do see use of lock in scheduling code, which adds to penalty), and those "few things" add [often] unnecessary performance cost. "Often" as in if flag used indiscriminately whether it's needed to prevent deadlock or if there is no possibility of deadlock even without it.

As I understand it, most tasks that result in calling async functions or task-returning functions default to inline execution. E.g. if you call a task-function without await and that function never results in an operation that requires some resource completion later (e.g. you call Stream.ReadAsync() where data was already in the buffer and didn't require waiting for a pending read operation to complete), the function will complete and continue to run the remaining part of the caller on the same thread. That's efficient. If crossing every task boundary was done via pool roundtrip, it would be much less performant (e.g. every Stream.ReadAsync() call result to be forced through the pool so that non-io completion doesn't run inline).²

So making all TaskCompletionSources be async continuations may help reduce unsuspecting deadlocks or w/e issues due to poorly written caller, I suspect that accomplishes so at a substantial performance cost. This is where I think the advice is not complete in its recommendation, and is misleading.

The real issue is not that continuation may run inline! The real issue is that caller and/or continuation are not written properly to handle such a case! (Ergo, wrong solution at the wrong place.) Which brings me to the second argument.

You can't control all TaskCompletionSources, or rather that all TaskCompletionSources in a code path will use TaskCreationOptions.RunContinuationsAsynchronously configuration option. It's not just a matter of changing it all in one's code! All 3rd party libraries, including .NET Core, would have to all be configured the same with that option. If one instance of TaskCompletionSource in a library one doesn't control doesn't make use of this option, and the scenario is where it can cause the problem, then it can cause the problem b/c it may run inline. Thus, if one is diligent to always set TaskCreationOptions.RunContinuationsAsynchronously on ALL their TaskCompletionSources in their code, it seems they're still screwed in case continuation is called from 3rd party library's TaskCompletionSource that didn't set it. Back to SquareOne™!!! Except now you don't even suspect that that's an issue, because you've been diligent to have it run through the pool!

The problem with this solution, it seems to me, is that it requires the callee (i.e. code issuing TaskCompletionSource instance) to do something special to play well with a caller that it doesn't know. It's much safer, though, when the caller and/or continuation makes itself safe and hardened against potential inline execution of continuation, and takes that burden upon itself, IMHO.

I suspect these are at least part of the reason TPL defaults to inline - to use more performant operation so as not to penalize by default, and because such issue is better resolved at the consumer rather than the producer end. (And doing so is even easier than the C++ example in the footnotes: with a simple await Task.Yield();).

I may very well have missed something, or failed to consider, so very much welcome your thoughts on the above.

¹ I mentioned earlier that I don't have a good C# example with which to improve, but I do have C++. One codebase I'm currently working on actually has in-house written C++ asynchronous library modeled, to a degree, on .NET's TPL. It has a notion similar to a promise, which, when resolved, will run all lambdas that were added to be notified of the resolution. It doesn't even have a concept of running such flag to call such functions via a thread pool. Such burden is placed on consumer of API to make such determination, and handle accordingly. And so example when such thing is done to avoid deadlock/exception is when caller is holding a non-recursive (non-reentrant) lock on std::mutex while calling an asynchronous function. Since that function may or may not complete while holding a lock, the handler the explicitly moves further processing via a pool. But other code, when not in danger of inline mutex, would continue to run biz logic potentially inline since in those cases it's safe to do so, and is faster than always queueing to pool. So:

Promise A::DoLockedAsync();  // Must hold lock when calling.
Promise A::DoAsync();  // no need to hold lock to call.

void B::DoIt()
{
  std::lock<std::mutex> l(_mutex);

  A::DoLockedAsync().wait([](auto result) {
    // This may run inline and we're holding a lock, so queue to pool.
    ThreadPool.enqueue([result](){
      std::lock_guard<std::mutex> g(_mutex);
      ConsumeLocked(result) });
  });

  l.unlock();

  A::DoAsync().wait([](auto result) {
    // This may run inline and don't care if so.
    Consume(result) });
  });
}

... but I don't recall C# having non-recursive locks, and so, as I mentioned, can't seem to come up with another example.

² This problem, by the way, is not native to .NET's TPL. Most JavaScript libraries that issue promises have it where attaching a handler to an already resolved promise will fire that handler inline, rather than via its message pump. AFAIK, these libraries do not attempt to avoid firing inline (by use of setTimeout(0, c), for example) and leave it up to the consumer of the API to handle so appropriately.

Hi! Thanks for useful examples!

Let's say I've created a dedicated thread for some long running background work. Some part of the work involves connecting to some backend service using a ClientWebSocket, which has only async methods.
That way I have no options but .Wait() for the results, do I?

Hi David, example with AsyncLazy doesn't compile as AsyncLazy is not awaitable.

To fix that

       var person = await _cache.GetOrAdd(id, (key) => new AsyncLazy<Person>(() => db.People.FindAsync(key)));

should be replaced with

       var person = await _cache.GetOrAdd(id, (key) => new AsyncLazy<Person>(() => db.People.FindAsync(key))).Value;

I believe it should be at least noted that if an exception occurs inside the factory function, the exception will be cached by the Task and since everything is a singleton, the application won't be able to recover from the failed state without some restart (unless I'm missing something).

Obviously this may or may not be a problem, depends on the developer needs, but usually this isn't the intended behavior. Most commonly is like not adding an item to a cache if the initialization fails.

Great repo David.

Any best practice about ConfigureAwai(false)? When is good to use it and when is not?ù

Thanks

Do You Have This Rules as Sonarqube Rules?If you have rules,Can you send github repo?

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/713b756bfc8c2d53dc35791cacbbc770c48fd482/Scenarios/Controllers/HttpClientController.cs#L31C13-L31C13

David, I figure you are a busy guy.. but I've noticed a discrepancy between .NET documentation and figured I'd get the best answer asking.

When it comes to the IHttpClientFactory.CreateClient is it best to utilize using given that it is an IDisposable? I know if I used typed clients I get disposal through DI.. Is it okay to inject clients into singletons, given that clients are supposed to be short lived?

Further.. should someone always using an IDisposable OR properly dispose via .Dispose() in the event of a short lived asset?

Am I correct in understanding that "typically" IDispoables are meant for releasing unmanaged resources that might not be correctly disposed by GC... ?

or does GC call .Dispose()?

I feel that a lot of disposal magic happens in DI, and so when working strictly inside a method that has some disposable assets I find myself getting a bit confused on what to using

Example of my typical flow (using httpclient as an example)

public async Task<Something> DoAsync()
{
   using var client = _factory.CreateClient("MaybeName");
   using var requestMessage = new HttpRequestMessage(....);
   
   using var response = await client.SendAsync(requestMessage);
   
   DoSomethingWithResponse...;
   
   return Something;
}

Any guidance is appreciated

The note about WebClient contains some contradictory advice.

WebClient is considered a legacy .NET API at this point and has been completely superseded by HttpClient. No new code should be written with HttpClient.

Seems to suggest HttpClient should both:

1. be used instead of WebClient
2. never be used

In https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md under "Async Void" the example code snippet won't compile anyway

In the AsyncGuidance document, there's a section that states that async/await is preferred over directly returning Task.

In his talk Correcting Common Async/Await Mistakes in .NET, Brandon Minnick advises to directly return the Task over async/await. (The specific bit is around 24:27.)

Since this is contradictory advice given by two MSFT employees, I feel like you guys should align this somehow, or clarify this use-case a bit more. :-)

I'm not sure if it may add more confusion than it's worth, but should an exception for a long-running asynchronous process be included for clarity? By long-running async process I mean a process that runs using async/await so it doesn't block any thread pool threads but which executes for an extended period of time. To my understanding, this pattern is acceptable, though it comes with other complexities.

I've definitely encountered cases where the advice on long-running work was interpreted to also include this pattern. I've seen this pattern used with Task.Factory.StartNew and TaskCreationOptions.LongRunning in the false assumption it puts all the work on a separate thread. In reality, it makes a thread just for startup, and then the first await puts the work back on the thread pool.

Example:

public class WorkerClass : IDisposable
{
    private static readonly TimeSpan WorkInterval = TimeSpan.FromMinutes(5);

    private readonly CancellationTokenSource _cts = new();

    public void Start()
    {
        bool restoreFlow = false;
        try
        {
            if (!ExecutionContext.IsFlowSuppressed)
            {
                ExecutionContext.SuppressFlow();
                restoreFlow = true;
            }

            _ = DoWorkAsync();
        }
        finally
        {
            if (restoreFlow)
            {
                ExecutionContext.RestoreFlow();
            }
        }
    }

    public async Task DoWorkAsync()
    {
        while (!_cts.IsCancellationRequested)
        {
            await Task.Delay(WorkInterval, _cts.Token);

            // Do async work here, assumption is the work item is not CPU intensive
        }
    }

    public void Dispose()
    {
        _cts.Cancel();
        _cts.Dispose();
    }
}

Hello,

Very great list of best practices. It guides me and coworkers about multiple async await usage.

But there is 1 question where people disagree.

Regarding the following code

var task1 = service.GetAsync1();
var task2 = service.GetAsync2();
await Task.WhenAll(task1, task2);
var result1 = task1.Result; 
var result2 = task2.Result;

We agree that starting the 2 first tasks before waiting any result is good. But we disagree how to get the result.

By following best practices to get the value using await on each tasks is imo safer. But I do not have much arguments about it. I prefer the following version. It's more readable. But I miss some knowledge about performance and safety.

var task1 = service.GetAsync1();
var task2 = service.GetAsync2();
var result1 = await task1; 
var result2 = await task2;

What is your mind ? What are the best version regarding safety, performance, readability ?

I know that this Repo is dedicated to AspNet core but which TaskCreationOptions should i take for .net versions below 4.6?
In .Net 4.5, TaskCreationOptions.RunContinuationsAsynchronously does not exist.

• Sync over async in factory
• Factory deadlock