From 84205e70790e31a7b2236e76e3ae53c5de1b211e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Oct 2025 19:44:00 +0000 Subject: [PATCH 1/4] Initial plan From db1c426d6d8ee3ab051d5f628170ab2aa67c891f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Oct 2025 19:55:18 +0000 Subject: [PATCH 2/4] Implement cancellation of regular hub invocations from client Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com> --- .../csharp/Client.Core/src/HubConnection.cs | 36 +++++++++++++++++++ .../Core/src/Internal/DefaultHubDispatcher.cs | 10 +++--- .../src/Internal/DefaultHubDispatcherLog.cs | 8 ++--- .../Core/src/Internal/HubMethodDescriptor.cs | 4 +-- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs index f52e5ba32357..a565e828f3f3 100644 --- a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs +++ b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs @@ -1078,6 +1078,37 @@ static void SetServerTags(Activity? activity, Uri? uri) private async Task InvokeCoreAsyncCore(string methodName, Type returnType, object?[] args, CancellationToken cancellationToken) { + async Task OnInvocationCanceled(InvocationRequest irq) + { + // We need to take the connection lock in order to ensure we a) have a connection and b) are the only one accessing the write end of the pipe. + await _state.WaitConnectionLockAsync(token: default).ConfigureAwait(false); + try + { + if (_state.CurrentConnectionStateUnsynchronized != null) + { + Log.SendingCancellation(_logger, irq.InvocationId); + + // Don't pass irq.CancellationToken, that would result in canceling the Flush and a delayed CancelInvocationMessage being sent. + await SendHubMessage(_state.CurrentConnectionStateUnsynchronized, new CancelInvocationMessage(irq.InvocationId), cancellationToken: default).ConfigureAwait(false); + } + else + { + Log.UnableToSendCancellation(_logger, irq.InvocationId); + } + } + catch + { + // Connection closed while trying to cancel an invocation. This is fine to ignore. + } + finally + { + _state.ReleaseConnectionLock(); + } + + // Cancel the invocation + irq.Dispose(); + } + var readers = default(Dictionary); CheckDisposed(); @@ -1094,6 +1125,11 @@ static void SetServerTags(Activity? activity, Uri? uri) var irq = InvocationRequest.Invoke(cancellationToken, returnType, connectionState.GetNextId(), _loggerFactory, this, activity, out invocationTask); await InvokeCore(connectionState, methodName, irq, args, streamIds?.ToArray(), cancellationToken).ConfigureAwait(false); + if (cancellationToken.CanBeCanceled) + { + cancellationToken.Register(state => _ = OnInvocationCanceled((InvocationRequest)state!), irq); + } + LaunchStreams(connectionState, readers, cancellationToken); } finally diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index 5e914b31e29e..af6ce8de138c 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -181,17 +181,17 @@ public override Task DispatchMessageAsync(HubConnectionContext connection, HubMe return ProcessInvocation(connection, streamInvocationMessage, isStreamResponse: true); case CancelInvocationMessage cancelInvocationMessage: - // Check if there is an associated active stream and cancel it if it exists. - // The cts will be removed when the streaming method completes executing + // Check if there is an associated active invocation or stream and cancel it if it exists. + // The cts will be removed when the hub method completes executing if (connection.ActiveRequestCancellationSources.TryGetValue(cancelInvocationMessage.InvocationId!, out var cts)) { - Log.CancelStream(_logger, cancelInvocationMessage.InvocationId!); + Log.CancelInvocation(_logger, cancelInvocationMessage.InvocationId!); cts.Cancel(); } else { - // Stream can be canceled on the server while client is canceling stream. - Log.UnexpectedCancel(_logger); + // Invocation can be canceled on the server while client is canceling invocation. + Log.UnexpectedCancel(_logger, cancelInvocationMessage.InvocationId!); } break; diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcherLog.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcherLog.cs index dce901a2d952..12f2a213884b 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcherLog.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcherLog.cs @@ -53,11 +53,11 @@ public static void SendingResult(ILogger logger, string? invocationId, ObjectMet [LoggerMessage(9, LogLevel.Trace, "'{HubName}' hub method '{HubMethod}' is bound.", EventName = "HubMethodBound")] public static partial void HubMethodBound(ILogger logger, string hubName, string hubMethod); - [LoggerMessage(10, LogLevel.Debug, "Canceling stream for invocation {InvocationId}.", EventName = "CancelStream")] - public static partial void CancelStream(ILogger logger, string invocationId); + [LoggerMessage(10, LogLevel.Debug, "Canceling invocation {InvocationId}.", EventName = "CancelInvocation")] + public static partial void CancelInvocation(ILogger logger, string invocationId); - [LoggerMessage(11, LogLevel.Debug, "CancelInvocationMessage received unexpectedly.", EventName = "UnexpectedCancel")] - public static partial void UnexpectedCancel(ILogger logger); + [LoggerMessage(11, LogLevel.Debug, "CancelInvocationMessage received for {InvocationId} but invocation was not found.", EventName = "UnexpectedCancel")] + public static partial void UnexpectedCancel(ILogger logger, string invocationId); [LoggerMessage(12, LogLevel.Debug, "Received stream hub invocation: {InvocationMessage}.", EventName = "ReceivedStreamHubInvocation")] public static partial void ReceivedStreamHubInvocation(ILogger logger, StreamInvocationMessage invocationMessage); diff --git a/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs b/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs index f32e5a9afc90..a55687250622 100644 --- a/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs +++ b/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs @@ -60,8 +60,8 @@ public HubMethodDescriptor(ObjectMethodExecutor methodExecutor, IServiceProvider // Take out synthetic arguments that will be provided by the server, this list will be given to the protocol parsers ParameterTypes = methodExecutor.MethodParameters.Where((p, index) => { - // Only streams can take CancellationTokens currently - if (IsStreamResponse && p.ParameterType == typeof(CancellationToken)) + // CancellationTokens are synthetic arguments provided by the server + if (p.ParameterType == typeof(CancellationToken)) { HasSyntheticArguments = true; return false; From c618b86d048d8887e30c9ad2e8768216628f8332 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Oct 2025 20:03:13 +0000 Subject: [PATCH 3/4] Add tests for hub method cancellation and fix server-side registration Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com> --- .../test/UnitTests/HubConnectionTests.cs | 29 +++++++++++++++++++ .../Core/src/Internal/DefaultHubDispatcher.cs | 23 +++++++++++++-- .../HubConnectionHandlerTestUtils/Hubs.cs | 8 +++++ .../HubConnectionHandlerTests.cs | 25 ++++++++++++---- 4 files changed, 77 insertions(+), 8 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs index 48f792c6b72a..f5d990233432 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs @@ -342,6 +342,35 @@ await connection.ReceiveJsonMessage( } } + [Fact] + public async Task CanCancelTokenDuringInvoke_SendsCancelInvocation() + { + using (StartVerifiableLog()) + { + var connection = new TestConnection(); + var hubConnection = CreateHubConnection(connection, loggerFactory: LoggerFactory); + + await hubConnection.StartAsync().DefaultTimeout(); + + using var cts = new CancellationTokenSource(); + var invokeTask = hubConnection.InvokeAsync("TestMethod", cts.Token); + + var item = await connection.ReadSentJsonAsync().DefaultTimeout(); + var invocationId = item["invocationId"]; + + // Cancel the invocation + cts.Cancel(); + + // Should receive CancelInvocationMessage + item = await connection.ReadSentJsonAsync().DefaultTimeout(); + Assert.Equal(HubProtocolConstants.CancelInvocationMessageType, item["type"]); + Assert.Equal(invocationId, item["invocationId"]); + + // Invocation on client-side completes with cancellation + await Assert.ThrowsAsync(async () => await invokeTask).DefaultTimeout(); + } + } + [Fact] public async Task ConnectionTerminatedIfServerTimeoutIntervalElapsesWithNoMessages() { diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index af6ce8de138c..f6313960ced0 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -390,7 +390,8 @@ static async Task ExecuteInvocation(DefaultHubDispatcher dispatcher, IHubActivator hubActivator, HubConnectionContext connection, HubMethodInvocationMessage hubMethodInvocationMessage, - bool isStreamCall) + bool isStreamCall, + CancellationTokenSource? cts) { var logger = dispatcher._logger; var enableDetailedErrors = dispatcher._enableDetailedErrors; @@ -406,6 +407,18 @@ static async Task ExecuteInvocation(DefaultHubDispatcher dispatcher, // We want to take HubMethodNameAttribute into account which will be the same as what the invocation target is var activity = StartActivity(SignalRServerActivitySource.InvocationIn, ActivityKind.Server, connection.OriginalActivity, scope.ServiceProvider, hubMethodInvocationMessage.Target, hubMethodInvocationMessage.Headers, logger); + // Register the CancellationTokenSource if present so CancelInvocationMessage can cancel it + if (cts != null && !string.IsNullOrEmpty(hubMethodInvocationMessage.InvocationId)) + { + if (!connection.ActiveRequestCancellationSources.TryAdd(hubMethodInvocationMessage.InvocationId, cts)) + { + Log.InvocationIdInUse(logger, hubMethodInvocationMessage.InvocationId); + await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, + $"Invocation ID '{hubMethodInvocationMessage.InvocationId}' is already in use."); + return; + } + } + object? result; try { @@ -430,6 +443,12 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, Activity.Current = previousActivity; } + // Remove the CancellationTokenSource from active requests + if (cts != null && !string.IsNullOrEmpty(hubMethodInvocationMessage.InvocationId)) + { + connection.ActiveRequestCancellationSources.TryRemove(hubMethodInvocationMessage.InvocationId, out _); + } + // Stream response handles cleanup in StreamResultsAsync // And normal invocations handle cleanup below in the finally if (isStreamCall) @@ -446,7 +465,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, } } - invocation = ExecuteInvocation(this, methodExecutor, hub, arguments, scope, hubActivator, connection, hubMethodInvocationMessage, isStreamCall); + invocation = ExecuteInvocation(this, methodExecutor, hub, arguments, scope, hubActivator, connection, hubMethodInvocationMessage, isStreamCall, cts); } if (isStreamCall || isStreamResponse) diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index 90fabf90265b..622728038504 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -1022,6 +1022,14 @@ public async Task LongRunningMethod() return 12; } + public async Task CancelableInvocation(CancellationToken token) + { + _tcsService.StartedMethod.SetResult(null); + await token.WaitForCancellationAsync(); + _tcsService.EndMethod.SetResult(null); + return 42; + } + public async Task> LongRunningStream() { _tcsService.StartedMethod.TrySetResult(null); diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index 486f6c53ea02..fc817bdb8fb5 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -4376,22 +4376,35 @@ public async Task StreamHubMethodCanAcceptNullableParameterWithCancellationToken } [Fact] - public async Task InvokeHubMethodCannotAcceptCancellationTokenAsArgument() + public async Task InvokeHubMethodCanAcceptCancellationTokenAsArgument() { using (StartVerifiableLog()) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(null, LoggerFactory); - var connectionHandler = serviceProvider.GetService>(); + var tcsService = new TcsService(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + { + builder.AddSingleton(tcsService); + }, LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); using (var client = new TestClient()) { var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); - var invocationId = await client.SendInvocationAsync(nameof(MethodHub.InvalidArgument)).DefaultTimeout(); + var invocationId = await client.SendInvocationAsync(nameof(LongRunningHub.CancelableInvocation)).DefaultTimeout(); + // Wait for the hub method to start + await tcsService.StartedMethod.Task.DefaultTimeout(); - var completion = Assert.IsType(await client.ReadAsync().DefaultTimeout()); + // Cancel the invocation which should trigger the CancellationToken in the hub method + await client.SendHubMessageAsync(new CancelInvocationMessage(invocationId)).DefaultTimeout(); + + var result = await client.ReadAsync().DefaultTimeout(); - Assert.Equal("Failed to invoke 'InvalidArgument' due to an error on the server.", completion.Error); + var completion = Assert.IsType(result); + Assert.Null(completion.Error); + + // CancellationToken passed to hub method will allow EndMethod to be triggered if it is canceled. + await tcsService.EndMethod.Task.DefaultTimeout(); client.Dispose(); From e410c32b008a5bbbae10e980ae46f6840bd8f484 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Oct 2025 20:10:13 +0000 Subject: [PATCH 4/4] Address code review feedback - add null-forgiving operators and clarifying comment Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com> --- src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs | 4 ++-- .../HubConnectionHandlerTestUtils/Hubs.cs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index f6313960ced0..9054f83c8bc6 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -410,7 +410,7 @@ static async Task ExecuteInvocation(DefaultHubDispatcher dispatcher, // Register the CancellationTokenSource if present so CancelInvocationMessage can cancel it if (cts != null && !string.IsNullOrEmpty(hubMethodInvocationMessage.InvocationId)) { - if (!connection.ActiveRequestCancellationSources.TryAdd(hubMethodInvocationMessage.InvocationId, cts)) + if (!connection.ActiveRequestCancellationSources.TryAdd(hubMethodInvocationMessage.InvocationId!, cts)) { Log.InvocationIdInUse(logger, hubMethodInvocationMessage.InvocationId); await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, @@ -446,7 +446,7 @@ await SendInvocationError(hubMethodInvocationMessage.InvocationId, connection, // Remove the CancellationTokenSource from active requests if (cts != null && !string.IsNullOrEmpty(hubMethodInvocationMessage.InvocationId)) { - connection.ActiveRequestCancellationSources.TryRemove(hubMethodInvocationMessage.InvocationId, out _); + connection.ActiveRequestCancellationSources.TryRemove(hubMethodInvocationMessage.InvocationId!, out _); } // Stream response handles cleanup in StreamResultsAsync diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index 622728038504..385c22df02c5 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -1025,6 +1025,7 @@ public async Task LongRunningMethod() public async Task CancelableInvocation(CancellationToken token) { _tcsService.StartedMethod.SetResult(null); + // Wait for cancellation. Test timeout is enforced by .DefaultTimeout() in the test itself. await token.WaitForCancellationAsync(); _tcsService.EndMethod.SetResult(null); return 42;