Add async/await guidelines to C# basics

[carrollj]

• Fixes C# basics page lacks crucial information about async code usage #6790

The documentation differs a bit from what the issue mentioned, but I think it's for good reason. I got the impression that the thread that spawned the issue (godotengine/godot#63725) spurred a misunderstanding: That _Process executing before awaits inside of _Ready completed was a problem. In reality, it's expected behavior and matches the behavior of GDScript.

This documentation provides a way to asynchronously execute code from top level functions such as _Ready without needing to mark those functions as async. I believe that is important, since many people familiar with async/await in C# have been warned against it very heavily.

I also included code where I marked the top level functions as async. I tested that code pretty heavily for all the common pitfalls people bring up when saying "avoid async void", and it does not appear to suffer from any of them. I could have missed something, but overall it seems to perform even marginally better than the non async void approach.

Finally, I included an approach using Task.Run(), since that seems to be the most commonly recommended approach to executing async methods in Godot. Personally, I think it robs you of all the value of having async methods with only additional risks to gain (switching threads). But since it's out there I thought it important to document the approach while describing the pitfalls and workarounds for those pitfalls, especially since I rarely see anyone mention them when recommending using Task.Run.

Another thing I excluded was the recommendation to use ConfigureAwait(false). Using it should be avoided in application level code since it bypasses the SynchronizationContext provided by the application (GodotSynchronizationContext) for the called method and every async method in that call chain. The GodotSynchronizationContext does a great job of keeping continuations on the same thread as its caller, and bypassing that has the potential to create problems where a continuation is unexpectedly run on a different thread than the caller, causing thread safety issues that are almost invisible to the average user. Besides, there aren't any issues in the provided examples that using ConfigureAwait(false) would solve, except some minor performance gains in some scenarios, so I figured it was best not to mention it.

One thing to note is that my SampleAsyncNode for avoiding race conditions does not work in the latest release of 4.2 beta 3 due to a bug introduced in 4.2 dev 4 (godotengine/godot#82279).

Sorry for the wall of text

[paulloz]

I have a hard time seeing this on the C# basics page. The ins and outs of async in C# aren't that basic IMHO. Especially when we introduce async code running on different threads at the end. Arguably, this is a topic that could have its dedicated page.

[paulloz]

We always explicitly type access modifiers. Ditto for all implicitly private fields.

[paulloz]

Probably or ValueTask/ValueTask<T> (or any generalized async type).

[carrollj]

I can reword this to "change the return type to an await-able type, such as Task or Task<T>". Sound good?

[paulloz]

I'm personally not a big fan of the "because wisdom". I'd rather have that explained, or at least linked to an explanation on the Microsoft docs.

[carrollj]

That is one of the things I ended up trimming due to length. I would prefer to explain the reasoning behind it, then explain why doing so in Godot does not cause any issues, but that got too long and not basic for what I felt belonged in the page.

I can add a link to an explanation of why to avoid it, but I would also like to include a paragraph explaining why it doesn't apply in Godot.

[carrollj]

I have a hard time seeing this on the C# basics page. The ins and outs of async in C# aren't that basic IMHO. Especially when we introduce async code running on different threads at the end.

I'm inclined to agree, I only put it here since it's where the issue requested it. If it belongs somewhere else, I'd be happy to move it. I skimmed over a lot of details because I felt it was getting far to long for the C# basics page.

[mhilbrunner]

cc @raulsntos :) Thoughts? Where should this go?

[raulsntos]

There's definetely a need for a documentation page that explains asynchronous programming in the context of Godot. I feel like it's a complicated topic though, so I think it deserves its own dedicated page.

The documentation should likely explain how Godot's SynchronizationContext works, and how it affects async/await. Some of this is just how C# works, unrelated to Godot, but it's complicated and not a lot of developers know about it. I wouldn't consider myself to be very knowledgeable about it either, and often find myself re-reading Stephen Toub's articles about it.

• How Async/Await Really Works in C#
• Async/Await FAQ

I haven't had a chance to review this PR in depth yet, but from a quick look it seems to be too superficial, it explains basic concepts that are already covered by Microsoft's documentation so we could probably just link to them.

In general, I prefer to avoid duplicating Microsoft's documentation, otherwise we risk our documentation getting outdated as new concepts get introduced in C#. Instead, Godot's documentation should focus on how the C# concepts (such as async and await) apply in the context of Godot. Do these work the same way as in a normal C# application? If not, how are they different? In this case, we have a custom SynchronizationContext implementation. What does that mean for users and how does it affect their async/await code?

So to summarize:

• I think this should be a dedicated page. Definitely a welcomed addition, thank you for contributing.
• The documentation should probably skip some basic C# concepts that are already explained in Microsoft's documentation and just link it.
• The documentation should at least mention that Godot implements a custom SynchronizationContext and try to explain how that affects game developers.

[carrollj]

Thanks for the feedback. I'll be happy to make the requested changes; I felt I had to omit too many things to keep the section in line with being in the C# Basics page. I wasn't sure about the policy towards linking to external sources. I know some sites, like Stackoverflow, prefer having the relevant concepts explained on page in case the external source changes, is moved, or goes down, so I erred on that side. I'll make sure to include links to documentation instead of explaining the fundamentals.

Stephen Toub's articles are excellent resources, is it okay to link to those as well?

It'll probably take me a few days to make the revisions due to Halloween and other obligations. In the meantime, where do you suggest this new page go? I'm guessing something like tutorials/scripting/c_sharp/c_sharp_async_await.rst with a link to it added in the toctree under the "Godot API for C#" section in /c_sharp/index.rst.

Additionally, there is a section on the await keyword in /c_sharp/c_sharp_differences.html. Should that be rolled into this new page? Or perhaps link to it?

[carrollj]

Sorry @AThousandShips, I didn't mean to trigger this to be re-reviewed, I was just squashing and rebasing this branch before making the requested changes

[AThousandShips]

No worries, I realized I would wait until the restructuring

[Perustaja]

I think there needs to just be a summarization with a separate page here. I also think some stuff could be rephrased/rearranged. For rephrasing, the term zombie virus seems a bit informal and out of place.

I also think that the first explanation essentially says "just use async void" but then starts off with an explanation using TaskFactory. I think start with the async void explanation and then show the rest, because of discussions here and here which, in my interpretation at least, puts that solution forward as a straight-forward fix.

It's a good explanation of how it works though, because using async code here is quite a bit different than, say, ASP.NET. If you want I'd be happy to give a draft with it all on the current page. It is a lot though, and the main basics page should probably tersely show how to make it work as expected, with a branch off explaining the nuances as you've done.

Also, may be worth mentioning other nuances, e.g. signal delegates must return void (not Task), which will also end up with different method signatures or only using synchronous code.

[tvardero]

open for criticism

You should block the thread that enters the _Ready function by synchronously waiting for all your async tasks (via .Result or .Wait()), whether they are long-running or short-running. This is necessary if awaiting them is critical for the integrity of the parent scene.

In the case of the thread entering the _Process function, it should only be blocked for really fast async tasks. For short-running tasks, I recommend considering making their code synchronous where possible. For long-running tasks, avoid awaiting them and instead run them in the background. One approach could be to enqueue work (private readonly Queue<YourWorkObject> _queue) and regularly check it in the background for incoming work. If you seek a different method, you can search the web for "how to enqueue work in the background" or "how to run tasks in the background without awaiting them."

Any network/database call, heavy mathematical calculation, or I/O operations (such as working with files) should be considered as long-running tasks in the context of the _Process function and should be left asynchronous. Regarding heavy mathematical calculations, they could indeed be made synchronous. However, if their performance impact is considerable and/or they could be parallelized for better performance, it's better to leave them as Task.Run. (By the way, it's nice to know about methods like Parallel.ForEach for your mathematical operations, terrain generations, etc.)

The use of .ConfigureAwait(false) is justified only for code used as part of a generic library that is agnostic to the environment in which it's being used. Examples include ORMs (EntityFramework), message bus connectors (MassTransit), background workers (Hangfire), and CS algorithms—they do not care whether they run in a web application, desktop app, or Godot game; hence, they do not care about the threading model there.

Using ConfigureAwait(false) in code that interacts with Godot could lead to synchronization issues or access violations. For instance, in WinForms, you are not allowed to interact with your canvas from any thread except the UI thread. Trying to paint a button red or move an input field to the right from such threads results in an access violation. ConfigureAwait(false) instructs your program that your async task could finish on any thread other than the one that started the task. If you start your falsely-configured awaitable task from the UI thread, you might end up with it finishing its job on another thread and having no access to the canvas in a WinForm application.

(Please note that if Godot does not have such considerations about accessing UI from different threads, then what I've written on the ConfigureAwait subject might not be very relevant.)

EDIT: The text goes not only for _Ready or _Process, but for event handlers too (Button onClick, Area onEnter and others). Awaiting something long there after f.e. button press will make UI hang until the task is done.

Edit2: Actually, do not use async method in _Process and _Input at all. If by a mistake the async method will be called each tick, then it will 1. allocate new task on heap and lead to memory leak, 2. enqueue work on thread pool and lead to thread pool exhaustion.

[tvardero]

Task 1 should complete before _Process starts.
Not waiting for Task 1 to complete may result in incomlete scene initalization (for example if we were loading something from internet in _Ready function)

[tvardero]

For now solution is to block _Ready thread:

public override _Ready() {
  DoStuffAsync().GetAwaiter().GetResult();
}

I don't thing it is good to exit _Ready() if the node is not yet ready de-facto. Telling users, who knows programming weakly, to implement IsNodeInitialized or IsInstanceValid might confuse them and make their games bugged.

[hawkerm]

If you use this analogy, could be good to quote the source articles here with other best practices: (first searchable reference these days here: https://learn.microsoft.com/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming - though this later article has an origin attributing a C# team member: https://learn.microsoft.com/archive/msdn-magazine/2015/november/asynchronous-programming-async-from-the-start)