Conversation
Cancellation with stateful reconnect needs to be more aggressive.
BrennanConroy
approved these changes
Mar 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes cancellation during SignalR stateful reconnect more aggressive so that writes blocked by MessageBuffer backpressure can be unblocked when a connection is closing/aborted, and adds/updates tests to validate the behavior.
Changes:
- Link
MessageBuffer.WriteAsynccancellation to connection-abort tokens (with a short-lived token forCloseMessage) on both server and client. - Add/adjust tests to cover cancelability under backpressure and update expectations for cancellation behavior.
- Add a debug assertion in
MessageBufferrequiring a cancelable token during backpressure waits.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SignalR/server/Core/src/HubConnectionContext.cs | Link cancellation to abort tokens for stateful reconnect writes; adjust exception handling around canceled writes |
| src/SignalR/common/Shared/MessageBuffer.cs | Assert a cancelable token is used so backpressure waits can be canceled |
| src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/Internal/MessageBufferTests.cs | Pass cancelable tokens to MessageBuffer.WriteAsync and add a new cancellation/backpressure test |
| src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs | Add a theory validating stateful reconnect backpressure is cancelable during close scenarios |
| src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/DefaultHubLifetimeManagerTests.cs | Update tests to assert cancellation throws OperationCanceledException and does not abort connections |
| src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs | Link client-side MessageBuffer writes to stop token; use short-lived token for sending CloseMessage |
| src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.cs | Verify a CloseMessage is actually sent when stopping with stateful reconnect enabled |
Comments suppressed due to low confidence (2)
src/SignalR/server/Core/src/HubConnectionContext.cs:401
- Same issue as CompleteWriteAsync: this catch filter only checks the method cancellationToken. If the write is canceled because the connection was aborted (linked token from ConnectionAborted), this path will still log FailedWritingMessage and set CloseException. Consider excluding OperationCanceledException when the connection’s abort token is canceled (or otherwise tying the filter to the linked token actually used by WriteCore) to avoid turning an expected shutdown into an error.
// We care about errors while serializing to the PipeWriter as that will leave the Pipe
// in an invalid (for our scenario) state. OCE shouldn't occur while serializing bytes and
// writing to the Pipe. We assume that PipeWriter.WriteAsync(buffer) always writes the full message before calling FlushAsync
catch (Exception ex) when (ex is not OperationCanceledException || !cancellationToken.IsCancellationRequested)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);
AbortAllowReconnect();
src/SignalR/server/Core/src/HubConnectionContext.cs:430
- Same cancellation-filter problem for serialized writes: OperationCanceledException caused by ConnectionAborted (via the linked token in WriteCore) will still be logged/recorded as a failure because this filter only checks the caller cancellationToken. Consider also checking ConnectionAborted (or the linked token) so abort-driven cancellation doesn’t produce FailedWritingMessage/CloseException.
// We care about errors while serializing to the PipeWriter as that will leave the Pipe
// in an invalid (for our scenario) state. OCE shouldn't occur while serializing bytes and
// writing to the Pipe. We assume that PipeWriter.WriteAsync(buffer) always writes the full message before calling FlushAsync
catch (Exception ex) when (ex is not OperationCanceledException || !cancellationToken.IsCancellationRequested)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);
AbortAllowReconnect();
This was referenced Mar 11, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cancellation with stateful reconnect needs to be more aggressive.