Dispose should close the server connection (#1960)

* Dispose should close the server connection

* Fix tests

* Disable flacky tests

Author: vicancy

src/Microsoft.Azure.SignalR.AspNet/ServerConnections/ServiceConnectionManager.cs

@@ -134,6 +134,7 @@ public Task StopGetServersPing()
 
         public void Dispose()
         {
+            StopAsync().GetAwaiter().GetResult();
         }
 
         private async Task ConnectionInitializedAsync()

src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionContainerBase.cs

@@ -261,6 +261,7 @@ public Task StopGetServersPing()
         // Ready for scalable containers
         public void Dispose()
         {
+            StopAsync().GetAwaiter().GetResult();
             _statusPing.Dispose();
             _serversPing.Dispose();
             Dispose(true);
@@ -726,7 +727,7 @@ private static class Log
                 LoggerMessage.Define<ServiceEndpoint, string>(LogLevel.Debug, new EventId(9, "ReceivedServersTagPing"), "Received a servers tag ping from {endpoint} for hub {hub}.");
 
             private static readonly Action<ILogger, string, Exception> _timerAlreadyStopped =
-                LoggerMessage.Define<string>(LogLevel.Warning, new EventId(10, "TimerAlreadyStopped"), "Failed to stop {pingName} timer as it's not started");
+                LoggerMessage.Define<string>(LogLevel.Debug, new EventId(10, "TimerAlreadyStopped"), "Failed to stop {pingName} timer as it's not started");
 
             public static void EndpointOnline(ILogger logger, HubServiceEndpoint endpoint)
             {

src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionManager.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.SignalR;
@@ -9,7 +10,7 @@
 
 namespace Microsoft.Azure.SignalR
 {
-    internal class ServiceConnectionManager<THub> : IServiceConnectionManager<THub> where THub : Hub
+    internal class ServiceConnectionManager<THub> : IDisposable, IServiceConnectionManager<THub> where THub : Hub
     {
         private IServiceConnectionContainer _serviceConnection = null;
 
@@ -52,5 +53,10 @@ public Task<bool> WriteAckableMessageAsync(ServiceMessage seviceMessage, Cancell
 
             return _serviceConnection.WriteAckableMessageAsync(seviceMessage, cancellationToken);
         }
+
+        public void Dispose()
+        {
+            StopAsync().GetAwaiter().GetResult();
+        }
     }
 }

test/Microsoft.Azure.SignalR.Common.Tests/CustomizedTimerTests.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Testing.xunit;
 using Microsoft.Azure.SignalR.Tests.Common;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
@@ -62,6 +63,7 @@ await RetryWhenExceptionThrows(async () =>
         }
 
         [Fact]
+        [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX, "Flacky tests due to slow machine")]
         public async Task StartStopStartStop()
         {
             using (StartVerifiableLog(out var loggerFactory, LogLevel.Warning))
@@ -84,14 +86,9 @@ public async Task StartStopStartStop()
         }
 
         [Fact]
+        [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX, "Flacky tests due to slow machine")]
         public async Task StartStopDispose_StartDisposeStop()
         {
-            if (Environment.OSVersion.Platform == PlatformID.Unix)
-            {
-                // it will fail in osx in github action, due to slow machine.
-                // skip it for now.
-                return;
-            }
             using (StartVerifiableLog(out var loggerFactory, LogLevel.Warning))
             {
                 var callbackCount = 0;

test/Microsoft.Azure.SignalR.Tests/ServiceConnectionContainerBaseTests.cs

@@ -23,9 +23,9 @@ public ServiceConnectionContainerBaseTests(ITestOutputHelper helper) : base(help
         [InlineData(3, 1, 0)]
         public async Task TestServersPing(int startCount, int stopCount, int expectedWarn)
         {
-            using (StartVerifiableLog(out var loggerFactory, LogLevel.Warning, logChecker: logs =>
+            using (StartVerifiableLog(out var loggerFactory, LogLevel.Debug, logChecker: logs =>
             {
-                var warns = logs.Where(s => s.Write.LogLevel == LogLevel.Warning).ToList();
+                var warns = logs.Where(s => s.Write.EventId.Name == "TimerAlreadyStopped").ToList();
                 Assert.Equal(expectedWarn, warns.Count);
                 if (expectedWarn > 0)
                 {
@@ -84,9 +84,9 @@ await Task.WhenAny(connections.Select(c =>
         [InlineData(1, 3, 2, 2, 2)] // first time error stop won't break second time write.
         public async Task TestServersPingWorkSecondTime(int firstStart, int firstStop, int secondStart, int secondStop, int expectedWarn)
         {
-            using (StartVerifiableLog(out var loggerFactory, LogLevel.Warning, logChecker: logs =>
+            using (StartVerifiableLog(out var loggerFactory, LogLevel.Debug, logChecker: logs =>
             {
-                var warns = logs.Where(s => s.Write.LogLevel == LogLevel.Warning).ToList();
+                var warns = logs.Where(s => s.Write.EventId.Name == "TimerAlreadyStopped").ToList();
                 Assert.Equal(expectedWarn, warns.Count);
                 if (expectedWarn > 0)
                 {