Skip to content

[SignalR] Avoid deadlock with closing and user callbacks #19612

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

Merged
merged 1 commit into from
Mar 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1214,11 +1214,13 @@ async Task StartProcessingInvocationMessages(ChannelReader<InvocationMessage> in
finally
{
invocationMessageChannel.Writer.TryComplete();
await invocationMessageReceiveTask;
timer.Stop();
await timerTask;
uploadStreamSource.Cancel();
await HandleConnectionClose(connectionState);

// await after the connection has been closed, otherwise could deadlock on a user's .On callback(s)
await invocationMessageReceiveTask;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public async Task UploadStreamCancelationSendsStreamComplete()
var complete = await connection.ReadSentJsonAsync().OrTimeout();
Assert.Equal(HubProtocolConstants.CompletionMessageType, complete["type"]);
Assert.EndsWith("canceled by client.", ((string)complete["error"]));
}
}
}

[Fact]
Expand Down Expand Up @@ -414,6 +414,59 @@ bool ExpectedErrors(WriteContext writeContext)
}
}

[Fact]
public async Task CanAwaitInvokeFromOnHandlerWithServerClosingConnection()
{
using (StartVerifiableLog())
{
var connection = new TestConnection();
var hubConnection = CreateHubConnection(connection, loggerFactory: LoggerFactory);
await hubConnection.StartAsync().OrTimeout();

var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
hubConnection.On<string>("Echo", async msg =>
{
try
{
// This should be canceled when the connection is closed
await hubConnection.InvokeAsync<string>("Echo", msg).OrTimeout();
}
catch (Exception ex)
{
tcs.SetException(ex);
return;
}

tcs.SetResult(null);
});

var closedTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
hubConnection.Closed += _ =>
{
closedTcs.SetResult(null);

return Task.CompletedTask;
};

await connection.ReceiveJsonMessage(new { type = HubProtocolConstants.InvocationMessageType, target = "Echo", arguments = new object[] { "42" } });

// Read sent message first to make sure invoke has been processed and is waiting for a response
await connection.ReadSentJsonAsync().OrTimeout();
await connection.ReceiveJsonMessage(new { type = HubProtocolConstants.CloseMessageType });

await closedTcs.Task.OrTimeout();

try
{
await tcs.Task.OrTimeout();
Assert.True(false);
}
catch (TaskCanceledException)
{
}
}
}

[Fact]
public async Task CanAwaitUsingHubConnection()
{
Expand Down