Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Muchaszewski opened this issue May 6, 2024 · 2 comments

Comments

@Muchaszewski
Copy link

Muchaszewski commented May 6, 2024

Tested versions

v4.2.1.stable.mono.official [b09f793], v4.2.2.stable.mono.official [15073af]

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

@Muchaszewski Muchaszewski changed the title await generates invalid code that never completes for HttpClient await generates invalid state machine that never completes for HttpClient May 6, 2024
@Muchaszewski Muchaszewski changed the title await generates invalid state machine that never completes for HttpClient await generates invalid state machine that never completes for System.Net.Http.HttpClient May 6, 2024
@fire fire changed the title await generates invalid state machine that never completes for System.Net.Http.HttpClient await generates invalid state machine that never completes for System.Net.Http.HttpClient May 6, 2024
@raulsntos
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants