diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubLifetimeManagerBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubLifetimeManagerBenchmark.cs index 72506c86c0..d2d0a438be 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubLifetimeManagerBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubLifetimeManagerBenchmark.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; @@ -122,4 +122,4 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks return _hubLifetimeManager.SendUsersAsync(_userIdentifiers, "MethodName", Array.Empty()); } } -} \ No newline at end of file +} diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionStartBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionStartBenchmark.cs index d39a4c2601..fbf8da9afb 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionStartBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionStartBenchmark.cs @@ -76,6 +76,6 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks public class TestConnectionInherentKeepAliveFeature : IConnectionInherentKeepAliveFeature { - public TimeSpan KeepAliveInterval { get; } = TimeSpan.Zero; + public bool HasInherentKeepAlive { get; } = true; } } diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/Shared/TestConnectionInherentKeepAliveFeature.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/Shared/TestConnectionInherentKeepAliveFeature.cs deleted file mode 100644 index d91b3f8580..0000000000 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/Shared/TestConnectionInherentKeepAliveFeature.cs +++ /dev/null @@ -1,13 +0,0 @@ -// 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; -using Microsoft.AspNetCore.Connections.Features; - -namespace Microsoft.AspNetCore.SignalR.Microbenchmarks.Shared -{ - public class TestConnectionInherentKeepAliveFeature : IConnectionInherentKeepAliveFeature - { - public TimeSpan KeepAliveInterval { get; } = TimeSpan.Zero; - } -} \ No newline at end of file diff --git a/build/dependencies.props b/build/dependencies.props index afbc6b2c05..de6e769e20 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -1,4 +1,4 @@ - + $(MSBuildAllProjects);$(MSBuildThisFileFullPath) @@ -14,7 +14,7 @@ 2.1.0-preview3-32233 2.1.0-preview3-32233 2.1.0-preview3-32233 - 2.1.0-preview3-32233 + 2.1.0-a-preview3-inherent-keep-alive-bool-17670 2.1.0-preview3-32233 2.1.0-preview3-32233 2.1.0-preview3-32233 @@ -28,7 +28,7 @@ 2.1.0-preview3-32233 2.1.0-preview3-32233 0.5.0-preview2-32233 - 2.1.0-preview3-32233 + 2.1.0-a-preview3-inherent-keep-alive-bool-17670 2.1.0-preview3-32233 2.1.0-preview3-32233 2.1.0-preview3-32233 diff --git a/samples/ClientSample/Tcp/TcpConnection.cs b/samples/ClientSample/Tcp/TcpConnection.cs index c59e73a0ad..1fbb213b9b 100644 --- a/samples/ClientSample/Tcp/TcpConnection.cs +++ b/samples/ClientSample/Tcp/TcpConnection.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.IO.Pipelines; @@ -40,7 +40,8 @@ namespace ClientSample public override string ConnectionId { get; set; } = Guid.NewGuid().ToString(); public override IDictionary Items { get; set; } = new ConnectionItems(); - public TimeSpan KeepAliveInterval => Timeout.InfiniteTimeSpan; + // We claim to have inherent keep-alive so the client doesn't kill the connection when it hasn't seen ping frames. + public bool HasInherentKeepAlive { get; } = true; public Task DisposeAsync() { diff --git a/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs b/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs index 470159ffd9..f2e827979a 100644 --- a/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs @@ -12,14 +12,13 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Connections.Client.Internal; -using Microsoft.AspNetCore.Http.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.AspNetCore.Http.Connections.Client { - public partial class HttpConnection : ConnectionContext + public partial class HttpConnection : ConnectionContext, IConnectionInherentKeepAliveFeature { private static readonly TimeSpan HttpClientTimeout = TimeSpan.FromSeconds(120); #if !NETCOREAPP2_1 @@ -31,6 +30,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Client private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(1, 1); private bool _started; private bool _disposed; + private bool _hasInherentKeepAlive; private readonly HttpClient _httpClient; private readonly HttpConnectionOptions _httpConnectionOptions; @@ -59,6 +59,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Client public override string ConnectionId { get; set; } public override IDictionary Items { get; set; } = new ConnectionItems(); + bool IConnectionInherentKeepAliveFeature.HasInherentKeepAlive => _hasInherentKeepAlive; + public HttpConnection(Uri url) : this(url, HttpTransports.All) { } @@ -102,6 +104,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Client _transportFactory = new DefaultTransportFactory(httpConnectionOptions.Transports, _loggerFactory, _httpClient, httpConnectionOptions); _logScope = new ConnectionLogScope(); _scopeDisposable = _logger.BeginScope(_logScope); + + Features.Set(this); } // Used by unit tests @@ -347,11 +351,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Client throw; } - if (transportType == HttpTransportType.LongPolling) - { - // Disable keep alives for long polling - Features.Set(new ConnectionInherentKeepAliveFeature(_httpClient.Timeout)); - } + // Disable keep alives for long polling + _hasInherentKeepAlive = transportType == HttpTransportType.LongPolling; // We successfully started, set the transport properties (we don't want to set these until the transport is definitely running). _transport = transport; @@ -465,4 +466,4 @@ namespace Microsoft.AspNetCore.Http.Connections.Client return negotiationResponse; } } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Http.Connections.Common/ConnectionInherentKeepAliveFeature.cs b/src/Microsoft.AspNetCore.Http.Connections.Common/ConnectionInherentKeepAliveFeature.cs deleted file mode 100644 index 72f6d547c6..0000000000 --- a/src/Microsoft.AspNetCore.Http.Connections.Common/ConnectionInherentKeepAliveFeature.cs +++ /dev/null @@ -1,18 +0,0 @@ -// 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; -using Microsoft.AspNetCore.Connections.Features; - -namespace Microsoft.AspNetCore.Http.Connections.Features -{ - public class ConnectionInherentKeepAliveFeature : IConnectionInherentKeepAliveFeature - { - public TimeSpan KeepAliveInterval { get; } - - public ConnectionInherentKeepAliveFeature(TimeSpan keepAliveInterval) - { - KeepAliveInterval = keepAliveInterval; - } - } -} diff --git a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs index 1e2b330860..d496d1d29d 100644 --- a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs +++ b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs @@ -24,7 +24,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal IConnectionHeartbeatFeature, ITransferFormatFeature, IHttpContextFeature, - IHttpTransportFeature + IHttpTransportFeature, + IConnectionInherentKeepAliveFeature { private readonly object _itemsLock = new object(); private readonly object _heartbeatLock = new object(); @@ -65,6 +66,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal Features.Set(this); Features.Set(this); Features.Set(this); + Features.Set(this); } public HttpConnectionContext(string id, IDuplexPipe transport, IDuplexPipe application, ILogger logger = null) @@ -97,6 +99,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal public ClaimsPrincipal User { get; set; } + public bool HasInherentKeepAlive { get; set; } + public override IDictionary Items { get diff --git a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs index 6695cb853c..fb1b7a8701 100644 --- a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs +++ b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs @@ -366,10 +366,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal private async Task ExecuteApplication(ConnectionDelegate connectionDelegate, HttpConnectionContext connection) { // Verify some initialization invariants - // We want to be positive that the IConnectionInherentKeepAliveFeature is initialized before invoking the application, if the long polling transport is in use. Debug.Assert(connection.TransportType != HttpTransportType.None, "Transport has not been initialized yet"); - Debug.Assert(connection.TransportType != HttpTransportType.LongPolling || - connection.Features.Get() != null, "Long-polling transport is in use but IConnectionInherentKeepAliveFeature as not configured"); // Jump onto the thread pool thread so blocking user code doesn't block the setup of the // connection and transport @@ -555,7 +552,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal // Configure transport-specific features. if (transportType == HttpTransportType.LongPolling) { - connection.Features.Set(new ConnectionInherentKeepAliveFeature(options.LongPolling.PollTimeout)); + connection.HasInherentKeepAlive = true; // For long polling, the requests come and go but the connection is still alive. // To make the IHttpContextFeature work well, we make a copy of the relevant properties diff --git a/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs b/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs index fd848ce440..44d6bdb969 100644 --- a/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs +++ b/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs @@ -720,7 +720,9 @@ namespace Microsoft.AspNetCore.SignalR.Client { // Check if we need keep-alive Timer timeoutTimer = null; - if (connectionState.Connection.Features.Get() == null) + + // We use '!== true' because it could be null, which we treat as false. + if (connectionState.Connection.Features.Get()?.HasInherentKeepAlive != true) { Log.StartingServerTimeoutTimer(_logger, ServerTimeout); timeoutTimer = new Timer( diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs index 5e368719a7..c0787f4f03 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs @@ -339,7 +339,8 @@ namespace Microsoft.AspNetCore.SignalR UserIdentifier = userIdProvider.GetUserId(this); - if (Features.Get() == null) + // != true needed because it could be null (which we treat as false) + if (Features.Get()?.HasInherentKeepAlive != true) { // Only register KeepAlive after protocol handshake otherwise KeepAliveTick could try to write without having a ProtocolReaderWriter Features.Get()?.OnHeartbeat(state => ((HubConnectionContext)state).KeepAliveTick(), this); diff --git a/test/Microsoft.AspNetCore.Http.Connections.Tests/HttpConnectionDispatcherTests.cs b/test/Microsoft.AspNetCore.Http.Connections.Tests/HttpConnectionDispatcherTests.cs index 2e96afd7a7..e0fbf43bd2 100644 --- a/test/Microsoft.AspNetCore.Http.Connections.Tests/HttpConnectionDispatcherTests.cs +++ b/test/Microsoft.AspNetCore.Http.Connections.Tests/HttpConnectionDispatcherTests.cs @@ -1587,12 +1587,12 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests var options = new HttpConnectionDispatcherOptions(); options.LongPolling.PollTimeout = TimeSpan.FromMilliseconds(1); // We don't care about the poll itself - Assert.Null(connection.Features.Get()); - await dispatcher.ExecuteAsync(context, options, app).OrTimeout(); - Assert.NotNull(connection.Features.Get()); - Assert.Equal(options.LongPolling.PollTimeout, connection.Features.Get().KeepAliveInterval); + Assert.True(connection.HasInherentKeepAlive); + + // Check via the feature as well to make sure it's there. + Assert.True(connection.Features.Get().HasInherentKeepAlive); } } diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Transport.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Transport.cs index 7b6bfc4bc8..8357e9e693 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Transport.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.Transport.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Connections; using Microsoft.AspNetCore.Http.Connections.Client; using Microsoft.AspNetCore.Http.Connections.Client.Internal; @@ -69,6 +70,35 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests Assert.True(requestsExecuted); } + [Theory] + [InlineData(HttpTransportType.LongPolling, true)] + [InlineData(HttpTransportType.ServerSentEvents, false)] + public async Task HttpConnectionSetsInherentKeepAliveFeature(HttpTransportType transportType, bool expectedValue) + { + var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false); + + testHttpHandler.OnRequest((request, next, token) => + { + return Task.FromResult(ResponseUtils.CreateResponse(HttpStatusCode.NoContent)); + }); + + testHttpHandler.OnNegotiate((_, cancellationToken) => + { + return ResponseUtils.CreateResponse(HttpStatusCode.OK, ResponseUtils.CreateNegotiationContent()); + }); + + await WithConnectionAsync( + CreateConnection(testHttpHandler, transportType: transportType), + async (connection) => + { + await connection.StartAsync(TransferFormat.Text).OrTimeout(); + + var feature = connection.Features.Get(); + Assert.NotNull(feature); + Assert.Equal(expectedValue, feature.HasInherentKeepAlive); + }); + } + [Theory] [InlineData(HttpTransportType.LongPolling)] [InlineData(HttpTransportType.ServerSentEvents)]