Aspnetcore: SignalR deadlocks client on internal server-side exceptions during nested InvokeAsync call

[Kant8]

As title says when client makes nested call to InvokeAsync (inside ConnectionHub.On() handler), server can throw InvalidDataException if message is larger than a limit. This results in ClosedMessage being received by client and then client tries to close connection. Few instructions later client awaits other currently processing calls before closing connection.

Unfortunately this await waits our own InvokeAsync, which was the cause of error. And this InvokeAsync never receives any exceptions from server, looks like because Dispatcher wasn't called, and there is no one who can send proper error to the client, like DefaultHubDispatcher does on exceptions in Hub methods.

As result, client waits for response from server, server already responded with disconnection message, but client cannot call ConnectionHub.Disconnected event because of deadlock few lines before.

If we use SendAsync everything is ok, cause obviously await finished right after message was sent, so there is no unprocessed messages to wait during connection closing phase.

If this is "by design", it should be at least documented somewhere in docs about InvokeAsync/SendAsync that you must not await InvokeAsync inside message handlers.

To Reproduce

Client:
``` C#
public class FooClient
{
private readonly HubConnection _connection;
private readonly TaskCompletionSource _tcs;
private readonly int _size;

public FooClient(string url, int size)
{
    _tcs = new TaskCompletionSource<bool>();
    _connection = new HubConnectionBuilder().WithUrl(url).Build();
    _size = size;
}

public async Task<bool> Do()
{
    _connection.On("SendMessage", SendMessageHandler);
    _connection.Closed += ClosedHandler;

    await _connection.StartAsync();
    await _connection.SendAsync("Prepare");

    var result = await _tcs.Task;
    await _connection.StopAsync();

    return result;
}

private async Task SendMessageHandler()
{
    try
    {
        var msg = new byte[_size];
        await _connection.InvokeAsync("Do", msg);

        // never get here if msg is big
        _tcs.SetResult(true);
    }
    catch (Exception ex)
    {
        // get here if msg is small
        _tcs.SetException(ex);
    }
}

private Task ClosedHandler(Exception ex)
{
    // never get here if msg is big
    if (ex != null)
        _tcs.SetException(ex);
    else
        _tcs.TrySetResult(false);

    return Task.CompletedTask;
}

}

Hub on server:
``` C#
public class FooHub : Hub
{
    public async Task Prepare()
    {
        await Clients.Caller.SendAsync("SendMessage");
    }

    public async Task Do(byte[] msg)
    {
        // emulate some hard work
        await Task.Delay(1000);

        //this ends ok if msg is small
        throw new Exception("Exception in Finish");
    }
}

If you pass size like 10mb client waits forever on await _tcs.Task, end never receives neither Closed event not exception in SendMessageHandler catch(). Change InvokeAsync to SendAsync and Closed event is fired as intended.

I've tested on ASP.NET Core 3.0, but should be the same in 3.1

[anurse]

Triage Notes: We should take a look at this. We don't think this is intentional or by design.

[davidfowl]

There's a bug here, I took a look. We should move the message processing wait after we close the connection. It would properly terminate all pending invocations.

[anurse]

That's about what we came up with while taking a quick look in Triage.