fix auto reconnect after stop bug (#707)

* try to fix * fix status bug * add test * get connection status * task extensions * modify get status * intent * skip * skip * ConditionalFact * timeout still need to reconnect * update * updated * update * expected error * remove pkg * move
Azure · Nov 4, 2019 · 6a17e9e · 6a17e9e
1 parent db2086d
commit 6a17e9e
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 12 deletions.
diff --git a/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs b/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs
@@ -181,7 +181,7 @@ public Task StopAsync()
             {
                 Log.UnexpectedExceptionInStop(Logger, ConnectionId, ex);
             }
-            
+
             return Task.CompletedTask;
         }
 
@@ -388,7 +388,7 @@ private async Task<bool> ReceiveHandshakeResponseAsync(PipeReader input, Cancell
                             {
                                 Log.HandshakeError(Logger, handshakeResponse.ErrorMessage, ConnectionId);
                             }
-                            
+
                             return false;
                         }
                     }

diff --git a/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionContainerBase.cs b/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionContainerBase.cs
@@ -26,10 +26,13 @@ internal abstract class ServiceConnectionContainerBase : IServiceConnectionConta
         private readonly object _statusLock = new object();
 
         private readonly AckHandler _ackHandler;
+
         private volatile List<IServiceConnection> _fixedServiceConnections;
 
         private volatile ServiceConnectionStatus _status;
 
+        private volatile bool _terminated = false;
+
         protected ILogger Logger { get; }
 
         protected List<IServiceConnection> FixedServiceConnections
@@ -88,7 +91,7 @@ protected ServiceConnectionContainerBase(IServiceConnectionFactory serviceConnec
             else
             {
                 initial = new List<IServiceConnection>(initialConnections);
-                foreach(var conn in initial)
+                foreach (var conn in initial)
                 {
                     conn.ConnectionStatusChanged += OnConnectionStatusChanged;
                 }
@@ -112,7 +115,11 @@ public Task StartAsync()
             return Task.WhenAll(FixedServiceConnections.Select(c => StartCoreAsync(c)));
         }
 
-        public virtual Task StopAsync() => Task.WhenAll(FixedServiceConnections.Select(c => c.StopAsync()));
+        public virtual Task StopAsync()
+        {
+            _terminated = true;
+            return Task.WhenAll(FixedServiceConnections.Select(c => c.StopAsync()));
+        }
 
         /// <summary>
         /// Start and manage the whole connection lifetime
@@ -155,6 +162,11 @@ protected virtual async Task OnConnectionComplete(IServiceConnection serviceConn
                 throw new ArgumentNullException(nameof(serviceConnection));
             }
 
+            if (_terminated)
+            {
+                return;
+            }
+
             serviceConnection.ConnectionStatusChanged -= OnConnectionStatusChanged;
 
             if (serviceConnection.Status == ServiceConnectionStatus.Connected)

diff --git a/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionStatus.cs b/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionStatus.cs
@@ -9,6 +9,6 @@ internal enum ServiceConnectionStatus
         Inited,
         Disconnected,
         Connecting,
-        Connected
+        Connected,
     }
 }
diff --git a/src/Microsoft.Azure.SignalR.Management/Properties/AssemblyInfo.cs b/src/Microsoft.Azure.SignalR.Management/Properties/AssemblyInfo.cs
@@ -1,3 +1,4 @@
 using System.Runtime.CompilerServices;
 
-[assembly : InternalsVisibleTo("Microsoft.Azure.SignalR.Management.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
+[assembly : InternalsVisibleTo("Microsoft.Azure.SignalR.Management.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
+[assembly : InternalsVisibleTo("Microsoft.Azure.SignalR.E2ETests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
diff --git a/src/Microsoft.Azure.SignalR.Management/ServiceHubContext.cs b/src/Microsoft.Azure.SignalR.Management/ServiceHubContext.cs
@@ -3,14 +3,19 @@
 
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.SignalR;
+using Microsoft.Azure.SignalR.Common.ServiceConnections;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.Azure.SignalR.Management
 {
     internal class ServiceHubContext : IServiceHubContext
     {
-        private IHubContext<Hub> _hubContext;
-        private ServiceProvider _serviceProvider;
+        private readonly IHubContext<Hub> _hubContext;
+
+        private readonly ServiceProvider _serviceProvider;
+
+        // for test only
+        internal IServiceConnectionContainer ConnectionContainer { get; }
 
         public IHubClients Clients => _hubContext.Clients;
 
@@ -23,6 +28,7 @@ public ServiceHubContext(IHubContext<Hub> hubContext, IHubLifetimeManagerForUser
             _hubContext = hubContext;
             UserGroups = new UserGroupsManager(lifetimeManager);
             _serviceProvider = serviceProvider;
+            ConnectionContainer = _serviceProvider.GetService<IServiceConnectionContainer>();
         }
 
         public async Task DisposeAsync()

diff --git a/src/Microsoft.Azure.SignalR.Management/ServiceManager.cs b/src/Microsoft.Azure.SignalR.Management/ServiceManager.cs
@@ -51,7 +51,11 @@ public async Task<IServiceHubContext> CreateHubContextAsync(string hubName, ILog
                         var clientConnectionFactory = new ClientConnectionFactory();
                         ConnectionDelegate connectionDelegate = connectionContext => Task.CompletedTask;
                         var serviceConnectionFactory = new ServiceConnectionFactory(serviceProtocol, clientConnectionManager, connectionFactory, loggerFactory, connectionDelegate, clientConnectionFactory);
-                        var weakConnectionContainer = new WeakServiceConnectionContainer(serviceConnectionFactory, _serviceManagerOptions.ConnectionCount, new HubServiceEndpoint(hubName, _endpointProvider, _endpoint), NullLogger.Instance);
+                        var weakConnectionContainer = new WeakServiceConnectionContainer(
+                            serviceConnectionFactory,
+                            _serviceManagerOptions.ConnectionCount,
+                            new HubServiceEndpoint(hubName, _endpointProvider, _endpoint),
+                            loggerFactory?.CreateLogger(nameof(WeakServiceConnectionContainer)) ?? NullLogger.Instance);
 
                         var serviceCollection = new ServiceCollection();
                         serviceCollection.AddSignalRCore();

diff --git a/src/Microsoft.Azure.SignalR.Management/TaskExtensions.cs b/src/Microsoft.Azure.SignalR.Management/TaskExtensions.cs
@@ -8,7 +8,7 @@ internal static class TaskExtensions
     {
         private static readonly TimeSpan _defaultTimeout = TimeSpan.FromMinutes(5);
 
-        public static async Task OrTimeout(this Task task, CancellationToken cancellationToken = default)
+        public static async Task OrTimeout(this Task task, CancellationToken cancellationToken)
         {
             if (cancellationToken == default)
             {

diff --git a/test/Microsoft.Azure.SignalR.E2ETests/Management/ServiceHubContextE2EFacts.cs b/test/Microsoft.Azure.SignalR.E2ETests/Management/ServiceHubContextE2EFacts.cs
@@ -162,7 +162,7 @@ await RunTestCore(clientEndpoint, clientAccessTokens,
         internal async Task SendToConnectionTest(ServiceTransportType serviceTransportType, string appName)
         {
             var testServer = _testServerFactory.Create(TestOutputHelper);
-            await testServer.StartAsync(new Dictionary<string, string>{ [TestStartup.ApplicationName] = appName });
+            await testServer.StartAsync(new Dictionary<string, string> { [TestStartup.ApplicationName] = appName });
 
             var task = testServer.HubConnectionManager.WaitForConnectionCountAsync(1);
 
@@ -171,7 +171,7 @@ internal async Task SendToConnectionTest(ServiceTransportType serviceTransportTy
             try
             {
                 await RunTestCore(clientEndpoint, clientAccessTokens,
-                    async () => 
+                    async () =>
                     {
                         var connectionId = await task.OrTimeout();
                         await serviceHubContext.Clients.Client(connectionId).SendAsync(MethodName, Message);
@@ -217,6 +217,28 @@ await RunTestCore(clientEndpoint, clientAccessTokens,
             }
         }
 
+        [ConditionalFact]
+        [SkipIfConnectionStringNotPresent]
+        internal async Task StopServiceHubContextTest()
+        {
+            using (StartVerifiableLog(out var loggerFactory, LogLevel.Debug, expectedErrors: context => context.EventId == new EventId(2, "EndpointOffline")))
+            {
+                var serviceManager = new ServiceManagerBuilder()
+                    .WithOptions(o =>
+                    {
+                        o.ConnectionString = TestConfiguration.Instance.ConnectionString;
+                        o.ConnectionCount = 1;
+                        o.ServiceTransportType = ServiceTransportType.Persistent;
+                    })
+                    .Build();
+                var serviceHubContext = await serviceManager.CreateHubContextAsync("hub", loggerFactory);
+                var connectionContainer = ((ServiceHubContext)serviceHubContext).ConnectionContainer;
+                await serviceHubContext.DisposeAsync();
+                await Task.Delay(500);
+                Assert.Equal(ServiceConnectionStatus.Disconnected, connectionContainer.Status);
+            }
+        }
+
         private static IDictionary<string, List<string>> GenerateUserGroupDict(IList<string> userNames, IList<string> groupNames)
         {
             return (from i in Enumerable.Range(0, userNames.Count)
-Original file line number
+Diff line change
@@ Expand Up / @@ -181,7 +181,7 @@ public Task StopAsync() @@
                 {
                     Log.UnexpectedExceptionInStop(Logger, ConnectionId, ex);
                 }
                 return Task.CompletedTask;
             }
@@ Expand Down Expand Up @@
                                 {
                                     Log.HandshakeError(Logger, handshakeResponse.ErrorMessage, ConnectionId);
                                 }
                                 return false;
                             }
                         }
@@ Expand Down @@