From 2c300579aac71983ce147cf8ca8ccdcc470881eb Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 4 Aug 2022 17:16:45 -0700 Subject: [PATCH 1/5] WIP default cancel timeout for invokes --- ...soft.AspNetCore.Server.Kestrel.Core.csproj | 3 +- .../Middleware/HttpsConnectionMiddleware.cs | 1 + .../CancellationTokenSourcePool.cs | 2 +- .../common/Shared/ClientResultsManager.cs | 2 +- .../Core/src/DefaultHubLifetimeManager.cs | 15 ++- .../Core/src/Internal/HubConnectionBinder.cs | 4 +- .../Microsoft.AspNetCore.SignalR.Core.csproj | 1 + .../HubConnectionHandlerTestUtils/Hubs.cs | 2 + .../HubConnectionHandlerTests.ClientResult.cs | 94 ++++++++++++++++++- ...pNetCore.SignalR.StackExchangeRedis.csproj | 1 + .../src/RedisHubLifetimeManager.cs | 19 +++- 11 files changed, 134 insertions(+), 10 deletions(-) rename src/{Servers/Kestrel/Core/src/Internal => Shared}/CancellationTokenSourcePool.cs (96%) diff --git a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj index cf77ba81c063..29990edece34 100644 --- a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj +++ b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj @@ -1,4 +1,4 @@ - + Core components of ASP.NET Core Kestrel cross-platform web server. @@ -29,6 +29,7 @@ + diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index a6aba0228722..51f5199f387d 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; diff --git a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs b/src/Shared/CancellationTokenSourcePool.cs similarity index 96% rename from src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs rename to src/Shared/CancellationTokenSourcePool.cs index 72e6465b7851..94279b1f970f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CancellationTokenSourcePool.cs +++ b/src/Shared/CancellationTokenSourcePool.cs @@ -3,7 +3,7 @@ using System.Collections.Concurrent; -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +namespace Microsoft.AspNetCore.Internal; internal sealed class CancellationTokenSourcePool { diff --git a/src/SignalR/common/Shared/ClientResultsManager.cs b/src/SignalR/common/Shared/ClientResultsManager.cs index 97b5d2c7023d..12544fb649dd 100644 --- a/src/SignalR/common/Shared/ClientResultsManager.cs +++ b/src/SignalR/common/Shared/ClientResultsManager.cs @@ -144,7 +144,7 @@ public void RegisterCancellation() { // TODO: RedisHubLifetimeManager will want to notify the other server (if there is one) about the cancellation // so it can clean up state and potentially forward that info to the connection - _clientResultsManager.TryCompleteResult(_connectionId, CompletionMessage.WithError(_invocationId, "Canceled")); + _clientResultsManager.TryCompleteResult(_connectionId, CompletionMessage.WithError(_invocationId, "Invocation canceled by the server.")); } public new void SetResult(T result) diff --git a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs index 45509fee030b..b8a5d2cc5a06 100644 --- a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs +++ b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.Extensions.Logging; @@ -16,6 +17,7 @@ namespace Microsoft.AspNetCore.SignalR; /// public class DefaultHubLifetimeManager : HubLifetimeManager where THub : Hub { + private static readonly CancellationTokenSourcePool _cancellationTokenSourcePool = new(); private readonly HubConnectionStore _connections = new HubConnectionStore(); private readonly HubGroupList _groups = new HubGroupList(); private readonly ILogger _logger; @@ -341,7 +343,18 @@ public override async Task InvokeConnectionAsync(string connectionId, stri } var invocationId = Interlocked.Increment(ref _lastInvocationId).ToString(NumberFormatInfo.InvariantInfo); - using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, + + CancellationTokenSource? cts = null; + // Add a default timeout if one isn't provided + if (!cancellationToken.CanBeCanceled) + { + cts = _cancellationTokenSourcePool.Rent(); + cts.CancelAfter(TimeSpan.FromSeconds(30)); + cancellationToken = cts.Token; + } + + using var _ = cts; + using var __ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, connection.ConnectionAborted, out var linkedToken); var task = _clientResultsManager.AddInvocation(connectionId, invocationId, linkedToken); diff --git a/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs b/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs index 45d80673825e..03366c2f1b36 100644 --- a/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs +++ b/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs @@ -27,7 +27,9 @@ public Type GetReturnType(string invocationId) { return type; } - throw new InvalidOperationException($"Unknown invocation ID '{invocationId}'."); + // If the id isn't found then it's possible the server canceled the request for a result but the client still sent the result. + // Return typeof(object) so the HubProtocol can still proceed (without throwing and closing the connection) and the hub dispatcher will log that the completion message is not expected. + return typeof(object); } public Type GetStreamItemType(string streamId) diff --git a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj index fd607f3d832b..ff7bfc4662cf 100644 --- a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj +++ b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj @@ -19,6 +19,7 @@ + diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs index ca84bad5cafc..bd7bacf13442 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs @@ -537,6 +537,8 @@ public interface ITest Task Broadcast(string message); Task GetClientResult(int value); + + Task GetClientResultWithCancellation(int value, CancellationToken cancellationToken); } public record ClientResults(int ClientResult, int CallerResult); diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs index 91133c96048d..23e23ce38b15 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs @@ -242,7 +242,7 @@ public async Task CanReturnClientResultToTypedHubTwoWays() }, LoggerFactory); var connectionHandler = serviceProvider.GetService>(); - using var client = new TestClient(invocationBinder: new GetClientResultThreeWaysInvocationBinder()); + using var client = new TestClient(invocationBinder: new GetClientResultTwoWaysInvocationBinder()); var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); @@ -266,7 +266,97 @@ public async Task CanReturnClientResultToTypedHubTwoWays() } } - private class GetClientResultThreeWaysInvocationBinder : IInvocationBinder + [Fact] + public async Task CanCancelClientResultsWithIHubContextT() + { + using (StartVerifiableLog()) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(null, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using var client = new TestClient(); + var connectionId = client.Connection.ConnectionId; + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler); + + // Wait for a connection, or for the endpoint to fail. + await client.Connected.OrThrowIfOtherFails(connectionHandlerTask).DefaultTimeout(); + + var context = serviceProvider.GetRequiredService>(); + + var cts = new CancellationTokenSource(); + var resultTask = context.Clients.Client(connectionId).GetClientResultWithCancellation(1, cts.Token); + + var message = await client.ReadAsync().DefaultTimeout(); + var invocation = Assert.IsType(message); + + Assert.Single(invocation.Arguments); + Assert.Equal(1L, invocation.Arguments[0]); + Assert.Equal("GetClientResultWithCancellation", invocation.Target); + + cts.Cancel(); + + var ex = await Assert.ThrowsAsync(() => resultTask).DefaultTimeout(); + Assert.Equal("Invocation canceled by the server.", ex.Message); + + // Sending result after the server is no longer expecting one results in a log and no-ops + await client.SendHubMessageAsync(CompletionMessage.WithResult(invocation.InvocationId, 2)).DefaultTimeout(); + + // Send another message from the client and get a result back to make sure the connection is still active. + // Regression test for when sending a client result after it was canceled would close the connection + var completion = await client.InvokeAsync("Echo", "test").DefaultTimeout(); + Assert.Equal("test", completion.Result); + + Assert.Contains(TestSink.Writes, c => c.EventId.Name == "UnexpectedCompletion"); + } + } + + [Fact] + public async Task CanCancelClientResultsWithIHubContext() + { + using (StartVerifiableLog()) + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(null, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using var client = new TestClient(); + var connectionId = client.Connection.ConnectionId; + + var connectionHandlerTask = await client.ConnectAsync(connectionHandler); + + // Wait for a connection, or for the endpoint to fail. + await client.Connected.OrThrowIfOtherFails(connectionHandlerTask).DefaultTimeout(); + + var context = serviceProvider.GetRequiredService>(); + + var cts = new CancellationTokenSource(); + var resultTask = context.Clients.Client(connectionId).InvokeAsync(nameof(MethodHub.GetClientResult), 1, cts.Token); + + var message = await client.ReadAsync().DefaultTimeout(); + var invocation = Assert.IsType(message); + + Assert.Single(invocation.Arguments); + Assert.Equal(1L, invocation.Arguments[0]); + Assert.Equal("GetClientResult", invocation.Target); + + cts.Cancel(); + + var ex = await Assert.ThrowsAsync(() => resultTask).DefaultTimeout(); + Assert.Equal("Invocation canceled by the server.", ex.Message); + + // Sending result after the server is no longer expecting one results in a log and no-ops + await client.SendHubMessageAsync(CompletionMessage.WithResult(invocation.InvocationId, 2)).DefaultTimeout(); + + // Send another message from the client and get a result back to make sure the connection is still active. + // Regression test for when sending a client result after it was canceled would close the connection + var completion = await client.InvokeAsync("Echo", "test").DefaultTimeout(); + Assert.Equal("test", completion.Result); + + Assert.Contains(TestSink.Writes, c => c.EventId.Name == "UnexpectedCompletion"); + } + } + + private class GetClientResultTwoWaysInvocationBinder : IInvocationBinder { public IReadOnlyList GetParameterTypes(string methodName) => new[] { typeof(int) }; public Type GetReturnType(string invocationId) => typeof(ClientResults); diff --git a/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj b/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj index e3ed270e72ef..228b5ccb2d75 100644 --- a/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj +++ b/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj @@ -12,6 +12,7 @@ + diff --git a/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs b/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs index eb3bbab0cfc3..d58020e5e511 100644 --- a/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs +++ b/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Text; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal; @@ -22,6 +23,8 @@ namespace Microsoft.AspNetCore.SignalR.StackExchangeRedis; /// The type of to manage connections for. public class RedisHubLifetimeManager : HubLifetimeManager, IDisposable where THub : Hub { + private static readonly CancellationTokenSourcePool _cancellationTokenSourcePool = new(); + private readonly HubConnectionStore _connections = new HubConnectionStore(); private readonly RedisSubscriptionManager _groups = new RedisSubscriptionManager(); private readonly RedisSubscriptionManager _users = new RedisSubscriptionManager(); @@ -419,7 +422,17 @@ public override async Task InvokeConnectionAsync(string connectionId, stri // Needs to be unique across servers, easiest way to do that is prefix with connection ID. var invocationId = $"{connectionId}{Interlocked.Increment(ref _lastInvocationId)}"; - using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, + CancellationTokenSource? cts = null; + // Add a default timeout if one isn't provided + if (!cancellationToken.CanBeCanceled) + { + cts = _cancellationTokenSourcePool.Rent(); + cts.CancelAfter(TimeSpan.FromSeconds(30)); + cancellationToken = cts.Token; + } + + using var _ = cts; + using var __ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, connection?.ConnectionAborted ?? default, out var linkedToken); var task = _clientResultsManager.AddInvocation(connectionId, invocationId, linkedToken); @@ -428,8 +441,8 @@ public override async Task InvokeConnectionAsync(string connectionId, stri if (connection == null) { // TODO: Need to handle other server going away while waiting for connection result - var m = _protocol.WriteInvocation(methodName, args, invocationId, returnChannel: _channels.ReturnResults(_serverName)); - var received = await PublishAsync(_channels.Connection(connectionId), m); + var messageBytes = _protocol.WriteInvocation(methodName, args, invocationId, returnChannel: _channels.ReturnResults(_serverName)); + var received = await PublishAsync(_channels.Connection(connectionId), messageBytes); if (received < 1) { throw new IOException($"Connection '{connectionId}' does not exist."); From 94db3e2bb412c51ee84156af60028b8b63315f16 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 12 Aug 2022 11:38:58 -0700 Subject: [PATCH 2/5] change --- .../src/Protocol/JsonHubProtocol.cs | 45 +++++++++++-- .../Protocol/MessagePackHubProtocolWorker.cs | 26 ++++++-- .../src/Protocol/NewtonsoftJsonHubProtocol.cs | 63 +++++++++++++++---- .../Protocol/JsonHubProtocolTestsBase.cs | 18 ++++-- .../Protocol/MessagePackHubProtocolTests.cs | 19 ++++-- .../Core/src/Internal/HubConnectionBinder.cs | 3 +- .../HubConnectionHandlerTests.ClientResult.cs | 43 +++++++++++-- 7 files changed, 178 insertions(+), 39 deletions(-) diff --git a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs index c3930f66dda2..a28bb73a5ded 100644 --- a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs @@ -230,8 +230,27 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) else { // If we have an invocation id already we can parse the end result - var returnType = binder.GetReturnType(invocationId); - result = BindType(ref reader, input, returnType); + Type? returnType; + try + { + returnType = binder.GetReturnType(invocationId); + } + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) + { + returnType = null; + } + if (returnType is null) + { + reader.Skip(); + result = null; + } + else + { + result = BindType(ref reader, input, returnType); + } } } else if (reader.ValueTextEquals(ItemPropertyNameBytes.EncodedUtf8Bytes)) @@ -408,8 +427,26 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) if (hasResultToken) { - var returnType = binder.GetReturnType(invocationId); - result = BindType(ref resultToken, input, returnType); + Type? returnType; + try + { + returnType = binder.GetReturnType(invocationId); + } + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) + { + returnType = null; + } + if (returnType is null) + { + result = null; + } + else + { + result = BindType(ref resultToken, input, returnType); + } } message = BindCompletionMessage(invocationId, error, result, hasResult); diff --git a/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs b/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs index 8ce10b662bd9..beaa7b1bb340 100644 --- a/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs +++ b/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs @@ -162,14 +162,32 @@ private CompletionMessage CreateCompletionMessage(ref MessagePackReader reader, error = ReadString(ref reader, "error"); break; case NonVoidResult: - var itemType = binder.GetReturnType(invocationId); - if (itemType == typeof(RawResult)) + Type? itemType; + try { - result = new RawResult(reader.ReadRaw()); + itemType = binder.GetReturnType(invocationId); + } + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) + { + itemType = null; + } + if (itemType is null) + { + reader.Skip(); } else { - result = DeserializeObject(ref reader, itemType, "argument"); + if (itemType == typeof(RawResult)) + { + result = new RawResult(reader.ReadRaw()); + } + else + { + result = DeserializeObject(ref reader, itemType, "argument"); + } } hasResult = true; break; diff --git a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs index 2df8002c66d9..f4a8dbc31ea8 100644 --- a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs @@ -209,21 +209,40 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) else { // If we have an invocation id already we can parse the end result - var returnType = binder.GetReturnType(invocationId); - - if (!JsonUtils.ReadForType(reader, returnType)) + Type? returnType; + try { - throw new JsonReaderException("Unexpected end when reading JSON"); + returnType = binder.GetReturnType(invocationId); } - - if (returnType == typeof(RawResult)) + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) { - var token = JToken.Load(reader); - result = GetRawResult(token); + returnType = null; + } + if (returnType is null) + { + reader.Skip(); + result = null; } else { - result = PayloadSerializer.Deserialize(reader, returnType); + + if (!JsonUtils.ReadForType(reader, returnType)) + { + throw new JsonReaderException("Unexpected end when reading JSON"); + } + + if (returnType == typeof(RawResult)) + { + var token = JToken.Load(reader); + result = GetRawResult(token); + } + else + { + result = PayloadSerializer.Deserialize(reader, returnType); + } } } break; @@ -397,14 +416,32 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) if (resultToken != null) { - var returnType = binder.GetReturnType(invocationId); - if (returnType == typeof(RawResult)) + Type? returnType; + try { - result = GetRawResult(resultToken); + returnType = binder.GetReturnType(invocationId); + } + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) + { + returnType = null; + } + if (returnType is null) + { + result = null; } else { - result = resultToken.ToObject(returnType, PayloadSerializer); + if (returnType == typeof(RawResult)) + { + result = GetRawResult(resultToken); + } + else + { + result = resultToken.ToObject(returnType, PayloadSerializer); + } } } diff --git a/src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs b/src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs index dcc68f102b14..59e765acf0c4 100644 --- a/src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs +++ b/src/SignalR/common/SignalR.Common/test/Internal/Protocol/JsonHubProtocolTestsBase.cs @@ -1,16 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; -using System.Collections.Generic; using System.Globalization; -using System.IO; -using System.Linq; using System.Text; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Protocol; -using Xunit; namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol; @@ -460,6 +455,19 @@ public void RawResultRoundTripsProperly(string testDataName) } } + [Fact] + public void UnexpectedClientResultGivesEmptyCompletionMessage() + { + var binder = new TestBinder(); + var message = Frame("{\"type\":3,\"result\":1,\"invocationId\":\"1\"}"); + var data = new ReadOnlySequence(Encoding.UTF8.GetBytes(message)); + Assert.True(JsonHubProtocol.TryParseMessage(ref data, binder, out var hubMessage)); + + var completion = Assert.IsType(hubMessage); + Assert.Null(completion.Result); + Assert.Equal("1", completion.InvocationId); + } + public static string Frame(string input) { var data = Encoding.UTF8.GetBytes(input); diff --git a/src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTests.cs b/src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTests.cs index 978d729cc4d5..647f50cd3324 100644 --- a/src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTests.cs +++ b/src/SignalR/common/SignalR.Common/test/Internal/Protocol/MessagePackHubProtocolTests.cs @@ -1,15 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; -using System.Collections.Generic; -using System.Diagnostics; -using System.IO; -using System.Linq; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Protocol; -using Xunit; namespace Microsoft.AspNetCore.SignalR.Common.Tests.Internal.Protocol; @@ -249,6 +243,19 @@ public void RawResultRoundTripsProperly(string testDataName) } } + [Fact] + public void UnexpectedClientResultGivesEmptyCompletionMessage() + { + var binder = new TestBinder(); + var input = Frame(Convert.FromBase64String("lQOAo3h5egPA")); + var data = new ReadOnlySequence(input); + Assert.True(HubProtocol.TryParseMessage(ref data, binder, out var hubMessage)); + + var completion = Assert.IsType(hubMessage); + Assert.Null(completion.Result); + Assert.Equal("xyz", completion.InvocationId); + } + public class ClientResultTestData { public string Name { get; } diff --git a/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs b/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs index 03366c2f1b36..9b844929b951 100644 --- a/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs +++ b/src/SignalR/server/Core/src/Internal/HubConnectionBinder.cs @@ -28,8 +28,7 @@ public Type GetReturnType(string invocationId) return type; } // If the id isn't found then it's possible the server canceled the request for a result but the client still sent the result. - // Return typeof(object) so the HubProtocol can still proceed (without throwing and closing the connection) and the hub dispatcher will log that the completion message is not expected. - return typeof(object); + throw new InvalidOperationException($"Unknown invocation ID '{invocationId}'."); } public Type GetStreamItemType(string streamId) diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs index 23e23ce38b15..8bc62b632a45 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs @@ -266,15 +266,48 @@ public async Task CanReturnClientResultToTypedHubTwoWays() } } - [Fact] - public async Task CanCancelClientResultsWithIHubContextT() + private class TestBinder : IInvocationBinder { + public IReadOnlyList GetParameterTypes(string methodName) + { + return new Type[] { typeof(int) }; + } + + public Type GetReturnType(string invocationId) + { + return typeof(string); + } + + public Type GetStreamItemType(string streamId) + { + throw new NotImplementedException(); + } + } + + [Theory] + [InlineData("MessagePack")] + [InlineData("Json")] + public async Task CanCancelClientResultsWithIHubContextT(string protocol) + { + IHubProtocol hubProtocol; + if (string.Equals(protocol, "MessagePack")) + { + hubProtocol = new MessagePackHubProtocol(); + } + else if (string.Equals(protocol, "Json")) + { + hubProtocol = new JsonHubProtocol(); + } + else + { + throw new Exception($"Protocol {protocol} not handled by test."); + } using (StartVerifiableLog()) { var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(null, LoggerFactory); var connectionHandler = serviceProvider.GetService>(); - using var client = new TestClient(); + using var client = new TestClient(hubProtocol, new TestBinder()); var connectionId = client.Connection.ConnectionId; var connectionHandlerTask = await client.ConnectAsync(connectionHandler); @@ -291,7 +324,7 @@ public async Task CanCancelClientResultsWithIHubContextT() var invocation = Assert.IsType(message); Assert.Single(invocation.Arguments); - Assert.Equal(1L, invocation.Arguments[0]); + Assert.Equal(1, invocation.Arguments[0]); Assert.Equal("GetClientResultWithCancellation", invocation.Target); cts.Cancel(); @@ -304,7 +337,7 @@ public async Task CanCancelClientResultsWithIHubContextT() // Send another message from the client and get a result back to make sure the connection is still active. // Regression test for when sending a client result after it was canceled would close the connection - var completion = await client.InvokeAsync("Echo", "test").DefaultTimeout(); + var completion = await client.InvokeAsync(nameof(HubT.Echo), "test").DefaultTimeout(); Assert.Equal("test", completion.Result); Assert.Contains(TestSink.Writes, c => c.EventId.Name == "UnexpectedCompletion"); From ec7b5346f8df5c5cb0a9440f5277c51256ea74a0 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Wed, 17 Aug 2022 09:55:18 -0700 Subject: [PATCH 3/5] remove default token and default timeout --- .../clients/ts/FunctionalTests/Startup.cs | 2 +- ...t.AspNetCore.SignalR.Protocols.Json.csproj | 1 + .../src/Protocol/JsonHubProtocol.cs | 26 ++--------------- ...tCore.SignalR.Protocols.MessagePack.csproj | 1 + .../Protocol/MessagePackHubProtocolWorker.cs | 13 +-------- ...re.SignalR.Protocols.NewtonsoftJson.csproj | 1 + .../src/Protocol/NewtonsoftJsonHubProtocol.cs | 26 ++--------------- src/SignalR/common/Shared/TryGetReturnType.cs | 24 ++++++++++++++++ .../server/Core/src/ClientProxyExtensions.cs | 22 +++++++-------- .../Core/src/DefaultHubLifetimeManager.cs | 16 ++--------- .../server/Core/src/HubLifetimeManager.cs | 4 +-- .../server/Core/src/ISingleClientProxy.cs | 4 +-- .../Microsoft.AspNetCore.SignalR.Core.csproj | 1 - .../server/Core/src/PublicAPI.Unshipped.txt | 28 +++++++++---------- .../server/SignalR/test/ClientProxyTests.cs | 2 +- .../HubConnectionHandlerTestUtils/Hubs.cs | 6 ++-- .../HubConnectionHandlerTests.ClientResult.cs | 2 +- .../src/HubLifetimeManagerTestBase.cs | 14 +++++----- .../src/ScaleoutHubLifetimeManagerTests.cs | 14 +++++----- ...pNetCore.SignalR.StackExchangeRedis.csproj | 1 - .../src/PublicAPI.Unshipped.txt | 2 +- .../src/RedisHubLifetimeManager.cs | 17 ++--------- 22 files changed, 86 insertions(+), 141 deletions(-) create mode 100644 src/SignalR/common/Shared/TryGetReturnType.cs diff --git a/src/SignalR/clients/ts/FunctionalTests/Startup.cs b/src/SignalR/clients/ts/FunctionalTests/Startup.cs index 1d831c9d28c9..67eb2878ade3 100644 --- a/src/SignalR/clients/ts/FunctionalTests/Startup.cs +++ b/src/SignalR/clients/ts/FunctionalTests/Startup.cs @@ -232,7 +232,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, ILogger< { try { - var result = await hubContext.Clients.Client(id).InvokeAsync("Result"); + var result = await hubContext.Clients.Client(id).InvokeAsync("Result", cancellationToken: default); return result.ToString(CultureInfo.InvariantCulture); } catch (Exception ex) diff --git a/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj b/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj index 34a5f17dd19d..38694e211d3f 100644 --- a/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj +++ b/src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj @@ -17,6 +17,7 @@ + diff --git a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs index a28bb73a5ded..6d726e0c2c17 100644 --- a/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs @@ -230,18 +230,7 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) else { // If we have an invocation id already we can parse the end result - Type? returnType; - try - { - returnType = binder.GetReturnType(invocationId); - } - // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result - // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message - // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling - catch (Exception) - { - returnType = null; - } + var returnType = ProtocolHelper.TryGetReturnType(binder, invocationId); if (returnType is null) { reader.Skip(); @@ -427,18 +416,7 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) if (hasResultToken) { - Type? returnType; - try - { - returnType = binder.GetReturnType(invocationId); - } - // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result - // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message - // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling - catch (Exception) - { - returnType = null; - } + var returnType = ProtocolHelper.TryGetReturnType(binder, invocationId); if (returnType is null) { result = null; diff --git a/src/SignalR/common/Protocols.MessagePack/src/Microsoft.AspNetCore.SignalR.Protocols.MessagePack.csproj b/src/SignalR/common/Protocols.MessagePack/src/Microsoft.AspNetCore.SignalR.Protocols.MessagePack.csproj index 69554b03f11a..7d3882c020b3 100644 --- a/src/SignalR/common/Protocols.MessagePack/src/Microsoft.AspNetCore.SignalR.Protocols.MessagePack.csproj +++ b/src/SignalR/common/Protocols.MessagePack/src/Microsoft.AspNetCore.SignalR.Protocols.MessagePack.csproj @@ -12,6 +12,7 @@ + diff --git a/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs b/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs index beaa7b1bb340..df0dc0c7a8a9 100644 --- a/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs +++ b/src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs @@ -162,18 +162,7 @@ private CompletionMessage CreateCompletionMessage(ref MessagePackReader reader, error = ReadString(ref reader, "error"); break; case NonVoidResult: - Type? itemType; - try - { - itemType = binder.GetReturnType(invocationId); - } - // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result - // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message - // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling - catch (Exception) - { - itemType = null; - } + var itemType = ProtocolHelper.TryGetReturnType(binder, invocationId); if (itemType is null) { reader.Skip(); diff --git a/src/SignalR/common/Protocols.NewtonsoftJson/src/Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj b/src/SignalR/common/Protocols.NewtonsoftJson/src/Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj index 2167aa456d47..f905437355d7 100644 --- a/src/SignalR/common/Protocols.NewtonsoftJson/src/Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj +++ b/src/SignalR/common/Protocols.NewtonsoftJson/src/Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj @@ -15,6 +15,7 @@ + diff --git a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs index f4a8dbc31ea8..2a0733a3790d 100644 --- a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs @@ -209,18 +209,7 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) else { // If we have an invocation id already we can parse the end result - Type? returnType; - try - { - returnType = binder.GetReturnType(invocationId); - } - // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result - // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message - // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling - catch (Exception) - { - returnType = null; - } + var returnType = ProtocolHelper.TryGetReturnType(binder, invocationId); if (returnType is null) { reader.Skip(); @@ -416,18 +405,7 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) if (resultToken != null) { - Type? returnType; - try - { - returnType = binder.GetReturnType(invocationId); - } - // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result - // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message - // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling - catch (Exception) - { - returnType = null; - } + var returnType = ProtocolHelper.TryGetReturnType(binder, invocationId); if (returnType is null) { result = null; diff --git a/src/SignalR/common/Shared/TryGetReturnType.cs b/src/SignalR/common/Shared/TryGetReturnType.cs new file mode 100644 index 000000000000..1cfcd0d189f1 --- /dev/null +++ b/src/SignalR/common/Shared/TryGetReturnType.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.AspNetCore.SignalR.Protocol; + +internal static class ProtocolHelper +{ + internal static Type? TryGetReturnType(IInvocationBinder binder, string invocationId) + { + try + { + return binder.GetReturnType(invocationId); + } + // GetReturnType throws if invocationId not found, this can be caused by the server canceling a client-result but the client still sending a result + // For now let's ignore the failure and skip parsing the result, server will log that the result wasn't expected anymore and ignore the message + // In the future we may want a CompletionBindingFailureMessage that we can flow to the dispatcher for handling + catch (Exception) + { + return null; + } + } +} diff --git a/src/SignalR/server/Core/src/ClientProxyExtensions.cs b/src/SignalR/server/Core/src/ClientProxyExtensions.cs index 7b2f0c8c8b8f..c7716ef8af51 100644 --- a/src/SignalR/server/Core/src/ClientProxyExtensions.cs +++ b/src/SignalR/server/Core/src/ClientProxyExtensions.cs @@ -227,7 +227,7 @@ public static Task SendAsync(this IClientProxy clientProxy, string method, objec /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, Array.Empty(), cancellationToken); } @@ -241,7 +241,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1 }, cancellationToken); } @@ -256,7 +256,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2 }, cancellationToken); } @@ -272,7 +272,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3 }, cancellationToken); } @@ -289,7 +289,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4 }, cancellationToken); } @@ -307,7 +307,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5 }, cancellationToken); } @@ -326,7 +326,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5, arg6 }, cancellationToken); } @@ -346,7 +346,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5, arg6, arg7 }, cancellationToken); } @@ -367,7 +367,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8 }, cancellationToken); } @@ -389,7 +389,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9 }, cancellationToken); } @@ -412,7 +412,7 @@ public static Task InvokeAsync(this ISingleClientProxy clientProxy, string /// The token to monitor for cancellation requests. The default value is . /// A that represents the asynchronous invoke. [SuppressMessage("ApiDesign", "RS0026:Do not add multiple overloads with optional parameters", Justification = "Required to maintain compatibility")] - public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, object? arg10, CancellationToken cancellationToken = default) + public static Task InvokeAsync(this ISingleClientProxy clientProxy, string method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, object? arg10, CancellationToken cancellationToken) { return clientProxy.InvokeCoreAsync(method, new[] { arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10 }, cancellationToken); } diff --git a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs index b8a5d2cc5a06..230af87fc33e 100644 --- a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs +++ b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs @@ -5,7 +5,6 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; -using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.Extensions.Logging; @@ -17,7 +16,6 @@ namespace Microsoft.AspNetCore.SignalR; /// public class DefaultHubLifetimeManager : HubLifetimeManager where THub : Hub { - private static readonly CancellationTokenSourcePool _cancellationTokenSourcePool = new(); private readonly HubConnectionStore _connections = new HubConnectionStore(); private readonly HubGroupList _groups = new HubGroupList(); private readonly ILogger _logger; @@ -328,7 +326,7 @@ public override Task SendUsersAsync(IReadOnlyList userIds, string method } /// - public override async Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken = default) + public override async Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken) { if (connectionId == null) { @@ -344,17 +342,7 @@ public override async Task InvokeConnectionAsync(string connectionId, stri var invocationId = Interlocked.Increment(ref _lastInvocationId).ToString(NumberFormatInfo.InvariantInfo); - CancellationTokenSource? cts = null; - // Add a default timeout if one isn't provided - if (!cancellationToken.CanBeCanceled) - { - cts = _cancellationTokenSourcePool.Rent(); - cts.CancelAfter(TimeSpan.FromSeconds(30)); - cancellationToken = cts.Token; - } - - using var _ = cts; - using var __ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, + using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, connection.ConnectionAborted, out var linkedToken); var task = _clientResultsManager.AddInvocation(connectionId, invocationId, linkedToken); diff --git a/src/SignalR/server/Core/src/HubLifetimeManager.cs b/src/SignalR/server/Core/src/HubLifetimeManager.cs index 14a294190876..f1bc8b058074 100644 --- a/src/SignalR/server/Core/src/HubLifetimeManager.cs +++ b/src/SignalR/server/Core/src/HubLifetimeManager.cs @@ -142,9 +142,9 @@ public abstract class HubLifetimeManager where THub : Hub /// The connection ID. /// The invocation method name. /// The invocation arguments. - /// The token to monitor for cancellation requests. The default value is . + /// The token to monitor for cancellation requests. It is recommended to set a max wait for expecting a result. /// The response from the connection. - public virtual Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken = default) + public virtual Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken) { throw new NotImplementedException($"{GetType().Name} does not support client return values."); } diff --git a/src/SignalR/server/Core/src/ISingleClientProxy.cs b/src/SignalR/server/Core/src/ISingleClientProxy.cs index f400b13e6acc..9a4451e3810e 100644 --- a/src/SignalR/server/Core/src/ISingleClientProxy.cs +++ b/src/SignalR/server/Core/src/ISingleClientProxy.cs @@ -18,7 +18,7 @@ public interface ISingleClientProxy : IClientProxy /// /// Name of the method to invoke. /// A collection of arguments to pass to the client. - /// The token to monitor for cancellation requests. The default value is . + /// The token to monitor for cancellation requests. It is recommended to set a max wait for expecting a result. /// A that represents the asynchronous invoke and wait for a client result. - Task InvokeCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default); + Task InvokeCoreAsync(string method, object?[] args, CancellationToken cancellationToken); } diff --git a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj index ff7bfc4662cf..fd607f3d832b 100644 --- a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj +++ b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj @@ -19,7 +19,6 @@ - diff --git a/src/SignalR/server/Core/src/PublicAPI.Unshipped.txt b/src/SignalR/server/Core/src/PublicAPI.Unshipped.txt index 860477539be5..f7dd250a0ce1 100644 --- a/src/SignalR/server/Core/src/PublicAPI.Unshipped.txt +++ b/src/SignalR/server/Core/src/PublicAPI.Unshipped.txt @@ -5,21 +5,21 @@ Microsoft.AspNetCore.SignalR.IHubCallerClients.Caller.get -> Microsoft.AspNetCor Microsoft.AspNetCore.SignalR.IHubCallerClients.Client(string! connectionId) -> Microsoft.AspNetCore.SignalR.ISingleClientProxy! Microsoft.AspNetCore.SignalR.IHubClients.Client(string! connectionId) -> Microsoft.AspNetCore.SignalR.ISingleClientProxy! Microsoft.AspNetCore.SignalR.ISingleClientProxy -Microsoft.AspNetCore.SignalR.ISingleClientProxy.InvokeCoreAsync(string! method, object?[]! args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -override Microsoft.AspNetCore.SignalR.DefaultHubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Microsoft.AspNetCore.SignalR.ISingleClientProxy.InvokeCoreAsync(string! method, object?[]! args, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +override Microsoft.AspNetCore.SignalR.DefaultHubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! override Microsoft.AspNetCore.SignalR.DefaultHubLifetimeManager.SetConnectionResultAsync(string! connectionId, Microsoft.AspNetCore.SignalR.Protocol.CompletionMessage! result) -> System.Threading.Tasks.Task! override Microsoft.AspNetCore.SignalR.DefaultHubLifetimeManager.TryGetReturnType(string! invocationId, out System.Type? type) -> bool -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, object? arg10, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -virtual Microsoft.AspNetCore.SignalR.HubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, object? arg10, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, object? arg9, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, object? arg4, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, object? arg3, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, object? arg2, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, object? arg1, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.SignalR.ClientProxyExtensions.InvokeAsync(this Microsoft.AspNetCore.SignalR.ISingleClientProxy! clientProxy, string! method, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! +virtual Microsoft.AspNetCore.SignalR.HubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! virtual Microsoft.AspNetCore.SignalR.HubLifetimeManager.SetConnectionResultAsync(string! connectionId, Microsoft.AspNetCore.SignalR.Protocol.CompletionMessage! result) -> System.Threading.Tasks.Task! virtual Microsoft.AspNetCore.SignalR.HubLifetimeManager.TryGetReturnType(string! invocationId, out System.Type? type) -> bool diff --git a/src/SignalR/server/SignalR/test/ClientProxyTests.cs b/src/SignalR/server/SignalR/test/ClientProxyTests.cs index 784fc98bf01f..708935fc8e6d 100644 --- a/src/SignalR/server/SignalR/test/ClientProxyTests.cs +++ b/src/SignalR/server/SignalR/test/ClientProxyTests.cs @@ -213,7 +213,7 @@ public async Task SingleClientProxyWithInvoke_ThrowsNotSupported() var hubLifetimeManager = new EmptyHubLifetimeManager(); var proxy = new SingleClientProxy(hubLifetimeManager, ""); - var ex = await Assert.ThrowsAsync(async () => await proxy.InvokeAsync("method")).DefaultTimeout(); + var ex = await Assert.ThrowsAsync(async () => await proxy.InvokeAsync("method", cancellationToken: default)).DefaultTimeout(); Assert.Equal("EmptyHubLifetimeManager`1 does not support client return values.", ex.Message); } diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs index bd7bacf13442..0b659acb416e 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs @@ -335,7 +335,7 @@ public async Task BlockingMethod() public async Task GetClientResult(int num) { - var sum = await Clients.Caller.InvokeAsync("Sum", num); + var sum = await Clients.Caller.InvokeAsync("Sum", num, cancellationToken: default); return sum; } } @@ -1260,7 +1260,7 @@ public class OnConnectedClientResultHub : Hub { public override async Task OnConnectedAsync() { - await Clients.Caller.InvokeAsync("Test"); + await Clients.Caller.InvokeAsync("Test", cancellationToken: default); } } @@ -1268,7 +1268,7 @@ public class OnDisconnectedClientResultHub : Hub { public override async Task OnDisconnectedAsync(Exception ex) { - await Clients.Caller.InvokeAsync("Test"); + await Clients.Caller.InvokeAsync("Test", cancellationToken: default); } } diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs index 8bc62b632a45..74b5dcec0b8f 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs @@ -180,7 +180,7 @@ public async Task CanUseClientResultsWithIHubContext() await client.Connected.OrThrowIfOtherFails(connectionHandlerTask).DefaultTimeout(); var context = serviceProvider.GetRequiredService>(); - var resultTask = context.Clients.Client(client.Connection.ConnectionId).InvokeAsync("GetClientResult", 1); + var resultTask = context.Clients.Client(client.Connection.ConnectionId).InvokeAsync("GetClientResult", 1, cancellationToken: default); var message = await client.ReadAsync().DefaultTimeout(); var invocation = Assert.IsType(message); diff --git a/src/SignalR/server/Specification.Tests/src/HubLifetimeManagerTestBase.cs b/src/SignalR/server/Specification.Tests/src/HubLifetimeManagerTestBase.cs index 19b061d1c6bd..426a06ddeebc 100644 --- a/src/SignalR/server/Specification.Tests/src/HubLifetimeManagerTestBase.cs +++ b/src/SignalR/server/Specification.Tests/src/HubLifetimeManagerTestBase.cs @@ -186,7 +186,7 @@ public async Task CanProcessClientReturnResult() await manager.OnConnectedAsync(connection1).DefaultTimeout(); - var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotNull(invocation.InvocationId); Assert.Equal("test", invocation.Arguments[0]); @@ -213,7 +213,7 @@ public async Task CanProcessClientReturnErrorResult() await manager.OnConnectedAsync(connection1).DefaultTimeout(); - var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotNull(invocation.InvocationId); Assert.Equal("test", invocation.Arguments[0]); @@ -243,7 +243,7 @@ public async Task ExceptionWhenIncorrectClientCompletesClientResult() await manager.OnConnectedAsync(connection1).DefaultTimeout(); await manager.OnConnectedAsync(connection2).DefaultTimeout(); - var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var resultTask = manager.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotNull(invocation.InvocationId); Assert.Equal("test", invocation.Arguments[0]); @@ -277,7 +277,7 @@ public async Task ConnectionIDNotPresentWhenInvokingClientResult() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); // No client with this ID - await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("none", "Result", new object[] { "test" })).DefaultTimeout(); + await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("none", "Result", new object[] { "test" }, cancellationToken: default)).DefaultTimeout(); } } @@ -299,8 +299,8 @@ public async Task InvokesForMultipleClientsDoNotCollide() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); await manager1.OnConnectedAsync(connection2).DefaultTimeout(); - var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); - var invoke2 = manager1.InvokeConnectionAsync(connection2.ConnectionId, "Result", new object[] { "test" }); + var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); + var invoke2 = manager1.InvokeConnectionAsync(connection2.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation1 = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); var invocation2 = Assert.IsType(await client2.ReadAsync().DefaultTimeout()); @@ -329,7 +329,7 @@ public async Task ClientDisconnectsWithoutCompletingClientResult() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); - var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); connection1.Abort(); await manager1.OnDisconnectedAsync(connection1).DefaultTimeout(); diff --git a/src/SignalR/server/Specification.Tests/src/ScaleoutHubLifetimeManagerTests.cs b/src/SignalR/server/Specification.Tests/src/ScaleoutHubLifetimeManagerTests.cs index 5fc1c9637c76..fcaad6c86345 100644 --- a/src/SignalR/server/Specification.Tests/src/ScaleoutHubLifetimeManagerTests.cs +++ b/src/SignalR/server/Specification.Tests/src/ScaleoutHubLifetimeManagerTests.cs @@ -482,7 +482,7 @@ public async Task CanProcessClientReturnResultAcrossServers() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); // Server2 asks for a result from client1 on Server1 - var resultTask = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var resultTask = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotNull(invocation.InvocationId); Assert.Equal("test", invocation.Arguments[0]); @@ -513,7 +513,7 @@ public async Task CanProcessClientReturnErrorResultAcrossServers() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); // Server2 asks for a result from client1 on Server1 - var resultTask = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var resultTask = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotNull(invocation.InvocationId); Assert.Equal("test", invocation.Arguments[0]); @@ -544,7 +544,7 @@ public async Task ConnectionIDNotPresentMultiServerWhenInvokingClientResult() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); // No client on any backplanes with this ID - await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("none", "Result", new object[] { "test" })).DefaultTimeout(); + await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("none", "Result", new object[] { "test" }, cancellationToken: default)).DefaultTimeout(); } } @@ -565,7 +565,7 @@ public async Task ClientDisconnectsWithoutCompletingClientResultOnSecondServer() await manager2.OnConnectedAsync(connection1).DefaultTimeout(); - var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var invoke1 = manager1.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); connection1.Abort(); @@ -597,10 +597,10 @@ public async Task InvocationsFromDifferentServersUseUniqueIDs() await manager1.OnConnectedAsync(connection1).DefaultTimeout(); await manager2.OnConnectedAsync(connection2).DefaultTimeout(); - var invoke1 = manager1.InvokeConnectionAsync(connection2.ConnectionId, "Result", new object[] { "test" }); + var invoke1 = manager1.InvokeConnectionAsync(connection2.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation2 = Assert.IsType(await client2.ReadAsync().DefaultTimeout()); - var invoke2 = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }); + var invoke2 = manager2.InvokeConnectionAsync(connection1.ConnectionId, "Result", new object[] { "test" }, cancellationToken: default); var invocation1 = Assert.IsType(await client1.ReadAsync().DefaultTimeout()); Assert.NotEqual(invocation1.InvocationId, invocation2.InvocationId); @@ -626,7 +626,7 @@ public async Task ConnectionDoesNotExist_FailsInvokeConnectionAsync() var manager1 = CreateNewHubLifetimeManager(backplane); var manager2 = CreateNewHubLifetimeManager(backplane); - var ex = await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("1234", "Result", new object[] { "test" })).DefaultTimeout(); + var ex = await Assert.ThrowsAsync(() => manager1.InvokeConnectionAsync("1234", "Result", new object[] { "test" }, cancellationToken: default)).DefaultTimeout(); Assert.Equal("Connection '1234' does not exist.", ex.Message); } } diff --git a/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj b/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj index 228b5ccb2d75..e3ed270e72ef 100644 --- a/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj +++ b/src/SignalR/server/StackExchangeRedis/src/Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj @@ -12,7 +12,6 @@ - diff --git a/src/SignalR/server/StackExchangeRedis/src/PublicAPI.Unshipped.txt b/src/SignalR/server/StackExchangeRedis/src/PublicAPI.Unshipped.txt index 3a2eac25c41d..09d6bc878431 100644 --- a/src/SignalR/server/StackExchangeRedis/src/PublicAPI.Unshipped.txt +++ b/src/SignalR/server/StackExchangeRedis/src/PublicAPI.Unshipped.txt @@ -1,4 +1,4 @@ #nullable enable -override Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +override Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager.InvokeConnectionAsync(string! connectionId, string! methodName, object?[]! args, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task! override Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager.SetConnectionResultAsync(string! connectionId, Microsoft.AspNetCore.SignalR.Protocol.CompletionMessage! result) -> System.Threading.Tasks.Task! override Microsoft.AspNetCore.SignalR.StackExchangeRedis.RedisHubLifetimeManager.TryGetReturnType(string! invocationId, out System.Type? type) -> bool diff --git a/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs b/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs index d58020e5e511..1d8397272ab4 100644 --- a/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs +++ b/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Text; using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal; @@ -23,8 +22,6 @@ namespace Microsoft.AspNetCore.SignalR.StackExchangeRedis; /// The type of to manage connections for. public class RedisHubLifetimeManager : HubLifetimeManager, IDisposable where THub : Hub { - private static readonly CancellationTokenSourcePool _cancellationTokenSourcePool = new(); - private readonly HubConnectionStore _connections = new HubConnectionStore(); private readonly RedisSubscriptionManager _groups = new RedisSubscriptionManager(); private readonly RedisSubscriptionManager _users = new RedisSubscriptionManager(); @@ -409,7 +406,7 @@ public void Dispose() } /// - public override async Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken = default) + public override async Task InvokeConnectionAsync(string connectionId, string methodName, object?[] args, CancellationToken cancellationToken) { // send thing if (connectionId == null) @@ -422,17 +419,7 @@ public override async Task InvokeConnectionAsync(string connectionId, stri // Needs to be unique across servers, easiest way to do that is prefix with connection ID. var invocationId = $"{connectionId}{Interlocked.Increment(ref _lastInvocationId)}"; - CancellationTokenSource? cts = null; - // Add a default timeout if one isn't provided - if (!cancellationToken.CanBeCanceled) - { - cts = _cancellationTokenSourcePool.Rent(); - cts.CancelAfter(TimeSpan.FromSeconds(30)); - cancellationToken = cts.Token; - } - - using var _ = cts; - using var __ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, + using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken, connection?.ConnectionAborted ?? default, out var linkedToken); var task = _clientResultsManager.AddInvocation(connectionId, invocationId, linkedToken); From 654a22943199cb398e3f945966f692606a804bb2 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Wed, 17 Aug 2022 10:08:30 -0700 Subject: [PATCH 4/5] fix build --- .../Server/src/Microsoft.AspNetCore.Components.Server.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj index 884967d73d78..8c3fd43461fb 100644 --- a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj +++ b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj @@ -69,6 +69,7 @@ + From bee820e54bb15e6bdb58d8dd9fc142430a2bc925 Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 19 Aug 2022 11:50:58 -0700 Subject: [PATCH 5/5] Update src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs --- .../src/Protocol/NewtonsoftJsonHubProtocol.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs index 2a0733a3790d..11dd9d107adb 100644 --- a/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs +++ b/src/SignalR/common/Protocols.NewtonsoftJson/src/Protocol/NewtonsoftJsonHubProtocol.cs @@ -217,7 +217,6 @@ public ReadOnlyMemory GetMessageBytes(HubMessage message) } else { - if (!JsonUtils.ReadForType(reader, returnType)) { throw new JsonReaderException("Unexpected end when reading JSON");