godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.13k stars 21.19k forks source link

`await` generates invalid state machine that never completes for `System.Net.Http.HttpClient` #91608

Open Muchaszewski opened 6 months ago

Muchaszewski commented 6 months ago

Tested versions

v4.2.1.stable.mono.official [b09f793f5], v4.2.2.stable.mono.official [15073afe3]

System information

Godot v4.2.2.stable.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4090 (NVIDIA; 31.0.15.5161) - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

Issue description

await generates an invalid state machine that never completes for System.Net.Http.HttpClient awaiting using .Wait() works as intended.

This is an issue with third-party libraries that use System HttpClient instead of Godot's HttpClient.

Steps to reproduce

Create a new C# project Import the code below and run with debugger on (see gif below for execution results)

using System.Threading.Tasks;
using Godot;

public partial class TestScript : Node
{
    private System.Net.Http.HttpClient _httpClient = new System.Net.Http.HttpClient();

    public override void _Ready()
    {
        Works();
        Hangs().Wait();
        base._Ready();
    }

    public void Works()
    {
        var task = _httpClient.GetAsync("https://www.google.com");
        task.Wait();
        var google = task.Result;
        var contentTask = google.Content.ReadAsStringAsync();
        contentTask.Wait();
        var googleContent = contentTask.Result;
        GD.Print(googleContent);
    }

    public async Task Hangs()
    {
        var google = await _httpClient.GetAsync("https://www.google.com");
        var googleContent = await google.Content.ReadAsStringAsync();
        GD.Print(googleContent);
    }
}

Animation

Minimal reproduction project (MRP)

demo.zip

raulsntos commented 6 months ago

Task.Wait blocks the calling thread which will cause a deadlock here because await expressions in Godot use a custom GodotSynchronizationContext that queues the continuation on Godot's main thread. This means that await continuations are waiting in a queue until Godot invokes them, but Godot is blocked because you called Task.Wait.

In general, you have to be careful when using Task.Wait or Task.Result. It's preferable to use await instead. And try to avoid blocking on engine callbacks (such as _Ready or _Process).

A possible solution to your deadlock could be to avoid GodotSynchronizationContext or using Task.ConfigureAwait(false). That way you avoid blocking the Godot main thread and the queued continuations can be invoked.

// Use Task.Run to avoid using GodotSynchronizationContext.
Task.Run(() => Hang()).Wait();

// Alternatively, use Task.ConfigureAwait(false) to indicate that
// the continuation doesn't go back to the original context.
var google = await _httpClient.GetAsync("https://www.google.com").ConfigureAwait(false);
Muchaszewski commented 6 months ago

Hard to say what to do about it, but it's definitely a noob trap. In my case, the _Ready blocking call was okay because I had a lot of processing to do at that step. But it would be nice to have some more information on the deadlock.

pseudo code solution?

if in DEBUG
register this new task at GodotSynchronizationContext
see that `_Process` is hung for some time and we didn't update for some time (5-10seconds?)
throw an exception and abort the Task/Thread
configurable via settings `GD.GodotSynchronizationContextDebugTimeout(Timespan.Infinity);`

Maybe it's more a matter of adding this to docs, rather then fixing the underlying issue as I guess it might be though.