Skip to content

Commit 99e056f

Browse files
committed
Fix race with CTS disposing
1 parent a2a9dce commit 99e056f

File tree

2 files changed

+39
-21
lines changed

2 files changed

+39
-21
lines changed

src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ private async Task WaitOnTasks(Task applicationTask, Task transportTask, bool cl
249249

250250
if (TransportType == HttpTransportType.WebSockets)
251251
{
252-
// The websocket transport will close the application output automatically when reading is cancelled
252+
// The websocket transport will close the application output automatically when reading is canceled
253253
Cancellation?.Cancel();
254254
}
255255
else
@@ -439,6 +439,37 @@ private void FailActivationUnsynchronized(HttpContext nonClonedContext, ILogger
439439
}
440440
}
441441

442+
internal async Task<bool> CancelPreviousPoll(HttpContext context)
443+
{
444+
CancellationTokenSource cts;
445+
lock (_stateLock)
446+
{
447+
// Need to sync cts access with DisposeAsync as that will dispose the cts
448+
cts = Status == HttpConnectionStatus.Disposed ? null : Cancellation;
449+
}
450+
451+
using (cts)
452+
{
453+
// Cancel the previous request
454+
cts?.Cancel();
455+
456+
try
457+
{
458+
// Wait for the previous request to drain
459+
await PreviousPollTask;
460+
}
461+
catch (OperationCanceledException)
462+
{
463+
// Previous poll canceled due to connection closing, close this poll too
464+
context.Response.ContentType = "text/plain";
465+
context.Response.StatusCode = StatusCodes.Status204NoContent;
466+
return false;
467+
}
468+
469+
return true;
470+
}
471+
}
472+
442473
public void MarkInactive()
443474
{
444475
lock (_stateLock)

src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private async Task ExecuteAsync(HttpContext context, ConnectionDelegate connecti
155155

156156
Log.EstablishedConnection(_logger);
157157

158-
// Allow the reads to be cancelled
158+
// Allow the reads to be canceled
159159
connection.Cancellation = new CancellationTokenSource();
160160

161161
var ws = new WebSocketsTransport(options.WebSockets, connection.Application, connection, _loggerFactory);
@@ -180,28 +180,15 @@ private async Task ExecuteAsync(HttpContext context, ConnectionDelegate connecti
180180
return;
181181
}
182182

183-
// Create a new Tcs every poll to keep track of the poll finishing, so we can properly wait on previous polls
184-
var currentRequestTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
185-
186-
using (connection.Cancellation)
183+
if (!await connection.CancelPreviousPoll(context))
187184
{
188-
// Cancel the previous request
189-
connection.Cancellation?.Cancel();
190-
191-
try
192-
{
193-
// Wait for the previous request to drain
194-
await connection.PreviousPollTask;
195-
}
196-
catch (OperationCanceledException)
197-
{
198-
// Previous poll canceled due to connection closing, close this poll too
199-
context.Response.ContentType = "text/plain";
200-
context.Response.StatusCode = StatusCodes.Status204NoContent;
201-
return;
202-
}
185+
// Connection closed. It's already set the response status code.
186+
return;
203187
}
204188

189+
// Create a new Tcs every poll to keep track of the poll finishing, so we can properly wait on previous polls
190+
var currentRequestTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
191+
205192
if (!connection.TryActivateLongPollingConnection(
206193
connectionDelegate, context, options.LongPolling.PollTimeout,
207194
currentRequestTcs.Task, _loggerFactory, _logger))

0 commit comments

Comments
 (0)