diff --git a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs index 576e69326f..2fbb560c85 100644 --- a/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs +++ b/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs @@ -1626,6 +1626,7 @@ namespace Microsoft.AspNetCore.SignalR.Client { private readonly HubConnection _hubConnection; private readonly ILogger _logger; + private readonly bool _hasInherentKeepAlive; private readonly object _lock = new object(); private readonly Dictionary _pendingCalls = new Dictionary(StringComparer.Ordinal); @@ -1637,7 +1638,6 @@ namespace Microsoft.AspNetCore.SignalR.Client private long _nextActivationServerTimeout; private long _nextActivationSendPing; - private bool _hasInherentKeepAlive; public ConnectionContext Connection { get; } public Task ReceiveTask { get; set; } @@ -1764,7 +1764,10 @@ namespace Microsoft.AspNetCore.SignalR.Client // Old clients never ping, and shouldn't be timed out, so ping to tell the server that we should be timed out if we stop. // The TimerLoop is started from the ReceiveLoop with the connection lock still acquired. _hubConnection._state.AssertInConnectionLock(); - await _hubConnection.SendHubMessage(this, PingMessage.Instance); + if (!_hasInherentKeepAlive) + { + await _hubConnection.SendHubMessage(this, PingMessage.Instance); + } // initialize the timers timer.Start(); @@ -1794,7 +1797,12 @@ namespace Microsoft.AspNetCore.SignalR.Client // Internal for testing internal async Task RunTimerActions() { - if (!_hasInherentKeepAlive && DateTime.UtcNow.Ticks > Volatile.Read(ref _nextActivationServerTimeout)) + if (_hasInherentKeepAlive) + { + return; + } + + if (DateTime.UtcNow.Ticks > Volatile.Read(ref _nextActivationServerTimeout)) { OnServerTimeout(); } diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs index 93cc3dd863..e254a79c5c 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.Protocol.cs @@ -648,6 +648,33 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests await connection.DisposeAsync().OrTimeout(); } } + + [Fact] + public async Task ClientWithInherentKeepAliveDoesNotPing() + { + var connection = new TestConnection(hasInherentKeepAlive: true); + var hubConnection = CreateHubConnection(connection); + + hubConnection.TickRate = TimeSpan.FromMilliseconds(30); + hubConnection.KeepAliveInterval = TimeSpan.FromMilliseconds(80); + + try + { + await hubConnection.StartAsync().OrTimeout(); + + await Task.Delay(1000); + + await hubConnection.DisposeAsync().OrTimeout(); + await connection.DisposeAsync().OrTimeout(); + + Assert.Equal(0, (await connection.ReadAllSentMessagesAsync(ignorePings: false).OrTimeout()).Count); + } + finally + { + await hubConnection.DisposeAsync().OrTimeout(); + await connection.DisposeAsync().OrTimeout(); + } + } } } } diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs index fbc516e95c..0da16cf160 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs @@ -10,6 +10,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Protocol; @@ -18,7 +19,7 @@ using Newtonsoft.Json.Linq; namespace Microsoft.AspNetCore.SignalR.Client.Tests { - internal class TestConnection : ConnectionContext + internal class TestConnection : ConnectionContext, IConnectionInherentKeepAliveFeature { private readonly bool _autoHandshake; private readonly TaskCompletionSource _started = new TaskCompletionSource(); @@ -30,6 +31,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests private readonly Func _onStart; private readonly Func _onDispose; + private readonly bool _hasInherentKeepAlive; public override string ConnectionId { get; set; } @@ -41,17 +43,22 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests public override IDictionary Items { get; set; } = new ConnectionItems(); - public TestConnection(Func onStart = null, Func onDispose = null, bool autoHandshake = true) + bool IConnectionInherentKeepAliveFeature.HasInherentKeepAlive => _hasInherentKeepAlive; + + public TestConnection(Func onStart = null, Func onDispose = null, bool autoHandshake = true, bool hasInherentKeepAlive = false) { _autoHandshake = autoHandshake; _onStart = onStart ?? (() => Task.CompletedTask); _onDispose = onDispose ?? (() => Task.CompletedTask); + _hasInherentKeepAlive = hasInherentKeepAlive; var options = new PipeOptions(readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); Application = pair.Application; Transport = pair.Transport; + + Features.Set(this); } public override ValueTask DisposeAsync() => DisposeCoreAsync(); @@ -119,6 +126,10 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests while (true) { var result = await ReadSentTextMessageAsyncInner(); + if (result == null) + { + return null; + } var receivedMessageType = (int?)JObject.Parse(result)["type"]; diff --git a/src/SignalR/clients/ts/signalr/src/HubConnection.ts b/src/SignalR/clients/ts/signalr/src/HubConnection.ts index d47006b1d3..c536bfa844 100644 --- a/src/SignalR/clients/ts/signalr/src/HubConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HubConnection.ts @@ -589,6 +589,10 @@ export class HubConnection { } private resetKeepAliveInterval() { + if (this.connection.features.inherentKeepAlive) { + return; + } + this.cleanupPingTimer(); this.pingServerHandle = setTimeout(async () => { if (this.connectionState === HubConnectionState.Connected) { diff --git a/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts index 2335e413e9..e74de54b22 100644 --- a/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts @@ -127,6 +127,25 @@ describe("HubConnection", () => { } }); }); + + it("does not send pings for connection with inherentKeepAlive", async () => { + await VerifyLogger.run(async (logger) => { + const connection = new TestConnection(true, true); + const hubConnection = createHubConnection(connection, logger); + + hubConnection.keepAliveIntervalInMilliseconds = 5; + + try { + await hubConnection.start(); + await delayUntil(500); + + const numPings = connection.sentData.filter((s) => JSON.parse(s).type === MessageType.Ping).length; + expect(numPings).toEqual(0); + } finally { + await hubConnection.stop(); + } + }); + }); }); describe("stop", () => { diff --git a/src/SignalR/clients/ts/signalr/tests/TestConnection.ts b/src/SignalR/clients/ts/signalr/tests/TestConnection.ts index 0ad7d89a95..e4f35281eb 100644 --- a/src/SignalR/clients/ts/signalr/tests/TestConnection.ts +++ b/src/SignalR/clients/ts/signalr/tests/TestConnection.ts @@ -17,13 +17,14 @@ export class TestConnection implements IConnection { private autoHandshake: boolean | null; - constructor(autoHandshake: boolean = true) { + constructor(autoHandshake: boolean = true, hasInherentKeepAlive: boolean = false) { this.onreceive = null; this.onclose = null; this.sentData = []; this.lastInvocationId = null; this.autoHandshake = autoHandshake; this.baseUrl = "http://example.com"; + this.features.inherentKeepAlive = hasInherentKeepAlive; } public start(): Promise {