diff --git a/eng/Baseline.Designer.props b/eng/Baseline.Designer.props index 0550859109..b0493f1b1a 100644 --- a/eng/Baseline.Designer.props +++ b/eng/Baseline.Designer.props @@ -2,7 +2,11 @@ $(MSBuildAllProjects);$(MSBuildThisFileFullPath) +<<<<<<< HEAD 3.1.1 +======= + 2.1.15 +>>>>>>> release/2.1 diff --git a/src/PackageArchive/Archive.CiServer.Patch.Compat/ArchiveBaseline.2.1.15.txt b/src/PackageArchive/Archive.CiServer.Patch.Compat/ArchiveBaseline.2.1.15.txt new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/src/PackageArchive/Archive.CiServer.Patch.Compat/ArchiveBaseline.2.1.15.txt @@ -0,0 +1 @@ + diff --git a/src/PackageArchive/Archive.CiServer.Patch/ArchiveBaseline.2.1.15.txt b/src/PackageArchive/Archive.CiServer.Patch/ArchiveBaseline.2.1.15.txt new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/src/PackageArchive/Archive.CiServer.Patch/ArchiveBaseline.2.1.15.txt @@ -0,0 +1 @@ + diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs index 6d01fae4e9..5d4a94ec56 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs @@ -35,7 +35,7 @@ namespace OpenIdConnectSample private void CheckSameSite(HttpContext httpContext, CookieOptions options) { - if (options.SameSite > SameSiteMode.Unspecified) + if (options.SameSite == SameSiteMode.None) { var userAgent = httpContext.Request.Headers["User-Agent"]; // TODO: Use your User Agent library of choice here. diff --git a/src/Security/Authentication/test/WsFederation/WsFederationTest.cs b/src/Security/Authentication/test/WsFederation/WsFederationTest.cs index 6cc25f6dd5..1c4fc9e94d 100644 --- a/src/Security/Authentication/test/WsFederation/WsFederationTest.cs +++ b/src/Security/Authentication/test/WsFederation/WsFederationTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -440,4 +440,4 @@ namespace Microsoft.AspNetCore.Authentication.WsFederation } } } -} \ No newline at end of file +} diff --git a/src/Security/CookiePolicy/test/CookieChunkingTests.cs b/src/Security/CookiePolicy/test/CookieChunkingTests.cs index 4b65df4073..a8b8c59305 100644 --- a/src/Security/CookiePolicy/test/CookieChunkingTests.cs +++ b/src/Security/CookiePolicy/test/CookieChunkingTests.cs @@ -44,6 +44,29 @@ namespace Microsoft.AspNetCore.Internal Assert.Equal($"TestCookie={testString}; expires={now.AddMinutes(5).ToString("R")}; max-age=300; domain=foo.com; path=/bar; secure; samesite=strict; httponly", values[0]); } + [Fact] + public void AppendLargeCookie_WithOptions_Appended() + { + HttpContext context = new DefaultHttpContext(); + var now = DateTimeOffset.UtcNow; + var options = new CookieOptions + { + Domain = "foo.com", + HttpOnly = true, + SameSite = SameSiteMode.Strict, + Path = "/bar", + Secure = true, + Expires = now.AddMinutes(5), + MaxAge = TimeSpan.FromMinutes(5) + }; + var testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + new ChunkingCookieManager() { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, options); + + var values = context.Response.Headers["Set-Cookie"]; + Assert.Single(values); + Assert.Equal($"TestCookie={testString}; expires={now.AddMinutes(5).ToString("R")}; max-age=300; domain=foo.com; path=/bar; secure; samesite=strict; httponly", values[0]); + } + [Fact] public void AppendLargeCookieWithLimit_Chunked() { diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index abf6b69524..1d617a3f51 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -33,7 +33,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal { private static long _tenSeconds = TimeSpan.FromSeconds(10).Ticks; +<<<<<<< HEAD private readonly object _stateLock = new object(); +======= +>>>>>>> release/2.1 private readonly object _itemsLock = new object(); private readonly object _heartbeatLock = new object(); private List<(Action handler, object state)> _heartbeatHandlers; @@ -46,6 +49,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal private bool _activeSend; private long _startedSendTime; private readonly object _sendingLock = new object(); +<<<<<<< HEAD +======= + +>>>>>>> release/2.1 internal CancellationToken SendingToken { get; private set; } // This tcs exists so that multiple calls to DisposeAsync all wait asynchronously @@ -292,6 +299,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal // Wait for either to finish var result = await Task.WhenAny(applicationTask, transportTask); +<<<<<<< HEAD // If the application is complete, complete the transport pipe (it's the pipe to the transport) if (result == applicationTask) { @@ -333,7 +341,47 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal Transport?.Output.Complete(); Transport?.Input.Complete(); +======= + // Normally it isn't safe to try and acquire this lock because the Send can hold onto it for a long time if there is backpressure + // It is safe to wait for this lock now because the Send will be in one of 4 states + // 1. In the middle of a write which is in the middle of being canceled by the CancelPendingFlush above, when it throws + // an OperationCanceledException it will complete the PipeWriter which will make any other Send waiting on the lock + // throw an InvalidOperationException if they call Write + // 2. About to write and see that there is a pending cancel from the CancelPendingFlush, go to 1 to see what happens + // 3. Enters the Send and sees the Dispose state from DisposeAndRemoveAsync and releases the lock + // 4. No Send in progress + await WriteLock.WaitAsync(); + try + { + // Complete the applications read loop + Application?.Output.Complete(transportTask.Exception?.InnerException); } + finally + { + WriteLock.Release(); +>>>>>>> release/2.1 + } + + Application?.Input.CancelPendingRead(); + + await transportTask.NoThrow(); + Application?.Input.Complete(); + + Log.WaitingForTransportAndApplication(_logger, TransportType); + + // A poorly written application *could* in theory get stuck forever and it'll show up as a memory leak + // Wait for application so we can complete the writer safely + await applicationTask.NoThrow(); + Log.TransportAndApplicationComplete(_logger, TransportType); + + // Shutdown application side now that it's finished + Transport?.Output.Complete(applicationTask.Exception?.InnerException); + + // Close the reading side after both sides run + Transport?.Input.Complete(); + + // Observe exceptions + await Task.WhenAll(transportTask, applicationTask); } // Notify all waiters that we're done disposing @@ -353,6 +401,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal } } +<<<<<<< HEAD internal bool TryActivatePersistentConnection( ConnectionDelegate connectionDelegate, IHttpTransport transport, @@ -533,6 +582,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal await connectionDelegate(this); } +======= +>>>>>>> release/2.1 internal void StartSendCancellation() { lock (_sendingLock) @@ -542,10 +593,18 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal _sendCts = new CancellationTokenSource(); SendingToken = _sendCts.Token; } +<<<<<<< HEAD +======= + +>>>>>>> release/2.1 _startedSendTime = DateTime.UtcNow.Ticks; _activeSend = true; } } +<<<<<<< HEAD +======= + +>>>>>>> release/2.1 internal void TryCancelSend(long currentTicks) { lock (_sendingLock) @@ -559,6 +618,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal } } } +<<<<<<< HEAD +======= + +>>>>>>> release/2.1 internal void StopSendCancellation() { lock (_sendingLock) diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs index c40a80b9ce..df6ce3b676 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs @@ -142,7 +142,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal connection.SupportedFormats = TransferFormat.Text; // We only need to provide the Input channel since writing to the application is handled through /send. +<<<<<<< HEAD var sse = new ServerSentEventsServerTransport(connection.Application.Input, connection.ConnectionId, connection, _loggerFactory); +======= + var sse = new ServerSentEventsTransport(connection.Application.Input, connection.ConnectionId, connection, _loggerFactory); +>>>>>>> release/2.1 await DoPersistentConnection(connectionDelegate, sse, context, connection); } @@ -191,9 +195,83 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal if (!await connection.CancelPreviousPoll(context)) { +<<<<<<< HEAD // Connection closed. It's already set the response status code. return; } +======= + if (connection.Status == HttpConnectionStatus.Disposed) + { + Log.ConnectionDisposed(_logger, connection.ConnectionId); + + // The connection was disposed + context.Response.StatusCode = StatusCodes.Status404NotFound; + context.Response.ContentType = "text/plain"; + return; + } + + if (connection.Status == HttpConnectionStatus.Active) + { + var existing = connection.GetHttpContext(); + Log.ConnectionAlreadyActive(_logger, connection.ConnectionId, existing.TraceIdentifier); + } + + using (connection.Cancellation) + { + // Cancel the previous request + connection.Cancellation?.Cancel(); + + try + { + // Wait for the previous request to drain + await connection.PreviousPollTask; + } + catch (OperationCanceledException) + { + // Previous poll canceled due to connection closing, close this poll too + context.Response.ContentType = "text/plain"; + context.Response.StatusCode = StatusCodes.Status204NoContent; + return; + } + + connection.PreviousPollTask = currentRequestTcs.Task; + } + + // Mark the connection as active + connection.Status = HttpConnectionStatus.Active; + + // Raise OnConnected for new connections only since polls happen all the time + if (connection.ApplicationTask == null) + { + Log.EstablishedConnection(_logger); + + connection.ApplicationTask = ExecuteApplication(connectionDelegate, connection); + + context.Response.ContentType = "application/octet-stream"; + + // This request has no content + context.Response.ContentLength = 0; + + // On the first poll, we flush the response immediately to mark the poll as "initialized" so future + // requests can be made safely + connection.TransportTask = context.Response.Body.FlushAsync(); + } + else + { + Log.ResumingConnection(_logger); + + // REVIEW: Performance of this isn't great as this does a bunch of per request allocations + connection.Cancellation = new CancellationTokenSource(); + + var timeoutSource = new CancellationTokenSource(); + var tokenSource = CancellationTokenSource.CreateLinkedTokenSource(connection.Cancellation.Token, context.RequestAborted, timeoutSource.Token); + + // Dispose these tokens when the request is over + context.Response.RegisterForDispose(timeoutSource); + context.Response.RegisterForDispose(tokenSource); + + var longPolling = new LongPollingTransport(timeoutSource.Token, connection.Application.Input, _loggerFactory, connection); +>>>>>>> release/2.1 // Create a new Tcs every poll to keep track of the poll finishing, so we can properly wait on previous polls var currentRequestTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -236,7 +314,23 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal connection.MarkInactive(); } } +<<<<<<< HEAD else if (resultTask.IsFaulted || resultTask.IsCanceled) +======= + else if (connection.TransportTask.IsFaulted || connection.TransportTask.IsCanceled) + { + // Cancel current request to release any waiting poll and let dispose aquire the lock + currentRequestTcs.TrySetCanceled(); + + // We should be able to safely dispose because there's no more data being written + // We don't need to wait for close here since we've already waited for both sides + await _manager.DisposeAndRemoveAsync(connection, closeGracefully: false); + + // Don't poll again if we've removed the connection completely + pollAgain = false; + } + else if (context.Response.StatusCode == StatusCodes.Status204NoContent) +>>>>>>> release/2.1 { // Cancel current request to release any waiting poll and let dispose acquire the lock currentRequestTcs.TrySetCanceled(); @@ -444,6 +538,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal // Other code isn't guaranteed to be able to acquire the lock before another write // even if CancelPendingFlush is called, and the other write could hang if there is backpressure connection.Application.Output.Complete(); +<<<<<<< HEAD return; } catch (IOException ex) @@ -453,6 +548,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal context.Response.StatusCode = StatusCodes.Status400BadRequest; context.Response.ContentType = "text/plain"; +======= +>>>>>>> release/2.1 return; } diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs index b0f4b079fb..85c9701242 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs @@ -32,7 +32,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal private readonly ILogger _logger; private readonly ILogger _connectionLogger; private readonly bool _useSendTimeout = true; +<<<<<<< HEAD private readonly TimeSpan _disconnectTimeout; +======= +>>>>>>> release/2.1 public HttpConnectionManager(ILoggerFactory loggerFactory, IHostApplicationLifetime appLifetime) : this(loggerFactory, appLifetime, Options.Create(new ConnectionOptions() { DisconnectTimeout = ConnectionOptionsSetup.DefaultDisconectTimeout })) @@ -53,6 +56,15 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal // Register these last as the callbacks could run immediately appLifetime.ApplicationStarted.Register(() => Start()); appLifetime.ApplicationStopping.Register(() => CloseConnections()); +<<<<<<< HEAD +======= + _nextHeartbeat = new TimerAwaitable(_heartbeatTickRate, _heartbeatTickRate); + + if (AppContext.TryGetSwitch("Microsoft.AspNetCore.Http.Connections.DoNotUseSendTimeout", out var timeoutDisabled)) + { + _useSendTimeout = !timeoutDisabled; + } +>>>>>>> release/2.1 } public void Start() @@ -161,10 +173,31 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal // Capture the connection state var lastSeenUtc = connection.LastSeenUtcIfInactive; +<<<<<<< HEAD var utcNow = DateTimeOffset.UtcNow; // Once the decision has been made to dispose we don't check the status again // But don't clean up connections while the debugger is attached. if (!Debugger.IsAttached && lastSeenUtc.HasValue && (utcNow - lastSeenUtc.Value).TotalSeconds > _disconnectTimeout.TotalSeconds) +======= + await connection.StateLock.WaitAsync(); + + try + { + // Capture the connection state + status = connection.Status; + + lastSeenUtc = connection.LastSeenUtc; + } + finally + { + connection.StateLock.Release(); + } + + var utcNow = DateTimeOffset.UtcNow; + // Once the decision has been made to dispose we don't check the status again + // But don't clean up connections while the debugger is attached. + if (!Debugger.IsAttached && status == HttpConnectionStatus.Inactive && (utcNow - lastSeenUtc).TotalSeconds > 5) +>>>>>>> release/2.1 { Log.ConnectionTimedOut(_logger, connection.ConnectionId); HttpConnectionsEventSource.Log.ConnectionTimedOut(connection.ConnectionId); diff --git a/src/SignalR/common/Http.Connections/src/Internal/TaskExtensions.cs b/src/SignalR/common/Http.Connections/src/Internal/TaskExtensions.cs index 9608a67272..efbbb4a239 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/TaskExtensions.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/TaskExtensions.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + using System.Runtime.CompilerServices; + namespace System.Threading.Tasks { internal static class TaskExtensions @@ -21,4 +23,4 @@ namespace System.Threading.Tasks public void OnCompleted(Action continuation) => _task.GetAwaiter().OnCompleted(continuation); public void UnsafeOnCompleted(Action continuation) => OnCompleted(continuation); } -} \ No newline at end of file +} diff --git a/src/SignalR/common/Http.Connections/src/Internal/Transports/LongPollingServerTransport.cs b/src/SignalR/common/Http.Connections/src/Internal/Transports/LongPollingServerTransport.cs index 3432e37039..8ecf395276 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/Transports/LongPollingServerTransport.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/Transports/LongPollingServerTransport.cs @@ -40,24 +40,24 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal.Transports var result = await _application.ReadAsync(token); var buffer = result.Buffer; - if (buffer.IsEmpty && (result.IsCompleted || result.IsCanceled)) - { - Log.LongPolling204(_logger); - context.Response.ContentType = "text/plain"; - context.Response.StatusCode = StatusCodes.Status204NoContent; - return; - } - - // We're intentionally not checking cancellation here because we need to drain messages we've got so far, - // but it's too late to emit the 204 required by being canceled. - - Log.LongPollingWritingMessage(_logger, buffer.Length); - - context.Response.ContentLength = buffer.Length; - context.Response.ContentType = "application/octet-stream"; - try { + if (buffer.IsEmpty && (result.IsCompleted || result.IsCanceled)) + { + Log.LongPolling204(_logger); + context.Response.ContentType = "text/plain"; + context.Response.StatusCode = StatusCodes.Status204NoContent; + return; + } + + // We're intentionally not checking cancellation here because we need to drain messages we've got so far, + // but it's too late to emit the 204 required by being canceled. + + Log.LongPollingWritingMessage(_logger, buffer.Length); + + context.Response.ContentLength = buffer.Length; + context.Response.ContentType = "application/octet-stream"; + _connection?.StartSendCancellation(); await context.Response.Body.WriteAsync(buffer, _connection?.SendingToken ?? default); } diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs index 2dabe1adbc..6fa5ed0a8a 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs @@ -1098,7 +1098,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests await _sync.WaitToContinue(); cancellationToken.ThrowIfCancellationRequested(); } -#if NETCOREAPP2_1 public override async ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) { if (_isSSE) @@ -1110,7 +1109,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests await _sync.WaitToContinue(); cancellationToken.ThrowIfCancellationRequested(); } -#endif } [Fact] @@ -1953,6 +1951,110 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests } } + [Fact] + public async Task DeleteEndpointTerminatesLongPollingWithHangingApplication() + { + using (StartVerifiableLog(out var loggerFactory, LogLevel.Debug)) + { + var manager = CreateConnectionManager(loggerFactory); + var pipeOptions = new PipeOptions(pauseWriterThreshold: 2, resumeWriterThreshold: 1); + var connection = manager.CreateConnection(pipeOptions, pipeOptions); + connection.TransportType = HttpTransportType.LongPolling; + + var dispatcher = new HttpConnectionDispatcher(manager, loggerFactory); + + var context = MakeRequest("/foo", connection); + + var services = new ServiceCollection(); + services.AddSingleton(); + var builder = new ConnectionBuilder(services.BuildServiceProvider()); + builder.UseConnectionHandler(); + var app = builder.Build(); + var options = new HttpConnectionDispatcherOptions(); + + var pollTask = dispatcher.ExecuteAsync(context, options, app); + Assert.True(pollTask.IsCompleted); + + // Now send the second poll + pollTask = dispatcher.ExecuteAsync(context, options, app); + + // Issue the delete request and make sure the poll completes + var deleteContext = new DefaultHttpContext(); + deleteContext.Request.Path = "/foo"; + deleteContext.Request.QueryString = new QueryString($"?id={connection.ConnectionId}"); + deleteContext.Request.Method = "DELETE"; + + Assert.False(pollTask.IsCompleted); + + await dispatcher.ExecuteAsync(deleteContext, options, app).OrTimeout(); + + await pollTask.OrTimeout(); + + // Verify that transport shuts down + await connection.TransportTask.OrTimeout(); + + // Verify the response from the DELETE request + Assert.Equal(StatusCodes.Status202Accepted, deleteContext.Response.StatusCode); + Assert.Equal("text/plain", deleteContext.Response.ContentType); + Assert.Equal(HttpConnectionStatus.Disposed, connection.Status); + + // Verify the connection not removed because application is hanging + Assert.True(manager.TryGetConnection(connection.ConnectionId, out _)); + } + } + + [Fact] + public async Task PollCanReceiveFinalMessageAfterAppCompletes() + { + using (StartVerifiableLog(out var loggerFactory, LogLevel.Debug)) + { + var transportType = HttpTransportType.LongPolling; + var manager = CreateConnectionManager(loggerFactory); + var dispatcher = new HttpConnectionDispatcher(manager, loggerFactory); + var connection = manager.CreateConnection(); + connection.TransportType = transportType; + + var waitForMessageTcs1 = new TaskCompletionSource(); + var messageTcs1 = new TaskCompletionSource(); + var waitForMessageTcs2 = new TaskCompletionSource(); + var messageTcs2 = new TaskCompletionSource(); + ConnectionDelegate connectionDelegate = async c => + { + await waitForMessageTcs1.Task.OrTimeout(); + await c.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes("Message1")).OrTimeout(); + messageTcs1.TrySetResult(null); + await waitForMessageTcs2.Task.OrTimeout(); + await c.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes("Message2")).OrTimeout(); + messageTcs2.TrySetResult(null); + }; + { + var options = new HttpConnectionDispatcherOptions(); + var context = MakeRequest("/foo", connection); + await dispatcher.ExecuteAsync(context, options, connectionDelegate).OrTimeout(); + + // second poll should have data + waitForMessageTcs1.SetResult(null); + await messageTcs1.Task.OrTimeout(); + + var ms = new MemoryStream(); + context.Response.Body = ms; + // Now send the second poll + await dispatcher.ExecuteAsync(context, options, connectionDelegate).OrTimeout(); + Assert.Equal("Message1", Encoding.UTF8.GetString(ms.ToArray())); + + waitForMessageTcs2.SetResult(null); + await messageTcs2.Task.OrTimeout(); + + context = MakeRequest("/foo", connection); + ms.Seek(0, SeekOrigin.Begin); + context.Response.Body = ms; + // This is the third poll which gets the final message after the app is complete + await dispatcher.ExecuteAsync(context, options, connectionDelegate).OrTimeout(); + Assert.Equal("Message2", Encoding.UTF8.GetString(ms.ToArray())); + } + } + } + [Fact] public async Task NegotiateDoesNotReturnWebSocketsWhenNotAvailable() { diff --git a/src/SignalR/common/Shared/PipeWriterStream.cs b/src/SignalR/common/Shared/PipeWriterStream.cs index 2afdc723d2..43474433f4 100644 --- a/src/SignalR/common/Shared/PipeWriterStream.cs +++ b/src/SignalR/common/Shared/PipeWriterStream.cs @@ -74,7 +74,6 @@ namespace System.IO.Pipelines _length += source.Length; var task = _pipeWriter.WriteAsync(source); - if (task.IsCompletedSuccessfully) { // Cancellation can be triggered by PipeWriter.CancelPendingFlush diff --git a/src/SignalR/common/testassets/Tests.Utils/VerifyNoErrorsScope.cs b/src/SignalR/common/testassets/Tests.Utils/VerifyNoErrorsScope.cs index f23712c8d5..0e3c8f47a4 100644 --- a/src/SignalR/common/testassets/Tests.Utils/VerifyNoErrorsScope.cs +++ b/src/SignalR/common/testassets/Tests.Utils/VerifyNoErrorsScope.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -32,6 +32,32 @@ namespace Microsoft.AspNetCore.SignalR.Tests var results = _sink.GetLogs().Where(w => w.Write.LogLevel >= LogLevel.Error).ToList(); +#if NETCOREAPP2_1 || NETCOREAPP2_2 || NET461 + // -- Remove this code after 2.2 -- + // This section of code is resolving test flakiness caused by a race in LongPolling + // The race has been resolved in version 3.0 + // The below code tries to find is a DELETE request has arrived from the client before removing error logs associated with the race + // We do this because we don't want to hide any actual issues, but we feel confident that looking for DELETE first wont hide any real problems + var foundDelete = false; + var allLogs = _sink.GetLogs(); + foreach (var log in allLogs) + { + if (foundDelete == false && log.Write.Message.Contains("Request starting") && log.Write.Message.Contains("DELETE")) + { + foundDelete = true; + } + + if (foundDelete) + { + if ((log.Write.EventId.Name == "LongPollingTerminated" || log.Write.EventId.Name == "ApplicationError" || log.Write.EventId.Name == "FailedDispose") + && log.Write.Exception?.Message.Contains("Reading is not allowed after reader was completed.") == true) + { + results.Remove(log); + } + } + } +#endif + if (_expectedErrorsFilter != null) { results = results.Where(w => !_expectedErrorsFilter(w.Write)).ToList(); @@ -62,4 +88,4 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } } -} \ No newline at end of file +} diff --git a/src/SignalR/server/Core/src/HubConnectionContext.cs b/src/SignalR/server/Core/src/HubConnectionContext.cs index 11e05c177c..b299b2c93b 100644 --- a/src/SignalR/server/Core/src/HubConnectionContext.cs +++ b/src/SignalR/server/Core/src/HubConnectionContext.cs @@ -318,6 +318,7 @@ namespace Microsoft.AspNetCore.SignalR return default; } + // TODO: cancel? return new ValueTask(TryWritePingSlowAsync()); } @@ -701,6 +702,26 @@ namespace Microsoft.AspNetCore.SignalR _receivedMessageTimeoutEnabled = false; } } + finally + { + _ = InnerAbortConnection(connection); + } + } + + private static async Task InnerAbortConnection(HubConnectionContext connection) + { + // We lock to make sure all writes are done before triggering the completion of the pipe + await connection._writeLock.WaitAsync(); + try + { + // Communicate the fact that we're finished triggering abort callbacks + // HubOnDisconnectedAsync is waiting on this to complete the Pipe + connection._abortCompletedTcs.TrySetResult(null); + } + finally + { + connection._writeLock.Release(); + } } private static class Log diff --git a/version.props b/version.props new file mode 100644 index 0000000000..aede0b4113 --- /dev/null +++ b/version.props @@ -0,0 +1,56 @@ + + + 2 + 1 + 16 + servicing + Servicing + t000 + $(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).$(AspNetCorePatchVersion) + 0.1.$(AspNetCorePatchVersion) + + + 1$(AspNetCoreMajorVersion) + $(AspNetCoreMinorVersion) + $(AspNetCorePatchVersion) + + $(PreReleaseLabel)-$(BuildNumber) + $(PreReleaseBrandingLabel) Build $(BuildNumber) + + + true + + false + true + false + + + $(VersionPrefix) + $(PackageBrandingVersion) $(BrandingVersionSuffix) + + + $(VersionPrefix) + $(VersionPrefix)-$(VersionSuffix) + + + $(ExperimentalVersionPrefix) + $(ExperimentalVersionPrefix)-$(VersionSuffix) + + pb-$(DotNetProductBuildId) + $(VersionSuffix)+$(VersionMetadata) + + release/$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion) + + + $(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).$([MSBuild]::Subtract($(AspNetCorePatchVersion), 1)) + + + + + + + + + + +