Skip to content

Commit f053620

Browse files
[SignalR] Avoid deadlock with closing and user callbacks (#19612)
1 parent 7fabb6c commit f053620

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,11 +1214,13 @@ async Task StartProcessingInvocationMessages(ChannelReader<InvocationMessage> in
12141214
finally
12151215
{
12161216
invocationMessageChannel.Writer.TryComplete();
1217-
await invocationMessageReceiveTask;
12181217
timer.Stop();
12191218
await timerTask;
12201219
uploadStreamSource.Cancel();
12211220
await HandleConnectionClose(connectionState);
1221+
1222+
// await after the connection has been closed, otherwise could deadlock on a user's .On callback(s)
1223+
await invocationMessageReceiveTask;
12221224
}
12231225
}
12241226

src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ public async Task UploadStreamCancelationSendsStreamComplete()
345345
var complete = await connection.ReadSentJsonAsync().OrTimeout();
346346
Assert.Equal(HubProtocolConstants.CompletionMessageType, complete["type"]);
347347
Assert.EndsWith("canceled by client.", ((string)complete["error"]));
348-
}
348+
}
349349
}
350350

351351
[Fact]
@@ -414,6 +414,59 @@ bool ExpectedErrors(WriteContext writeContext)
414414
}
415415
}
416416

417+
[Fact]
418+
public async Task CanAwaitInvokeFromOnHandlerWithServerClosingConnection()
419+
{
420+
using (StartVerifiableLog())
421+
{
422+
var connection = new TestConnection();
423+
var hubConnection = CreateHubConnection(connection, loggerFactory: LoggerFactory);
424+
await hubConnection.StartAsync().OrTimeout();
425+
426+
var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
427+
hubConnection.On<string>("Echo", async msg =>
428+
{
429+
try
430+
{
431+
// This should be canceled when the connection is closed
432+
await hubConnection.InvokeAsync<string>("Echo", msg).OrTimeout();
433+
}
434+
catch (Exception ex)
435+
{
436+
tcs.SetException(ex);
437+
return;
438+
}
439+
440+
tcs.SetResult(null);
441+
});
442+
443+
var closedTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
444+
hubConnection.Closed += _ =>
445+
{
446+
closedTcs.SetResult(null);
447+
448+
return Task.CompletedTask;
449+
};
450+
451+
await connection.ReceiveJsonMessage(new { type = HubProtocolConstants.InvocationMessageType, target = "Echo", arguments = new object[] { "42" } });
452+
453+
// Read sent message first to make sure invoke has been processed and is waiting for a response
454+
await connection.ReadSentJsonAsync().OrTimeout();
455+
await connection.ReceiveJsonMessage(new { type = HubProtocolConstants.CloseMessageType });
456+
457+
await closedTcs.Task.OrTimeout();
458+
459+
try
460+
{
461+
await tcs.Task.OrTimeout();
462+
Assert.True(false);
463+
}
464+
catch (TaskCanceledException)
465+
{
466+
}
467+
}
468+
}
469+
417470
[Fact]
418471
public async Task CanAwaitUsingHubConnection()
419472
{

0 commit comments

Comments
 (0)